fix(accordion): clean up spacing issues #4139

rise-erpelding · 2025-08-18T16:31:29Z

Description

Resolves the previous issue with accordion where, when the action button was added to an item's direct actions area, the action button was taller than the accordion item's height. (This was only an issue with compact XL.)

The updates made to accordion adjust the spacing tokens so that the action button always fits within the accordion item, although its focus outline may be outside of the accordion item (just as the accordion item's focus outline is outside of the item).

After confirming with design, the tokens used for minimum height have been updated to be more spacious, which better matches the designs. Note that these tokens aren't always used; there is a calculation that sets the min height to the maximum between the token or the sum of the space needed (top & bottom padding + font-size * line-height) to ensure that the accordion item always has enough space even with adjustments to padding, font-size, or line-height.

Since accordion items have more space, some adjustments were also made to center the accordion's itemTitle vertically.

Also addresses a small issue with the hover/key focus state where the itemTitle extends to the edge of the direct actions, this was more noticeable if there was a switch direct action, but not an action button:
Before:

After:

CSS-1260

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

Use the PR preview or check out the branch locally:

Check every combination of t-shirt size/density to confirm that minimum height looks as expected and items are centered/aligned vertically. (@cdransf)
Check hover state and key focus with switch direct action only to confirm that this looks as expected (@cdransf)
Check hover state and key focus with both direct actions (switch and action button) to confirm that this looks as expected (@cdransf)
Check hover state and key focus with no direct actions to confirm that this looks as expected (@cdransf)
If the line-height is adjusted (for instance if we switch to Japanese), the accordion item will make more space to accommodate that line height. Does this look ok? (It's occurred to me that this does throw the alignment of the disclosure icon off a little bit, are we ok with that for now or do we want to look more closely for a solution that will align the chevron and the text for this use case?) (@cdransf)
Are there any other items that might be helpful to address re: accordion while we're here? Do tests/documentation look good, for instance? (@cdransf)

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~~.

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. ✨

github-actions · 2025-08-18T16:36:45Z

📚 Branch preview

PR #4139 has been deployed to Azure Blob Storage: https://spectrumcss.z13.web.core.windows.net/pr-4139/index.html.

github-actions · 2025-08-18T16:43:47Z

File metrics

Summary

Total size: 1.43 MB^*

Package	Size	Minified	Gzipped
accordion	21.58 KB	20.71 KB	2.64 KB

accordion

Filename	Head	Minified	Gzipped	Compared to base
index.css	21.58 KB	20.71 KB	2.64 KB	🔴 ⬆ 0.17 KB
metadata.json	11.43 KB	-	-	🔴 ⬆ < 0.01 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.

changeset-bot · 2025-08-25T18:19:47Z

🦋 Changeset detected

Latest commit: 113ecd9

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	Minor
@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

rise-erpelding · 2025-08-27T16:51:30Z

components/accordion/index.css

@@ -13,7 +13,7 @@

 .spectrum-Accordion {
 	/* Layout and spacing */
-	--spectrum-accordion-item-minimum-height: var(--spectrum-component-height-100);
+	--spectrum-accordion-item-minimum-height: var(--spectrum-component-height-200);


These are going to be one size up all the way through! It goes something like this, and if you check out the design, you'll see that these min heights align with what's seen there too:

Size Compact Regular Spacious

S 24px/component-height-75 32px/component-height-100 40px/component-height-200

M 32px/component-height-100 40px/component-height-200 48px/component-height-300

L 40px/component-height-200 48px/component-height-300 56px/component-height-400

XL 48px/component-height-300 56px/component-height-400 64px/component-height-500

Can we put this table in the changelog?! It's awesome.

rise-erpelding · 2025-08-27T17:03:51Z

components/accordion/index.css

+	/* if we use min-height token, sometimes we'll have extra space so we'll need to center the text vertically */
+	display: flex;
+	align-items: center;
+
+	min-block-size: var(--mod-accordion-item-min-block-size, var(--spectrum-accordion-item-min-block-size));
+	box-sizing: border-box; /* respect min-block-size but include padding */


Here's an example to illustrate why we need these properties, if I take them out for compact XL for example, I get this:

Versus if I add them back in, here you can see that the chevron disclosure icon is much more aligned with the title text:

This is happening because when we adjusted the min-height tokens, we created more space in the accordion item. The vertical space that is taken up by the padding tokens, font-size & line height no longer fills the whole item. (This happens in the design too, I think the ticket for the issue has a screen shot of this.)

rise-erpelding · 2025-08-27T17:27:07Z

components/accordion/stories/accordion.test.js

-					}),
-				],
-			}),
+			content: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eleifend est mollis ligula lobortis, tempus ultricies sapien lacinia. Nulla ut turpis velit. Sed finibus dapibus diam et sollicitudin. Phasellus in ipsum nec ante elementum congue eget in leo. Morbi eleifend justo non rutrum venenatis. Fusce cursus et lectus eu facilisis. Ut laoreet felis in magna dignissim feugiat.",


