feat(accordion): spectrum 2 migration #3684

jawinn · 2025-04-23T20:37:26Z

Description

Accordion Spectrum 2 migration

Accordion now uses Spectrum 2 tokens and specifications. New sized tokens are used for corner rounding, the spacing around the disclosure icon, and the spacing around the content area.

A few notes on tokens: Medium top-to-text spacing tokens appear to be the wrong values (they appear to be the large values, not the values seen in the spec), I've followed up on this with design and assume that the correct values will be added later. Background color tokens also don't match the spec, I've followed up on this as well and have temporary colors that match the spec in place. Finally, so far we've noticed that the design spec appears to have a focus indicator gap but the token spec is 0 (no gap), so that is another follow-up item. The accordion-item-to-divider token is not used here per previous guidance from the designer.

New features

Adds the optional "quiet" style, which does not show dividers between accordion items.
Adds the "no inline padding" variant. This variant will be used within "standard panel".
Adds CSS transition to animate the rotation of the disclosure indicator when expanding and
collapsing the accordion item.
Per design spec, accordion items have a min-width and default width. Default width can be
overridden with --mod-accordion-item-width but should not be smaller than
--spectrum-accordion-item-minimum-width.

Markup changes

The HTML markup has changed slightly for the accordion header. The disclosure icon has been moved
from outside the the button (spectrum-Accordion-itemHeader), to within the button. The extra
element with class spectrum-Accordion-itemIconContainer, previously wrapping the icon, has
been removed. A span with class spectrum-Accordion-itemTitle has been added around the heading
text.

Mod changes

The following --mod custom properties have been renamed to better reflect how they are used:

--mod-accordion-item-height has been renamed to --mod-accordion-item-minimum-height
--mod-accordion-min-block-size has been renamed to --mod-accordion-item-min-block-size
--mod-accordion-component-edge-to-text has been renamed to --mod-accordion-content-padding-inline

New mods:

--mod-accordion-animation-duration
--mod-accordion-edge-to-content-area
--mod-accordion-item-minimum-width
--mod-accordion-top-to-disclosure-indicator

CSS-1018

Note: this does not include the new "direct actions" feature. That will be done in a follow-up PR/task.

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Open the storybook for the accordion component:
- Verify the boolean Quiet Style Variant has been added. When applied it should not show dividers between accordion items.
- Verify the No Inline Padding variant has been added . When applied it should remove inline padding from the accordion.
- Verify working CSS transition that animates the rotation of the disclosure indicator when expanding and
  collapsing the accordion item
- Verify all related documentation has been added
- Verify token additions align with token spec (see notes above in the description)

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

If components have been modified, VRTs have been run on this branch:

VRTs have been run and looked at.
Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
✨ This pull request is ready to merge. ✨

changeset-bot · 2025-04-23T20:37:29Z

🦋 Changeset detected

Latest commit: d85d936

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@spectrum-css/accordion	Major
@spectrum-css/bundle	Patch
@spectrum-css/preview	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2025-04-23T20:43:30Z

File metrics

Summary

Total size: 1.42 MB^*

Package	Size	Minified	Gzipped
accordion	19.97 KB	19.17 KB	2.45 KB

accordion

Filename	Head	Minified	Gzipped	Compared to base
index.css	19.97 KB	19.17 KB	2.45 KB	🔴 ⬆ 2.47 KB
metadata.json	10.87 KB	-	-	🔴 ⬆ 1.50 KB

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

github-actions · 2025-04-23T20:43:45Z

🚀 Deployed on https://pr-3684--spectrum-css.netlify.app

rise-erpelding

Nice work, I did some review on the documentation for a first pass! I will also take a look at the CSS and tokens but decided to put that in a separate review since those usually take a bit to validate and I already have quite a few comments in this review.

components/accordion/stories/accordion.stories.js

.storybook/types/variants.js

components/accordion/stories/accordion.stories.js

