Accordion: animate the first time a section is opened - #3190
Open
jonahgoldsaito wants to merge 2 commits into
Open
Accordion: animate the first time a section is opened#3190jonahgoldsaito wants to merge 2 commits into
jonahgoldsaito wants to merge 2 commits into
Conversation
AccordionSection with `animated` did not animate its expand the first time a section was opened; it snapped. Every toggle after that animated correctly. Aphrodite merges each style list into a single generated class and injects that class's rule lazily. Swapping between `wrapperCollapsed` and `wrapperExpanded` therefore pointed the first expand at a rule that had not been injected yet, so `grid-template-rows` computed to `none` -- which cannot interpolate -- and the section snapped open. Keep one stable class on the wrapper instead, and select the collapsed size off a `data-expanded` attribute. Both rules are injected together on first render, so no toggle ever encounters a missing rule. Verified in the browser: measured `grid-template-rows` frame by frame across the first open for all three cornerKinds, which now ramps over ~300ms instead of jumping in a single frame. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 6047f84 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (2fb48c9) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR3190"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR3190 |
Contributor
|
Size Change: +18 B (+0.01%) Total Size: 132 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-bhhyxxeorw.chromatic.com/ Chromatic results:
|
The changeset blamed a changed class name, but Wonder Blocks' Jest inlines Aphrodite styles, so the wrapper renders class="" and no class name appears in snapshots at all. The consequence worth warning about is the one on the inlining path: nested selectors are not applied there, so a collapsed wrapper reports grid-template-rows: min-content 1fr and reads as expanded. Also record, next to the selector, that the collapsed value outranks a consumer `style` override of the same property. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jonahgoldsaito
marked this pull request as ready for review
August 28, 2026 20:40
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3190 +/- ##
============================
============================
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
jandrade
approved these changes
Aug 28, 2026
jandrade
left a comment
jandrade
left a comment
Member
There was a problem hiding this comment.
Looks great! and it is in line with the ongoing CSS modules migration. Thanks for your contribution! hoping to get more of this 👏 🚢
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
smooth-first-open.mp4
Summary
The
AccordionSectioncomponent doesn't animate the first time it's opened was opened — it snapped open . Every toggle after that animates smoothly as intendedRoot cause
Aphrodite merges each style list into a single generated class and injects that class's rule lazily. Swapping between
wrapperCollapsedandwrapperExpandedpointed the first expand at a rule that hadn't been injected yet, sogrid-template-rowscomputed tonone— which cannot interpolate — and the section snapped open.The caret icons rotation works because but
none → rotate(180deg)is interpolable.Fix
Now there's one stable class on the wrapper. Collapsing keys off the
data-expandedattribute.Both rules are injected together on first render, so no toggle ever meets a missing rule.
Precedent: attribute selectors in
StyleSheet.createare established WB practice (action-styles.ts,cell-core.tsx,tab.tsx).Verification
Claude took screengrabs and measured
grid-template-rowsframe by frame in the browser across the first open:0 → 52pxin one frame0 → 0.65 → 3.9 → 11.3 → … → 52pxover ~300msConfirmed the diagnosis before fixing: in a
rounded-per-sectionaccordion all sections share one merged class, so opening section 1 (snap) injected the rule that let section 2's first open animate. Same component, same props — which ruled out a layout-measurement cause.Checked for all three
cornerKinds, the collapse direction,animated={false}(still instant), mount-expanded (no transition on mount), andcollapsible={false}. Collapsed content staysvisibility: hidden; zero frames where the panel'soverflowlefthidden.Jest (4/4 accordion suites, existing transition tests unchanged), typecheck, and lint all pass.
Reviewer notes
frontendblast radius is zero. There are 40 render sites, but 0 snapshots render an accordion, 0 style-based assertions, 0data-expandedcollisions, 0 Cypress selectors. Every accordion test there asserts viaaria-expanded. Yay!:not([data-expanded="true"]), which is more specific than the base class. A consumer overridinggridTemplateRowsviastylewould still win when expanded but be ignored when collapsed.