Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USWDS-Next - Alpha: Create usa-details web component #29

Draft
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Jun 26, 2024

Summary

Created an alpha version of a usa-details web component

Related issue

Closes #19

Preview link

Details component (Storybook)

Sample implementation

<usa-details name="example-group-name">
  <details open="true">
    <summary>First Amendment</summary>
    <div slot="details-body">Congress shall make no law respecting an establishment of religion, or prohibiting the free exercise thereof; or abridging the freedom of speech, or of the press; or the right of the people peaceably to assemble, and to petition the Government for a redress of grievances.</div>
  </details>
  <details>
    <summary>Second Amendment</summary>
    <div slot="details-body">A well regulated Militia, being necessary to the security of a free State, the right of the people to keep and bear Arms, shall not be infringed.</div>
  </details>      
  <details>
    <summary>Third Amendment</summary>
    <div slot="details-body">No Soldier shall, in time of peace be quartered in any house, without the consent of the Owner, nor in time of war, but in a manner to be prescribed by law.</div>
  </details>
</usa-details>

Reference

Details element (MDN)

Requirements notes (WIP)

Common component names

Details, Accordion, Collapse

Variants

Type Name Description
Border Borderless (default) No border
Bordered Border around panel and button
Group Multi select (default) Allow multiple panels in a group to be open at one time
Single select Open only one panel in a group at a time

Settings and CSS Variables

Description Related USWDS setting New CSS variable
Border width $theme-accordion-border-width --theme-details-border-width
Border color $theme-accordion-border-color --theme-details-border-color
Button background color $theme-accordion-button-background-color --theme-details-summary-background-color
Panel background color $theme-accordion-background-color --theme-details-background-color
Component font family $theme-accordion-font-family --theme-details-font-family

Possibilities for additional customization:

  • Summary/content font weight
  • Summary/content padding
  • Summary/content text color
  • Summary/content font size
  • Open/close icons
  • Icon placement (left/right)

Accessibility

See related accessibility tests

Attributes & Props

Attribute Element Description
open="true" <details> If the open attribute is present on the <details> element, the content inside its details-content slot should be visible on load.
name="[group name]" <details> If a defined name attribute is present on the <details> element, the <details> elements with the same name value will be treated as a single-select group and toggle each other.

Slots

Slot name Expected Element Example code Count
details-body <div> <div slot="details-body">[Accordion panel content]</div> 1

Expected interactions:

  • All:
    • You can show and hide content panels with enter, spacebar, mouse click, or voice command
    • You can insert rich text and other components inside the content region
  • Single select: When you open a panel in the group, the other panels in the group close
  • Multi select: When you open a panel in the group, the other panels stay in their current open/closed state

Test cases

  • Be able to change background colors, text colors, border colors, change icons, update icon placement
  • Add new style property and customize it

@amyleadem amyleadem changed the title USWDS-Next: Set up initial details component USWDS-Next: Create usa-details web component Jun 27, 2024
@amyleadem amyleadem changed the title USWDS-Next: Create usa-details web component USWDS-Next - Alpha: Create usa-details web component Jun 27, 2024
Copy link
Contributor Author

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: I had originally imagined that the implementation would look something like this for single select "accordion" behavior:

<usa-details-group>
  <usa-details name="example-details-group">
    <details>
      <summary>First Amendment</summary>
      <div slot="details-body">Congress shall make no law respecting an establishment of religion, or prohibiting the free exercise thereof; or abridging the freedom of speech, or of the press; or the right of the people peaceably to assemble, and to petition the Government for a redress of grievances.</div>
    </details>
  </usa-details>
  <usa-details name="example-details-group">
    <details>
      <summary>Second Amendment</summary>
      <div slot="details-body">A well regulated Militia, being necessary to the security of a free State, the right of the people to keep and bear Arms, shall not be infringed.</div>
    </details>
  </usa-details>
</usa-details-group>

However, this structure isolated the two <details> elements from each other in the shadow DOM and prevented the native name attribute from working as expected. I updated the structure to this instead so that we could rely on the the native functionality of the <details> element to toggle the component instead of writing custom JS:

<usa-details>
  <details open="true" name="example-details-group">
    <summary>First Amendment</summary>
    <div slot="details-body">Congress shall make no law respecting an establishment of religion, or prohibiting the free exercise thereof; or abridging the freedom of speech, or of the press; or the right of the people peaceably to assemble, and to petition the Government for a redress of grievances.</div>
  </details>
  <details name="example-details-group">
    <summary>Second Amendment</summary>
    <div slot="details-body">A well regulated Militia, being necessary to the security of a free State, the right of the people to keep and bear Arms, shall not be infringed.</div>
  </details>
</usa-details>

Curious to hear if anyone has concerns about this update.

@amyleadem amyleadem marked this pull request as ready for review July 3, 2024 19:28
Copy link
Collaborator

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! It looks just like the existing accordion. My testing focused a lot on the functionality of the component.

I tested the following:

  • Changing controls
  • Updating detail attributes & content
  • Comparing custom element properties to native details element

Can we add a basic example of how to use in the PR description? Additionally a preview link would help too.


PR comments use conventional comment style.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Are there any events we can trigger via JS on this component?

For example, <usa-banner> can be toggled via toggle() in console.

details-toggle-2024-07-05.at.13.17.58.webm

Comment on lines 10 to 20
item1Summary: {name: "Item 1 - Summary text"},
item1Content: { name: "Item 1 - Panel content"},
item1Open: { name: "Item 1 - Open panel on load"},
item2Show: { name: "Include item 2 in preview"},
item2Summary: {name: "Item 2 - Summary text", if: { arg: 'item2Show' } },
item2Content: { name: "Item 2 - Panel content", if: { arg: 'item2Show' } },
item2Open: { name: "Item 2 - Open panel on load", if: { arg: 'item2Show' } },
item3Show: { name: "Include item 3 in preview"},
item3Summary: {name: "Item 3 - Summary text", if: { arg: 'item3Show' } },
item3Content: { name: "Item 3 - Panel content", if: { arg: 'item3Show' } },
item3Open: { name: "Item 3 - Open panel on load", if: { arg: 'item3Show' } },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (if-minor): These are marked as defaults, but they're only used in two variants.

It'd be nice to cleanly separate this. Some suggestions:

  1. Separate default strings from default controls. For example, default args would be reserved for args that are seen on most/every variant.
  2. Adding ability to automatically add any number of accordions (depending on LOE).

Comment on lines 25 to 27
static properties = {
bordered: { type: Boolean }
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (non-blocking): Should open be listed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was only listing the props that should be added to <usa-details> here. In the current iteration, the open attribute should be added to the the child <details> element, so I left it off the properties list for now. Let me know if that should change!


template() {
const classes = {
"usa-details": true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: This should be on the element if it's not dynamic.

this.content.classList.add('usa-details__content');

return html`
<details class="${classMap(classes)}" open="${this.open || nothing}" name="${this.name || nothing}">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: In native disclosure, the open attribute doesn't have a value. For example something like open="false" will still keep the element open.

We should follow this pattern to keep things consistent.

The Details disclosure element - HTML: HyperText Markup Language | MDN

src/components/usa-details/index.js Outdated Show resolved Hide resolved
Comment on lines 14 to 18
* @cssprop --theme-details-font-family - Sets the font family for the details element
* @cssprop --theme-details-border-color - Sets the border width for the details element
* @cssprop --theme-details-border-width - Sets the border color for the details element
* @cssprop --theme-details-background-color - Sets the background color of the content panels
* @cssprop --theme-details-summary-background-color - Sets the background color of the summary element
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought: I wonder if there's a way to show these component settings in storybook args.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Curious why didn't we use usa-accordion SCSS here?

Comment on lines 10 to 20
item1Summary: {name: "Item 1 - Summary text"},
item1Content: { name: "Item 1 - Panel content"},
item1Open: { name: "Item 1 - Open panel on load"},
item2Show: { name: "Include item 2 in preview"},
item2Summary: {name: "Item 2 - Summary text", if: { arg: 'item2Show' } },
item2Content: { name: "Item 2 - Panel content", if: { arg: 'item2Show' } },
item2Open: { name: "Item 2 - Open panel on load", if: { arg: 'item2Show' } },
item3Show: { name: "Include item 3 in preview"},
item3Summary: {name: "Item 3 - Summary text", if: { arg: 'item3Show' } },
item3Content: { name: "Item 3 - Panel content", if: { arg: 'item3Show' } },
item3Open: { name: "Item 3 - Open panel on load", if: { arg: 'item3Show' } },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought: It'd be nice if we could do this in a more intuitive way that wasn't focused on a fixed number of elements.

Possible control ideas

  1. A button in controls that toggles fields for header/content.
  2. An array of objects for header/body content.
  3. A textarea to input any number of <details>.

.usa-details__summary {
/* TODO: use local files */
/* TODO: determine if background image is still the best way to add icons here */
background-image: url(https://designsystem.digital.gov/assets/img/usa-icons/add.svg);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (non-blocking): Is it possible to override this? How would I set my own icon?

this.content.classList.add('usa-details__content');

return html`
<details class="${classMap(classes)}" open="${this.open || nothing}" name="${this.name || nothing}">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<details class="${classMap(classes)}" open="${this.open || nothing}" name="${this.name || nothing}">
<details class="usa-details ${classMap(classes)}" open="${this.open || nothing}" name="${this.name || nothing}">

@amyleadem amyleadem marked this pull request as draft July 10, 2024 23:47
@amyleadem
Copy link
Contributor Author

Converting to draft while experiment with theming and adding usa-accordion component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USWDS-Next - Alpha: Create a details (accordion) component
2 participants