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

Opening a drawer scrolls to the top #55

Closed
jackvane opened this issue Aug 19, 2023 · 18 comments
Closed

Opening a drawer scrolls to the top #55

jackvane opened this issue Aug 19, 2023 · 18 comments

Comments

@jackvane
Copy link

I have a button that opens up a drawer near the bottom of a page.

Whenever I open the drawer, it will automatically scroll to the top of the page, which is unwanted behavior. Can you make this opt in?

The code I used is the Without scaled background codesandbox example and I'm using Next JS.

@emilkowalski
Copy link
Owner

@jackvane Which version of Vaul are you using? This should be fixed in the latest one.

@jackvane
Copy link
Author

@emilkowalski thanks for the quick response. This is the version I'm using:

    "vaul": "^0.3.4",

@emilkowalski
Copy link
Owner

@jackvane Could you make a demo with a reproduction on codesandbox?

@emilkowalski
Copy link
Owner

Also, which browser and OS are you using?

@jackvane
Copy link
Author

Hmm, I couldn't seem to reproduce it in codesandbox

But I can confirm to you that it is behaving as I'm seeing it in my project.

Screen.Recording.2023-08-19.at.19.22.40.mov

I'm on MacOS Ventura 13.5 and the behavior was the same on both Google Chrome and Brave.

The code is the exact same as from the example except that I'm passing an avatar component as a child to the trigger.

@emilkowalski
Copy link
Owner

There is nothing in the code that could be causing this issue. Are you sure this isn't something in your code? If that's not the case, please provide a reproducible example on codesandbox, as I can't reproduce this behavior myself.

@emilkowalski
Copy link
Owner

I fixed a related bug in #57, please re-open an issue with a reproduction if it persists. Thanks for reporting.

@jackvane
Copy link
Author

@emilkowalski

I'll keep probing to see if I can find what the problem could be, the latest version did not fix the issue.

Just a heads up, your latest version has a stray console log.

      console.log(window.scrollY, 'coming from onScroll');

@emilkowalski
Copy link
Owner

@jackvane Just seeing it now, let me know if you find out what the problem is.

@raimondlume
Copy link

raimondlume commented Sep 12, 2023

@emilkowalski
Ran into the same issue in our application as well.
Attempted a reproduction in codesandbox, but also without luck :/

It scrolls to the top if drawer opened and snaps back with a delay after it's closed, but not always.

Screen.Recording.2023-09-12.at.20.55.07.mov

A hint to what this could be is that removing Overlay seems to fix the issue:

Screen.Recording.2023-09-12.at.21.00.41.mov

We're using Radix's vanilla Dialog as well for desktop in the exact same pattern, the scroll issue isn't happening there, even when using an overlay.

EDIT: this is with the latest 0.6.0 version and on Chrome (both desktop and mobile)

@emilkowalski emilkowalski reopened this Sep 12, 2023
@emilkowalski
Copy link
Owner

Can you show me how your body and html styles look like @raimondlume?

@raimondlume
Copy link

raimondlume commented Sep 13, 2023

@emilkowalski
h-full on both.

And I got it reproduced as well https://codesandbox.io/p/sandbox/drawer-without-scale-forked-sjmrm7 🎉

Sidenote - this is recommended in TailwindUI's layouts

image

@emilkowalski
Copy link
Owner

emilkowalski commented Sep 13, 2023

Is it fixed on this preview url for you? https://vaul-git-fix-position-fixed-with-height-100-emilkowalski-s-team.vercel.app/

@raimondlume
Copy link

raimondlume commented Sep 13, 2023

Is it fixed on this preview url for you? https://vaul-git-fix-position-fixed-with-height-100-emilkowalski-s-team.vercel.app/

Seems that something's funky with the overflow there, can't seem to scroll at all

@emilkowalski
Copy link
Owner

Forgot to update body styles, can you try now? @raimondlume

@raimondlume
Copy link

raimondlume commented Sep 13, 2023

Seems to be resolved now 🙌
@emilkowalski

@emilkowalski
Copy link
Owner

Fixed in #98

@raimondlume
Copy link

Thanks, you're amazing @emilkowalski!

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

No branches or pull requests

3 participants