adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.53k stars 1.08k forks source link

iOS keyboard covers inputs when using useDialog/useModalOverlay #5926

Open JRS-Developer opened 6 months ago

JRS-Developer commented 6 months ago

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

LFDanLu commented 6 months ago

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

JRS-Developer commented 6 months ago

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

LFDanLu commented 6 months ago

@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

JRS-Developer commented 6 months ago

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

LFDanLu commented 6 months ago

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.

JRS-Developer commented 6 months ago

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

LFDanLu commented 6 months ago

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

JRS-Developer commented 6 months ago

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 commented 6 months ago

Maybe some logic inside Overlay or useModalOverlay, no really sure

LFDanLu commented 6 months ago

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 commented 6 months ago

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

IonelLupu commented 4 months ago

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

brianudensi commented 3 months ago

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

brianudensi commented 3 months ago

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

LFDanLu commented 3 months ago

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!

thebuilder commented 2 months ago

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

subvertallchris commented 2 months ago

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

LFDanLu commented 1 month ago

It is still in our backlog unfortunately.

Emiliano-Bucci commented 1 month ago

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

LFDanLu commented 1 month ago

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.

Emiliano-Bucci commented 1 month ago

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!

devongovett commented 1 month ago

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 commented 1 month ago

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?

snowystinger commented 1 month ago

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?

Emiliano-Bucci commented 1 month ago

@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 :)

thebuilder commented 1 month ago

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.