fix(F0Wizard): make wizard dialog full height to fix broken scroll#4737
Closed
martin1740 wants to merge 8 commits into
Closed
fix(F0Wizard): make wizard dialog full height to fix broken scroll#4737martin1740 wants to merge 8 commits into
martin1740 wants to merge 8 commits into
Conversation
F0Wizard hardcoded its step-content column to h-[58vh], nested inside F0Dialog's own ScrollArea-managed content region. Since F0Dialog never had a fixed height (only max-h-full), the two independently-managed scroll containers could clip content instead of scrolling it once a step had enough fields to exceed 58vh (e.g. the it_management new-device form). Thread the existing (but previously unwired) fullHeight prop from DialogWrapper up through F0Dialog's public props, and have F0Wizard opt into it so the dialog gets a real fixed height and its step content fills it via h-full instead of an arbitrary viewport fraction.
Contributor
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
Contributor
📦 Alpha Package Version PublishedUse Use |
Contributor
🔍 Visual review for your branch is published 🔍Here are the links to: |
Contributor
|
Contributor
Coverage Report for packages/react
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
martin1740
marked this pull request as ready for review
July 20, 2026 12:13
Add tests for both the symptom (F0Wizard step content sizes to h-full instead of a fixed viewport fraction) and the mechanism (fullHeight now threads through DialogWrapper to DialogContent), so a future regression in either spot is caught. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d of clipping Content's flex-1 wrapper had no min-h-0, so under the default flex min-height:auto it never shrank below its content size. With fullHeight set (F0Wizard), the parent's fixed height plus overflow-hidden meant content taller than the dialog got clipped instead of scrolled, since the ScrollArea never actually got constrained. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Simulate scrollHeight/clientHeight (jsdom does no real layout) and assert the bottom scroll-shadow only disappears once the viewport has actually reached the end of the content, so the fix is verified at a behavioral level and not just via className assertions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lly gets a real height min-h-0 alone wasn't enough: Radix's ScrollArea wraps children in an auto-height div, so a fullHeight consumer's own h-full (e.g. F0Wizard's step row) had no definite ancestor height to resolve against and never actually overflowed — nothing to scroll, so the outer dialog just clipped it instead. Content only reacted to position === "fullscreen" to force that wrapper's height; fullHeight (position="center") never reached it since DialogWrapperProvider didn't carry it. Thread fullHeight through the context so Content applies the same h-full wrapper fix whenever the dialog is fullHeight, not just when it's a fullscreen drawer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
F0Wizardrendered its step content in a hardcodedh-[58vh] overflow-y-autocolumn, nested insideF0Dialog's ownScrollArea-managed content region — which never had a fixed height (onlymax-h-fullunlessfullHeightis passed).it_management"new device" wizard, ~13 fields on its first step) got visually clipped instead of scrolling — users reported being unable to scroll down.fullHeightprop fromDialogWrapperup throughF0Dialog's public props (DialogAlikeInternalProps→DialogInternal), and hasF0Wizardopt into it so the dialog gets a real, fixed height. The step content now fills it viah-fullinstead of an arbitrary58vh.fullHeightdefaults tofalse, so this is additive and doesn't change any otherF0Dialogconsumer.F0Draweralready relies on the sameDialogWrapper.fullHeightmechanism, so this follows an established pattern.Test plan
pnpm tsconpackages/react— no new errors introduced (pre-existing unrelated errors only, none in touched files)pnpm lint(oxlint) on touched files — 0 warnings/errorsvitest run src/ui/F0Wizard src/components/dialog-alike— 100/100 tests passing, no regressionsit_managementnew-device wizard) that the step content scrolls correctly with many fields🤖 Generated with Claude Code