Page jumps to top when opening the drawer

OrenMag commented 2 months ago

Loom:

https://www.loom.com/share/c146667a27774baebbcebeeb6985313f

Seems like the top of body is set to 0 upon opening (as well as right, left and position fixed)

vaul version: 0.9.0

Couldn't create reproducible sandbox

LouisGS44 commented 2 months ago

Same here, version 0.9.0

maiconcarraro commented 2 months ago

I believe the problem can be related to this line:

https://github.com/emilkowalski/vaul/blob/a30da7d1618afd8b33637982695f0676130346ff/src/use-prevent-scroll.ts#L260

I'm only facing this issue for Iphone devices, not Androids

Orestli commented 2 months ago

Any updates?

OrenMag commented 2 months ago

Is this planned to be fixed?

michael-convertdigital commented 1 month ago

i'm experiencing the same issue in remix, it seems to happen randomly, maybe every 3rd or 4th close of the drawer

joaom00 commented 1 month ago

Can you create a reproducible example?

emilkowalski commented 1 month ago

Couldn't create reproducible sandbox

This proves that this is a tough issue to fix. I'd love to fix it, but without a consistent reproduction I could only guess as to what is causing the problem. Removing window.scrollTo(0, 0) might help potentially, but it'll break other stuff.

maiconcarraro commented 1 month ago

This proves that this is a tough issue to fix. I'd love to fix it, but without a consistent reproduction I could only guess as to what is causing the problem. Removing window.scrollTo(0, 0) might help potentially, but it'll break other stuff.

Hi @emilkowalski , thank you for replying to this issue, at least for my situation it only happens for iOS devices + combining two drawers, the issue happens in a consistent way to me, here's the link for sandbox: https://codesandbox.io/p/devbox/shadcn-playground-forked-gfm4rj

It only happens when testing through iOS devices, it won't work emulating Chrome DevTools w/ iOS user-agent.

Here's a video showing it:

https://github.com/emilkowalski/vaul/assets/13419087/ef19991d-7362-4e32-9d2a-59130b43bc2c

It could be a race condition with the restoring position fixed + opening the second, let me know if you prefer that I open a new issue because it's a little bit different from the original comment.

I don't have the same issue when using radix-dialog alone (no vaul), or when using android devices.

levitatejay commented 1 month ago

It only happens when testing through iOS devices, it won't work emulating Chrome DevTools w/ iOS user-agent.

Your repro is reliable for me, I toggle Chrome dev tools and the iPhone view setting and I'm unable to open the drawer

Do you know a workaround? I've had to disable the drawer because it won't work with window virtual scrolling on mobile

levitatejay commented 1 month ago

I fixed this in my project by upgrading to v0.9.1 and adding noBodyStyles and preventScrollRestoration={false}

joshuafranks commented 1 month ago

Also having this issue on iOS devices when opening a second drawer. It seems to have something to do with <body> styles being applied incorrectly. I can't talk with too much authority as I haven't dug into the source code, but anecdotally, drawers that are not affected apply these styles to <body>:

right: 0px; position: fixed !important; top: -424px; left: 0px; height: auto; pointer-events: none;

But affected drawers apply these styles to <body> instead:

right: unset; pointer-events: none;

If you go into web inspector and add those styles manually, the drawer will return to the expected position.

Validated this through the iOS 17.4 simulator on Sonoma 14.3. Safari doesn't seem to like it whereas other browsers handle the behaviour without any observable issues.

EDIT: to be clear, the scenario is:

Drawer A is opened (in my case, using the open prop and controlling state through a Context)
Drawer A has right: 0px; position: fixed !important; top: -424px; left: 0px; height: auto; pointer-events: none; set on <body>
Drawer A is closed and Drawer B is opened immediately after via a button click in Drawer A
Drawer B has right: unset; pointer-events: none; set on <body>

This definitely feels like a race condition – presumably in the setPositionFixed callback logic – because if I wrap the state change in a setTimeout callback with a 5000ms delay, it opens without any issues. I'm on holiday at the moment so I don't have time to clone the repo and debug locally, but a cursory scan of the codebase suggests that the callback may be the culprit.

levitatejay commented 4 weeks ago

I fixed this in my project by upgrading to v0.9.1 and adding noBodyStyles and preventScrollRestoration={false}

Turns out it was only fixed in the Chrome mobile dev tools and the issue still occurs in iOS Safari.

I'm using the drawer with the TanStack virtual scroller using the window as the scroller. With noBodyStyles and preventScrollRestoration={false} it jumps to the top of the page when the drawer is open, and while there is a touch interaction with the drawer it temporarily resets to the correct page position

michael-convertdigital commented 4 weeks ago

after upgrading to v0.9.1 in remix i now get hydration errors

Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's
output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this,
useLayoutEffect should only be used in components that render exclusively on the client. See
https://reactjs.org/link/uselayouteffect-ssr for common fixes.
     at Root (/Users/convert/Documents/projects/cue/packages/ui/node_modules/vaul/dist/index.mjs:749:23)
     at Drawer2 (/Users/convert/Documents/projects/cue/packages/ui/@/components/ui/drawer.tsx:6:19)
     at /Users/convert/Documents/projects/cue/packages/ui/src/components/Slideout/Slideout.tsx:15:5

emilkowalski / vaul

Page jumps to top when opening the drawer #318