chore(accordion): full fidelity feature update#6319
Conversation
Co-authored-by: Nikki Massaro <massaro@adobe.com>
|
📚 Branch Preview Links🔍 First Generation Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…0-full-fidelity Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c6341e3 to
e63c363
Compare
rise-erpelding
left a comment
There was a problem hiding this comment.
This is looking really nice overall and could use some polish in a few places! My big takeaway right now is that I realize that the previous PR definitely should have set selectors up a bit better--I'll address that ASAP
| gap: token("accordion-disclosure-indicator-to-text-medium"); | ||
| align-items: center; | ||
| inline-size: 100%; | ||
| padding-block: var(--_swc-accordion-pad-top-m, token("accordion-top-to-text-medium")) var(--_swc-accordion-pad-bottom-m, token("accordion-bottom-to-text-medium")); |
There was a problem hiding this comment.
This isn't very well documented in the spec for accordion as far as I've seen, but my understanding is that block padding uses different tokens than the token spec or Spectrum-CSS because we changed our font to Adobe Clean VF - maybe the styling skill missed it?
So we'll want to change these for all densities/sizes, it does look like the design data repo recommends a new value, so for instance this one would use token("base-padding-vertical-large") for both top and bottom instead, might be worth pointing Claude to the repo to dig up all the correct replacement values.
There was a problem hiding this comment.
head's up that we don't yet have the latest version of the tokens data so ones like base- aren't there yet
There was a problem hiding this comment.
Oop now I remember you mentioning that before! What's the best way to handle that, would it be better to hard-code the spacing values for now rather than using separate top/bottom tokens?
There was a problem hiding this comment.
I think you can still switch to something like component-padding-vertical-100 if those match up? That's what we're using for now in like button and badge
There was a problem hiding this comment.
They generally match up, but spacious large and extra-large hit the ceiling. component-padding-vertical-300 (13px) is the highest available scale and the design value goes to 16px. I could use calc(token("component-padding-vertical-300") + 3px) as a stopgap with a comment to replace it once a higher-scale token lands in the token set?
There was a problem hiding this comment.
Hmm, do we have a spacing token that is a better match? Since the idea is that those are used for uniform padding, at least it would be tokenized. You could add an inline comment though to explain the need to switch token type.
There was a problem hiding this comment.
It seems both options have trade offs:
- calc(token("component-padding-vertical-300") + 3px) - partially theme-aware (the base scales, but the +3px is fixed)
- spacing-300 - fully fixed, no theme-responsiveness at all
Whats the process for getting new tokens added? I wonder if that's something we want to entertain.
There was a problem hiding this comment.
See Josh's Slack thread re: platform scale but essentially the theme responsiveness is less of a concern going forward.
5t3ph
left a comment
There was a problem hiding this comment.
A few areas for improvements!
I would suggest pointing your agent at these custom property contributor docs (local path) to try to help sort out private vs public properties. We can huddle on that too, if needed!
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…to swc-1860-full-fidelity

Description
Completes full-fidelity rendering, styling, and Storybook documentation for
<swc-accordion>and<swc-accordion-item>.Changes:
accordion-item.css— new file with all Spectrum 2 token-driven item styles: header layout, chevron, panel, sizes (s/m/l/xl), and forced-colors supportaccordion.css— density overrides (compact/spacious) andquietvariant (transparent dividers, rounded header corners) cascaded to child items via custom propertiesAccordionItem.ts— fix incorrect CSS import (accordion.css→accordion-item.css)accordion.stories.ts— Storybook stories: Anatomy, Options (sizes, density, quiet, allow-multiple, heading level), States (disabled, item-disabled), Behaviors (toggle event, exclusive open), and Accessibility.Note: Storybook stories are for testing for full fidelity review. Storybook docs will be further refined in following tickets.
Motivation and context
Phase 5 (styling) of the accordion 2nd-gen migration. This PR adds the visual layer and Storybook coverage on top.
Related issue(s)
Screenshots (if appropriate)
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases