react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

iOS keyboard covers inputs when using useDialog/useModalOverlay

Open JRS-Developer opened this issue 1 year ago โ€ข 15 comments
trafficstars

Additional information: https://github.com/adobe/react-spectrum/issues/6405 shows that this is not only limited to full screen dialogs, but modal experiences in general.

Provide a general summary of the issue here

we noticed that using the example from https://react-spectrum.adobe.com/react-aria/useDialog.html causes issues when creating a full screen dialog where an input is at the bottom of the dialog, it's impossible to see the input, the keyboard covers it, it only happens on iOS, old and new safari versions. we tried using radix dialog and it works properly so it seems some internal logic of react-aria.

https://github.com/adobe/react-spectrum/assets/77207702/9e493c13-bc9d-499a-af16-5a5817f3f71c

๐Ÿค” Expected Behavior?

the dialog should leave the browser to focus the input and "fix" by itself the keyboard covering.

๐Ÿ˜ฏ Current Behavior

the keyboard covers the input and it's impossible to scroll down, the dialog scrolls up when trying it.

๐Ÿ’ Possible Solution

no sure

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

  1. Open this codesandbox, it uses react aria and radix, same styling.
  2. Click on Open React aria dialog and select input at the bottom of the dialog and the keyboard will cover the input and it won't zoom correctly.
  3. Try the same with Radix and it won't happen

Version

3.32.1

What browsers are you seeing the problem on?

Safari

If other, please specify.

No response

What operating system are you using?

iOS

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

JRS-Developer avatar Feb 22 '24 22:02 JRS-Developer

Since you are using a hook implementation, you'll probably want to use useViewportSize to determine the height of your modal for cases like these. Alternatively, you could use React Aria Components for your Modal which will provide the viewport's size, see https://react-spectrum.adobe.com/react-aria/Modal.html#modaloverlay

LFDanLu avatar Feb 23 '24 01:02 LFDanLu

Use the height of the modal for? Could you explain more? @LFDanLu

JRS-Developer avatar Feb 23 '24 15:02 JRS-Developer

@JRS-Developer since the virtual keyboard takes up space in the viewport, you can set the height of your fullscreen modal to match the height of the the visual viewport and thus have it adapt to when the virtual keyboard is open. You can see this in action with the edit modal in the first Table example on the React Aria home page. The style in question: https://github.com/adobe/react-spectrum/blob/fa3862eab668b43459dd99f7fd4bf74e6eda492b/packages/dev/docs/pages/react-aria/home/ExampleApp.tsx#L488

LFDanLu avatar Feb 23 '24 18:02 LFDanLu

Hello @LFDanLu, will this work in cases where the modal is bigger that the mobile screen? Like an modal that overflows? Like the scroll=body of Mui? https://mui.com/material-ui/react-dialog/#scrolling-long-content

JRS-Developer avatar Feb 27 '24 15:02 JRS-Developer

The scroll=body case shown for material-ui probably wouldn't work since the strategy I suggested above is to make sure that the modal's height is equal to the visual viewport's height and to make the body of the modal scroll instead.

LFDanLu avatar Feb 27 '24 18:02 LFDanLu

@LFDanLu Yeah, we use a lot dialogs that work like the scroll=body and it's pretty anoying for IOS users

JRS-Developer avatar Feb 27 '24 18:02 JRS-Developer

I would need to dig further into what radix is doing vs our implementation, but it honestly feels to me to be a pretty odd experience in that case too. From your video, the virtual keyboard still covers the textfield when opened until after some delay (not sure what caused it to scroll into view). If an additional interaction was required to make the input scroll into view after the virtual keyboard opened then that feels less than ideal. With the example in the React Aria home page, the modal shrinks and the input remains in view:

https://github.com/adobe/react-spectrum/assets/9661127/561b8b6c-f45f-4d19-ac02-75830fbd8924

LFDanLu avatar Feb 27 '24 19:02 LFDanLu

Hello, I updated the example with a really simple dialog without any library and it works as radix, it's really an issue with react-aria, here is the video using all dialogs including the simple one

https://github.com/adobe/react-spectrum/assets/77207702/f84579e5-13f1-48b2-8bef-caaf01bcf36b

JRS-Developer avatar Feb 27 '24 23:02 JRS-Developer

Maybe some logic inside Overlay or useModalOverlay, no really sure

JRS-Developer avatar Feb 27 '24 23:02 JRS-Developer

It seems to be due to something in our preventScroll code for iOS Safari specifically, getting rid of that makes it behave like the native dialog that you shared. That code was written originally to prevent people from scrolling the background element beneath the Modal's underlay when the Modal is open so maybe we can get away with not enabling that behavior when the modal being used is a full screen modal that doesn't have a underlay. However, there could be a case where you have a very tall modal that still have an underlay visible that we will still need to solve for here. I've tried locally tearing out bits and pieces of that code but no luck so this will likely need a deeper dive since the fix itself was something hacky to work around mobile Safari quirks. All in all though, this code and much of our other modal specific code was developed with the contents of the modal being scrollable and the height of the modal itself never exceeding the visual viewport's height...

LFDanLu avatar Feb 28 '24 00:02 LFDanLu