components/accordion/stories/accordion.test.js

components/menu/dist/metadata.json

.changeset/chatty-lands-attack.md

5t3ph · 2025-05-20T18:50:07Z

Looks like rounded corners should only be "for keyboard focus and quiet hover" but is currently always applying:

accordion now has min width, default width, and max width

matches spec for top-to-text spacing and bottom-to-text-spacing removes now-deprecated top-to-text-regular- tokens

change rgb syntax for background colors fix: use different colors for background The `background-opacity-hover`, `background-opacity-down`, and `background-opacity-key-focus` tokens' values did not match the design spec, design has been asked and the custom properties here have been updated to match the colors in the spec with the understanding that this may be temporary. Additionally, the previous rgb syntax did not work and comma syntax was required, if we need to use opacity tokens when the correct ones become available, we will likely need to use the comma syntax instead of the slash syntax that was used previously.

Adds a :not(.is-disabled) selector for hover/focus/active states to ensure that those styles do not apply for disabled accordion item headers. In many cases this is not necessary, but when a --mod is used for hover/ focus/active colors, the mod was still being applied even when the disabled class was present.

was previously being flagged as unused. the icon's size is also set by ui-icons, but this provides a fallback for sizing as well as a mod, --mod-accordion-disclosure-indicator-height

This reverts commit cdcba08. I had initially thought this was skipped over, but after reviewing some of the previous discussion on this via Slack, it seems like it is safe not to include it, so the commit is being reverted. In addition to reverting the commit, metadata has been updated.

…led" oops This reverts commit 3372bd9.

Instead of putting the --mod variables for colors where styles are defined, --mods are now used in the custom property definition. This aligns with a couple of the other color definitions, such as --spectrum-accordion-item-content-color and --spectrum-accordion-divider-color. This also makes it so that a mod used for the background, for example, is overridden by disabled styles instead of persisting through the disabled styles.

To keep the pattern of defining custom properties for tokens consistent, this defines a custom property for the accordion-focus-indicator-gap token. It has been renamed to accordion-item-focus-indicator-gap. To keep patterns consistent, other focus-indicator custom properties have been renamed to include "item" in the name. Metadata has been updated and changeset changes have been documented.

@hover

Accordion item's background color should change on active, but it wasn't visible when it was active and hovered. Active styles should take precedence over hover styles but the @hover media query prevents this. To fix this, the specificity of the active styles was increased, similar to #3767.

- clarify language around density options - other minor documentation edits for clarity, grammar, format, and consistent class notation (`.class-name`) - use sentence case where it wasn't used - add description to hasNoInlinePadding - add migrated tags/status - move custom width story to hang out with the other docs only stories - centers accordion - because default width tokens are applied, the padded layout no longer seems necessary and looks awkward left-aligned, but we can bring this back if there are unintended side effects

jawinn added the wip This is a work in progress, don't judge. label Apr 23, 2025

jawinn force-pushed the jawinn/css-1018-s2-accordion branch from ad07a57 to 7c4c408 Compare April 23, 2025 20:38

jawinn force-pushed the jawinn/css-1018-s2-accordion branch from 7c4c408 to 17ba3bc Compare April 29, 2025 17:05

castastrophe force-pushed the spectrum-two branch 3 times, most recently from 7e8269c to 4ce8d19 Compare May 5, 2025 21:18

jawinn force-pushed the jawinn/css-1018-s2-accordion branch from bbd7643 to 6a48b9f Compare May 7, 2025 20:20

jawinn added the S2 Spectrum 2 label May 8, 2025

jawinn force-pushed the jawinn/css-1018-s2-accordion branch from cb89f13 to 3eb63f2 Compare May 8, 2025 18:52

saashapina added size-5 L ~30-42hrs; lots of effort or complexity, most of a sprint needed to complete. ready-for-review and removed wip This is a work in progress, don't judge. labels May 15, 2025

saashapina marked this pull request as ready for review May 15, 2025 20:17

saashapina force-pushed the jawinn/css-1018-s2-accordion branch from 6f53ef1 to b935581 Compare May 15, 2025 20:22

saashapina added ready-for-review and removed ready-for-review labels May 15, 2025

This comment was marked as resolved.

Sign in to view

saashapina force-pushed the jawinn/css-1018-s2-accordion branch from 214cc10 to b935581 Compare May 15, 2025 22:07

saashapina self-assigned this May 19, 2025

rise-erpelding self-requested a review May 20, 2025 13:56

rise-erpelding previously requested changes May 20, 2025

View reviewed changes

saashapina marked this pull request as draft May 21, 2025 14:59

saashapina added wip This is a work in progress, don't judge. and removed ready-for-review labels May 21, 2025

saashapina force-pushed the jawinn/css-1018-s2-accordion branch from aef4d60 to ff25c8e Compare June 9, 2025 12:54

saashapina marked this pull request as ready for review June 9, 2025 12:56

saashapina and others added 26 commits June 18, 2025 06:10

feat(accordion): add no inline padding variant

f5ff709

chore: pr feedback

e3f5d4a

chore: add new accordion tokens

721b6ee

fix: adjust min-width and default width

5d1abb1

accordion now has min width, default width, and max width

fix: add comment about start spacing token

70bfd2f

feat: use accordion-item-to-divider spacing token, use mod

0812829

refactor: adjust top-to-text and bottom-to-text tokens

f049ed0

matches spec for top-to-text spacing and bottom-to-text-spacing removes now-deprecated top-to-text-regular- tokens

fix: change outline-offset to not be negative

a7eb6ef

fix: use correct -icon- token for top to disclosure compact large

c24755b

feat: quiet down state has border radius

7841fab

test: remove hover from disabled test case

10ced88

fix: remove disclosure icon space calc comment and move custom prop

ad5ca39

fix: use accordion-disclosure-indicator-height custom property

3129a5e

was previously being flagged as unused. the icon's size is also set by ui-icons, but this provides a fallback for sizing as well as a mod, --mod-accordion-disclosure-indicator-height

chore: update metadata

37e0a00

chore: update changeset

e9a5ab3

Revert "fix: remove application of hover/focus/active states if disab…

f07d7a2

…led" oops This reverts commit 3372bd9.

docs: document min-width and default-width

adf3a2b

chore: update metadata

b1f232e

fix: focus gap token and changeset updates

d85d936

rise-erpelding force-pushed the jawinn/css-1018-s2-accordion branch from 5954349 to d85d936 Compare June 18, 2025 16:10

rise-erpelding merged commit f0c6273 into spectrum-two Jun 18, 2025
12 checks passed

rise-erpelding deleted the jawinn/css-1018-s2-accordion branch June 18, 2025 16:54

github-actions bot mentioned this pull request Jun 18, 2025

chore: release (next) #3967

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(accordion): spectrum 2 migration #3684

feat(accordion): spectrum 2 migration #3684

Uh oh!

jawinn commented Apr 23, 2025 •

edited by rise-erpelding

Loading

Uh oh!

changeset-bot bot commented Apr 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 23, 2025 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

rise-erpelding left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

5t3ph commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

feat(accordion): spectrum 2 migration #3684

feat(accordion): spectrum 2 migration #3684

Uh oh!

Conversation

jawinn commented Apr 23, 2025 • edited by rise-erpelding Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Accordion Spectrum 2 migration

New features

Markup changes

Mod changes

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

Uh oh!

changeset-bot bot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

accordion

Uh oh!

github-actions bot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

5t3ph commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

jawinn commented Apr 23, 2025 •

edited by rise-erpelding

Loading

changeset-bot bot commented Apr 23, 2025 •

edited

Loading

github-actions bot commented Apr 23, 2025 •

edited

Loading

github-actions bot commented Apr 23, 2025 •

edited

Loading