Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Ensure drawer opens at the specified active snap point on initial render #473

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

KhalafAbdi
Copy link

@KhalafAbdi KhalafAbdi commented Oct 2, 2024

Issue: Drawer Initializes at First Snap Point Regardless of activeSnapPoint Setting

When setting activeSnapPoint to any value other than the first element in the snapPoints array, the drawer component always initializes at the first snap point on the initial render. It only updates to the correct activeSnapPoint after an external re-render occurs. This behavior is due to how the initial style is assigned and how snapPointsOffset is calculated when the component first mounts.

This can be recreated by running the repo localy, and try to use /initial-snap test route. This does not work of the bat (it will open drawer with offset for the first element in the snapPoints array), but opening initial-snap/page.tsx, and saving any change, will force a re-render which will then open the drawer at the correct snap point.

Current Behavior:

The DialogPrimitive.Content component assigns the CSS custom property --snap-point-height to snapPointsOffset[0], which corresponds to the first snap point.

vaul/src/index.tsx

Lines 885 to 892 in 1142f66

style={
snapPointsOffset && snapPointsOffset.length > 0
? ({
'--snap-point-height': `${snapPointsOffset[0]!}px`,
...style,
} as React.CSSProperties)
: style
}

Proposed Change in PR:

In this PR, I changed the initialization of --snap-point-height to use activeSnapPointIndex instead of always defaulting to the first element. This ensures that the drawer starts at the correct snap point specified by activeSnapPoint without requiring a re-render.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant