Fix test drive modal not moving when keyboard openes#62799
Conversation
|
@shawnborton @ZhenjaHorbach One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
cc @Expensify/design - I think this is fine, though I could have sworn we have a pattern where bottom-docked modals leave a little bit of space above them (for the status bar, etc), and then the modal content itself would scroll down. Does that sound right? |
Yeah I definitely remember that as well. It's not terrible as is, but I'd love for all these to behave the same. |
|
Can you take a look at that @jmusial? I think you can make a lot of saved searches on the mobile Reports page, and then you should see the bottom-docked scrolling behavior I am referencing. We might have it elsewhere too for other onboarding flows for all I know! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2025-05-29.10.16.51.movAndroid: mWeb Chrome2025-05-29.10.17.46.moviOS: HybridApp2025-06-05.19.12.35.moviOS: mWeb Safari2025-06-05.19.13.56.movMacOS: Chrome / Safari2025-05-29.09.17.38.movMacOS: Desktop2025-05-29.09.17.38.mov |
Agreed. IOS looks a bit broken as is with the button being covered by the keyboard |
|
hey @shawnborton sure, I'll take a look |
|
@jmusial |
|
Android apps work good ! But on ios web noticed that 2025-05-29.09.22.59.movAnd on ios we don't have modal animation when we open a keyboard 2025-05-29.10.10.13.mov |
That seems like something we should fix. Same with the other feedback. |
|
I checked with the old modal and it seems the behaviour is the same there. Lack of modal animation is an issue for Android Native as well. The empty space on IOS web is likely due to the difference of how keyboard is added on andorid web vs ios web. But nontheless I'll try to fix those issues. |
|
@jmusial |
|
hey @ZhenjaHorbach, sorry got sidetracked on other issues. |
|
@ZhenjaHorbach update on this PR - still trying to resolve ios/safari scroll bug. Some context The problemSafari overscrolls the content when keyboard is present. The blank space is not part of the html :( . Seems the issue is not fresh (and common enough to have medium articles written about it. The causeI managed to trace the problem and it's safari not supporting Unexpected findSeems like staging build behaves different from dev -.-' @mountiny could you run ad hoc web build for me on this pr? I'd like to check if it's reproducible then. EDIT: OK seems, like this may be connected to the safari top bar coloring (white on staging, grey on dev). Hence the area below the modal seems gray on the videos @ZhenjaHorbach @shawnborton is the white space on ios safari ok ? |
Hey @shawnborton I was trying to identify the behaviour you're mentioning. Can it be that you're referring to modal behaving differently on other platforms ? Please see attached vids (none affected by this PRs' changes) AFAIK the test drive modal is probably the only bottom docked modal with input (apart from emoji modal, but this one is smaller) causing the keyboard to show. Test drive IOS nativeios_native_test_drive.mp4Test drive IOS safariios_safari_test_drive.mp4Saved searches IOS nativeios_native_saved_searches.mp4Saved searches IOS safariios_safari_saved_searches.mp4 |
|
This is what I would expect in terms of scrolling, etc - let me know if this helps, and if this is possible! CleanShot.2025-06-05.at.11.34.22.mp4cc @Expensify/design to see if you agree as well. |
|
Great video to illustrate. Agree that the modal/sheet content itself should scroll. |
|
@shawnborton do you mind if I do it as a separate issue ? Changes in this brand do not introduce any regressions vs what is on prod |
|
I am cool with that 👍 |
|
100% agree with you Shawn! |
|
Cool, thx! @ZhenjaHorbach can you complete the review of the rest when you have time please ? |
|
Created an issue for you here: #63569 |
Nice |
|
Okay LGTM ! |
mountiny
left a comment
There was a problem hiding this comment.
Thanks! Lets fix that issue in a follow up
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.63-0 🚀
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.1.63-6 🚀
|
Explanation of Change
This PR reverts workaround from #62701 and implements fix in the new bottom docked modal. The root cause of this issue was how keyframe animations work in reanimated (they add a fixed height container for the Animated.View, and it was not adapting when keyboard was appearing in mWeb Chrome)
Fixed Issues
$ #62682
$ #49354
PROPOSAL:
N/A
Tests
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
fix_trainig_android_native.mov
Android: mWeb Chrome
fix_training_andorid_chrome.mov
iOS: Native
fix_training_ios_native.mp4
iOS: mWeb Safari
fix_training_ios_safari.mp4
MacOS: Chrome / Safari
MacOS: Desktop