Issues when using drawer with autofocus input on iOS

[ze-kel]

I was trying to use drawer like this:

1. user clicks an "edit" button
2. drawer opens with a single input that has autofocus. keyboard opens simultaneously
3. user clicks away or clicks done. input calls onBlur, drawer closes

While trying to do this I encountered problems

1. Animations are janky while opening or closing a keyboard on iOS
2. Auto focus is broken on ios when using controlled drawer(open + onOpenChange)
3. There is a noticeable delay when closing the drawer in this scenario

For #1 I've added a prop to disable usePreventScroll.

We can theoretically try to stagger both in animation and keyboard closing or try to implement some sort of auto detection for "keyboard and drawer open at the same time" case, but I'm not sure it's worth it.

Also I am aware that setting modal=false will also disable usePreventScroll, however I do need focus trapping inside component that radix does when modal is true.

---

2 and happens due to extra renders when the following happens:

1. User opens the modal
2. onOpenChange(true) fires
3. Drawer renders with openProp=true and isOpen=false
4. useEffect works and sets isOpen to true
5. Drawer renders with openProp=true and isOpen=true and is now opened

A far as I understand this extra render makes iOS think that input with autofocus appeared not as a result of a user action and therefore it ignores autofocus.

This can be fixed simply by making internal value changes simultaneously with onOpenChange. Update: after looking at tests and thinking some more I realised that my approach was wrong. We can't just assume that onOpenChange will change the value to whatever it's called with. Maybe someone has a form in a drawer that prevents user from closing it until all fields are validated or something like that. A better way to solve this is by using useLayoutEffect. It's the same hook except when fired it will fire before rendering, set internal prop and the render correctly.

---

Both of these also fixed #3 and made it way smoother.

Videos for reference before and after.

https://github.com/emilkowalski/vaul/assets/63189390/41ed0b45-949a-4826-944e-1c89f63528d4

^notice that keyboard closes first and then after some delay the drawer also disappears

https://github.com/emilkowalski/vaul/assets/63189390/f71357b6-ee38-41c1-af33-20621ba83d75

[vercel[bot]]

@ze-kel is attempting to deploy a commit to the emil Team on Vercel.

A member of the Team first needs to authorize it.

[emilkowalski]

This is amazing, can we document this prop in the readme and ensure the tests pass?

[ze-kel]

Added prop to readme. Found a better way of solving second issue, updated head post

~This can be fixed simply by making internal value changes simultaneously with onOpenChange.~ Update: after looking at tests and thinking some more I realised that my approach was wrong. We can't just assume that onOpenChange will change the value to whatever it's called with. Maybe someone has a form in a drawer that prevents user from closing it until all fields are validated or something like that. A better way to solve this is by using useLayoutEffect. It's the same hook except when fired it will fire before rendering, set internal prop and the render correctly.

This also fixed all tests, except for one: 1) [iPhone] › tests/base.spec.ts:53:7 › Base tests › should not close when dragged up

However this one fails for me even on main branch. Not sure why, maybe that will pass on CI env.

[ze-kel]

Looked into broken test. It was caused by using event.screenY(screenX) instead of event.clientY(clientX). I replaced it in all functions. The difference is basically that screenY accounts for zoom level. I guess playwright was running tests with browser zoom for some reason.

This also causes incorrect distance estimation when browser zoom is not 100%. Here's before and after:

https://github.com/emilkowalski/vaul/assets/63189390/8b5715ad-cf0f-4032-8e9c-4cf7dde032f8

Now all tests pass locally on my machine, even the commented one('should close when dragged down') which I uncommented,

[Fr0stmourne]

Our team has come across first issue as well, for now we're using patch-package fix to disable usePreventScroll. Would be great if this one was merged

[ReangeloJ]

@emilkowalski It hasn't been merged in main yet.

[ant1m4tt3r]

@Fr0stmourne did you patched the minified published version? i'm having some trouble doing so while this commit isn't published.

[ant1m4tt3r]

it was published a few hours ago, thank you!