Fix go back/forward gesture doesn't work#52355
Conversation
|
@mananjadhav Please 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] |
|
NOTE: gestures don't work inside the report screen. If we remove the inverted, then it works fine. So, looks like it's a react-native-web bug with the inverted props. |
|
Checking at my end too. |
|
I am going to approve this as I didn't find a way to quickly solve this. @Beamanator are we okay to not have this for the Report screen. |
|
As long as we're not making anything WORSE on the Report Screen, I'd say we're ok to move forward. But it would be nice to eventually get this fixed to make it consistent everywhere - @bernhardoj is that something you're willing to work on - figuring out what the fix could be in |
|
I found that it's because in react-native-web, if we set inverted, it will set a 'wheel' listener to handle the inverted scrolling and calls e.preventDefault which prevents the gesture to work. I don't think we can do anything here. |
|
@Beamanator Can you please generate adhoc build for this one? I'll continue with the testing on a real device. |
|
🚧 @Beamanator has triggered a test build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
😢 That's not a "can-do" attitude!!! 😅 But ya ok sounds good, for now let's not worry about that 🙏 |
|
Did one test on iOS. Need to test on Android, will finish over the weekend. |
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid-gestures.movAndroid: mWeb Chromemweb-chrome-gestures.moviOS: Nativeios-gesture-regression.MP4.MP4iOS: mWeb Safarimweb-safari-gesture-regression.MP4MacOS: Chrome / Safariweb-gestures.movMacOS: Desktopdesktop-gestures.mov |
|
Since the only commit here was 2 weeks ago, @bernhardoj can you please pull main & do a quick retest so we can hopefully eliminate bugs that may have been caused by recent related changes? 🙏 |
|
Done. Still working |
|
✋ 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/Beamanator in version: 9.0.67-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.67-9 🚀
|
Explanation of Change
Disabling the overscroll-behavior: none disable the gestures, so we remove it.
Fixed Issues
$ #51704
PROPOSAL: #51704 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Gestures available only on web:
NOTE: gestures don't work inside the report/chat screen
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mp4
MacOS: Desktop