When showing the Storybook preview to Kami, she noticed the discrepancies between the item content formatting that we had before, where some of the items use plain text styled according to the design spec, but some demonstrate use of the Typography component, which has different line spacing and a larger font size (even if we use the same t-shirt size). I see how that's confusing and inconsistent, so I've taken this out of the default story.

Since it is useful within the custom width story that has links within its content, the typography classes are still used there.

rise-erpelding · 2025-08-27T17:27:45Z

components/accordion/stories/accordion.test.js

@@ -80,9 +61,10 @@ export const longerContent = new Map([
 		"Are Adobe products worth it?",
 		{
 			content: Typography({
+				size: "s",


The typography font size doesn't line up with the spec'd font size for accordion item content, so we're using a smaller typography size here.

rise-erpelding · 2025-08-27T17:40:58Z

components/accordion/index.css

@@ -334,7 +334,7 @@

 	&:has(+ .spectrum-Accordion-itemDirectActions) {
 		/* set spacing between header and direct actions, whether or not noInlinePadding variant is used */
-		padding-inline-end: var(--mod-accordion-item-header-to-direct-actions-space, var(--spectrum-accordion-item-header-to-direct-actions-space));
+		margin-inline-end: var(--mod-accordion-item-header-to-direct-actions-space, var(--spectrum-accordion-item-header-to-direct-actions-space));


Just makes the space look a little prettier when there's hover/key focus!

cdransf · 2025-08-27T21:25:53Z

.changeset/pre.json

@@ -142,6 +142,7 @@
    "itchy-shrimps-help",
    "itchy-waves-work",
    "khaki-icons-hammer",
+    "kind-cycles-check",


Should this be included as part of the changes for this PR? ✨

Looks like another rebase resolved these!

cdransf · 2025-08-27T21:26:22Z

.storybook/CHANGELOG.md

@@ -1,5 +1,14 @@
 # Change Log

+## 12.1.0-next.15


This as well — it looks like it pertains to one of @castastrophe's recent changes? ✨

Looks like another rebase resolved these!

cdransf

Ran through all the validation steps and everything looks great to me! ✨ 👏🏻

(Left a few comments about commit content, but nothing blocking.)

castastrophe

Approved! Thanks for the detailed notes inline. My only feedback is that I'd love to see that table you shared embedded in the changeset you wrote out. Thanks again!

castastrophe · 2025-08-28T15:08:39Z

components/accordion/index.css

@@ -13,7 +13,7 @@

 .spectrum-Accordion {
 	/* Layout and spacing */
-	--spectrum-accordion-item-minimum-height: var(--spectrum-component-height-100);
+	--spectrum-accordion-item-minimum-height: var(--spectrum-component-height-200);


Can we put this table in the changelog?! It's awesome.

… more accurate

rise-erpelding self-assigned this Aug 18, 2025

rise-erpelding added size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. ask design Questions or topics for the design team wip This is a work in progress, don't judge. S2 Spectrum 2 labels Aug 18, 2025

rise-erpelding changed the base branch from main to spectrum-two August 18, 2025 16:31

rise-erpelding force-pushed the rise-erpelding/accordion-density-min-heights branch from ec06316 to c9ef6f4 Compare August 25, 2025 18:19

rise-erpelding force-pushed the rise-erpelding/accordion-density-min-heights branch from c9ef6f4 to 782a1aa Compare August 25, 2025 22:42

rise-erpelding removed the ask design Questions or topics for the design team label Aug 26, 2025

rise-erpelding force-pushed the rise-erpelding/accordion-density-min-heights branch 2 times, most recently from 36de749 to b349d2c Compare August 27, 2025 16:38

rise-erpelding commented Aug 27, 2025

View reviewed changes

rise-erpelding added ready-for-review and removed wip This is a work in progress, don't judge. labels Aug 27, 2025

rise-erpelding marked this pull request as ready for review August 27, 2025 18:32

rise-erpelding added the run_vrt For use on PRs looking to kick off VRT label Aug 27, 2025

castastrophe force-pushed the spectrum-two branch from 58ea3eb to 5bf2d51 Compare August 27, 2025 19:27

cdransf reviewed Aug 27, 2025

View reviewed changes

cdransf approved these changes Aug 27, 2025

View reviewed changes

rise-erpelding force-pushed the rise-erpelding/accordion-density-min-heights branch from b349d2c to 56aeea1 Compare August 28, 2025 13:44

castastrophe approved these changes Aug 28, 2025

View reviewed changes

rise-erpelding removed the ready-for-review label Aug 28, 2025

rise-erpelding added the ready-to-merge label Aug 28, 2025

rise-erpelding added 7 commits August 28, 2025 12:21

docs(accordion): adjust chromatic params

955345e

fix(accordion): add taller component height tokens

6f67550

fix(accordion): vertical centering with padding

4b37fda

fix(accordion): use margin instead of padding to make the hover state…

bd3ef8e

… more accurate

docs(accordion): move typography elements to docs only

fa13276

chore(accordion): changeset

c2f4f23

chore(accordion): add table to changeset

113ecd9

rise-erpelding force-pushed the rise-erpelding/accordion-density-min-heights branch from 6e1d8b5 to 113ecd9 Compare August 28, 2025 17:21

rise-erpelding merged commit 6d835ff into spectrum-two Aug 28, 2025
15 checks passed

rise-erpelding deleted the rise-erpelding/accordion-density-min-heights branch August 28, 2025 17:42

github-actions bot mentioned this pull request Aug 28, 2025

chore: release (next) #4177

Open

Size	Compact	Regular	Spacious
S	24px/`component-height-75`	32px/`component-height-100`	40px/`component-height-200`
M	32px/`component-height-100`	40px/`component-height-200`	48px/`component-height-300`
L	40px/`component-height-200`	48px/`component-height-300`	56px/`component-height-400`
XL	48px/`component-height-300`	56px/`component-height-400`	64px/`component-height-500`

fix(accordion): clean up spacing issues #4139

fix(accordion): clean up spacing issues #4139

Conversation

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

Uh oh!

Description

How and where has this been tested?

Validation steps

Regression testing

To-do list

Uh oh!

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

Uh oh!

📚 Branch preview

Uh oh!

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

Uh oh!

File metrics

Summary

accordion

Uh oh!

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

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdransf left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

castastrophe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rise-erpelding commented Aug 18, 2025 •

edited by cdransf

Loading

github-actions bot commented Aug 18, 2025 •

edited

Loading

github-actions bot commented Aug 18, 2025 •

edited

Loading

changeset-bot bot commented Aug 25, 2025 •

edited

Loading

cdransf left a comment •

edited

Loading