fix(design-system): restore max-height strategy for MaxHeightSmoother when minHeight > 0#479
Open
IgnazioDS wants to merge 1 commit into
Open
Conversation
… when minHeight > 0 Fixes the two-step / delayed expand described in aymericzip#294. The Strategy A/B implementation (grid-template-rows for minHeight === 0, max-height via --mhs-collapsed/--mhs-expanded CSS variables for minHeight > 0) landed in 19bb8a2 but was unintentionally reverted by the large editor rework in b2d302d, which restored the old grid-rows-only body while keeping the JSDoc and the Storybook play assertions that describe Strategy B. Since then the component and its own stories have disagreed: the stories assert --mhs-* variables and max-h-[var(...)] classes the component no longer rendered, and the minHeight dead-time bug returned to every consumer that sets a preview floor (ExpandCollapse, CommonQuestions). This restores the documented implementation, credit to @TheoStracke for the original fix. Consumers audited: all isHidden-only call sites (Accordion, CollapsibleTable, DropDown, FileTree, MobileNavbar, dashboard forms) stay on Strategy A, which remains a grid so DropDown's group-hover:grid-rows overrides keep working; Accordion's role/id overrides still win through the props spread.
|
The preview deployment for website failed. 🔴 Open Build Logs | Open Application Logs Last updated at: 2026-07-12 22:53:51 CET |
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
1 participant
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.
Fixes #294
What happened
The two-step expand is a regression, not a missing feature. The max-height strategy for
minHeight > 0landed in 19bb8a2 (credit @TheoStracke), but the editor rework in b2d302d restored the previous grid-rows-only component body while keeping the new JSDoc and Storybook stories. Since then the component has disagreed with its own documentation and tests: the stories assert--mhs-collapsed/--mhs-expandedCSS variables andmax-h-[var(...)]classes that the component no longer renders, and the dead-time bug returned to every consumer that sets a preview floor (ExpandCollapse,CommonQuestions).What this PR does
Restores the documented two-strategy implementation:
minHeight === 0):grid-template-rowstransition, unchanged in spirit;min-h-0on the inner wrapper so the track can fully collapse.minHeight > 0): animatesmax-heightbetween--mhs-collapsed(the minHeight floor) and--mhs-expanded, so the expansion starts from the already-visible floor in one continuous motion instead of the silent0fr -> minHeightphase followed by the visible jump.No changes to the JSDoc or the stories - they already describe exactly this behavior.
Verification
maxheightsmoother.stories.tsxpasses against the restored component (Strategy A classes, Strategy B CSS variables and classes,role/tabIndex/aria-expanded).isHidden-only call sites (Accordion,CollapsibleTable,DropDown,FileTree,MobileNavbar, the dashboard form sections) stay on Strategy A, which remains a grid, soDropDown'sgroup-hover/dropdown:grid-rows-[1fr]overrides keep working.Accordion'srole="region"/id/aria-labelledbystill win through the props spread.One thing worth your eyes on review: Strategy A now clips overflow on the inner wrapper rather than the container (as in 19bb8a2), so a consumer that overrides overflow on the container via
className(DropDown passesoverflow-x-visible) no longer changes the clipping of flowing content. Dropdown panels are absolutely positioned against the container, so they are not clipped by the inner wrapper - but if you prefer the container-level overflow of the current main, that is a two-line adjustment.