For whoever picks up this bug in the future, note that the same issue can exhibit itself for our fullscreen modal dialog in RSP as well, looks like either an issue with the scrollIntoView code calculation or a race condition between when the scrollIntoView code calls itself and when the modal adjusts its own height to match the new visual viewport height

LFDanLu avatar Feb 28 '24 00:02 LFDanLu

I also tested in a simulator and even if it's not similar to a real device, there is some similar behaviour between the two. Focusing on date elements in the date selector makes the UI jump up and down as seen in the last part of the video:

https://github.com/adobe/react-spectrum/assets/4083652/5904d043-9628-4714-8059-f8fed2354485

IonelLupu avatar May 06 '24 15:05 IonelLupu

Similar issue is also observed when using the useModalOverlay hook to create a modal. As seen here

brianudensi avatar May 17 '24 12:05 brianudensi

@LFDanLu thanks for the swift response, just curious on when we should be looking out for an update regarding this issue?

brianudensi avatar May 21 '24 07:05 brianudensi

Hard for me to say, the team is pretty booked for the next couple of sprints, but I can see about bring this up in our next planning session and see if we can get it slotted in. Any help investigating solutions is welcome!

LFDanLu avatar May 21 '24 16:05 LFDanLu

Running into the same issue - Have implemented https://react-spectrum.adobe.com/react-aria/examples/framer-modal-sheet.html and was afraid it was related to transform from Framer Motion. But after removing all styling on the modal/dialog it still happens. It's definitely related to the scroll prevention. You can see it tries to scroll the screen for a split second, before snapping back.

https://github.com/adobe/react-spectrum/assets/3764345/c1f1b71d-f043-4436-ac68-339c995e84ef

thebuilder avatar Jul 03 '24 09:07 thebuilder

So very sorry to be that guy but any chance this made it into the queue @LFDanLu ?

subvertallchris avatar Jul 03 '24 23:07 subvertallchris

It is still in our backlog unfortunately.

LFDanLu avatar Jul 08 '24 16:07 LFDanLu

@LFDanLu Great but this is a blocking issue, not something that don't impact..

Emiliano-Bucci avatar Jul 26 '24 19:07 Emiliano-Bucci

I can see about bringing it up again for our upcoming sprint planning. If you could thumbs up the issue, that would be helpful for us to gauge priority.

LFDanLu avatar Jul 29 '24 17:07 LFDanLu

So basically if a critical issue that broke the usage of a dialog in iOS don't have enough likes is not a priority? Love Adobe philosofy!

Emiliano-Bucci avatar Jul 29 '24 19:07 Emiliano-Bucci

This is a free and open source library, provided as is. We aren't paid to work on your personal priorities. You are welcome to fork the repo, use patch-package, or contribute a fix if it is blocking you, otherwise, we will try to incorporate it into our upcoming work. Thanks for your patience.

devongovett avatar Jul 29 '24 21:07 devongovett

Here's a fork of the original sandbox that works for me: https://codesandbox.io/p/github/JRS-Developer/testing-dialogs/csb-jh5gwz/draft/nameless-sea See demo here on iOS: https://jh5gwz-3000.csb.app/.

The only change I made was to set a height on the position: fixed element that uses the visual viewport height (the space above the keyboard) instead of setting bottom: 0. If you're using hooks, you could do it like this:

height: useViewportSize().height + "px",

If you're using React Aria Components, this is available as the --visual-viewport-height custom property. The example in our docs also uses this in its styles, and appears to work correctly on iOS.

We expect that the dialog will be positioned above the keyboard, and scroll within that area, rather than extending behind the keyboard. This needs to be handled in your styles.

I noticed that the other examples in the sandbox (radix and simple dialog) scroll the input into view initially, but you can't actually scroll up to the top of the dialog anymore once the keyboard is visible. That's because the dialog gets pushed up, and it is too tall so extends outside the viewport. I think it's a better experience to shrink the height of the dialog when the keyboard is open, so it is scrollable above the keyboard and all content is reachable.

Given this is working with the above tweak, it seems like this isn't so much a bug as something we could improve in the docs. Am I missing something else that's broken?

devongovett avatar Jul 30 '24 01:07 devongovett

Hi, I see that you are frustrated by things not working as you expected. That is understandable. I'd like to take a moment and remind us all the code of conduct we have https://github.com/adobe/react-spectrum/blob/main/CODE_OF_CONDUCT.md.

Were you able to resolve your issue with the methods outlined above?

snowystinger avatar Jul 30 '24 07:07 snowystinger

@snowystinger I see that you've deleted my comment. I want you to understand that I'm not frustrated at all, because I just simply resolved my problem by using another library that work as expected without spending time trying to make things work because people is not able to do it's job right. I was just saying that the fact that you work free doesn't give you the right to break stuff in the internet, that's not a good philosofy in life :)

Emiliano-Bucci avatar Jul 30 '24 07:07 Emiliano-Bucci

Thanks for the solution @devongovett - Much appreciated. It also seemed like a weird oversight given all the work you have put into making everything in this library work in tandem.

It would be good to clarify the the importance of the visual viewport in docs/examples, since it's not super clear what it actually does.

The Modal Sheet example could use a documentation tweak, so it also includes the visual viewportsize. It's a bit more tricky here since it adds Framer Motion, that relies on the calculated height. Right now it just reads window.innerHeight.

thebuilder avatar Jul 30 '24 11:07 thebuilder