-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix simultaneous scrolling in two directions #60340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix simultaneous scrolling in two directions #60340
Conversation
|
@DylanDylann 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] |
So if we hope we get the original patch merged upstream, how can we fix this without forever maintaining a patch for this? |
It's a good note that I thought about, but quite a tricky question: It's a typical FlatList trait that it scrolls even when user wheels diagonally, so ideally what we would try to achieve is blocking the ReportActionItem event from reaching him, while letting event get to it's child, which is the Carousel, when user's scrolling it. It's definitely not typical to block parent's event from inside him while also letting it pass to the child and none of the methods that could be implemented from inside of Current solution doesn't change the propagation of the events, but how the inverted list handles it - namely scrolls only when delta of the list's direction is higher than the second one (f.ex. deltaX > deltaY). We didn't find a way to change it outside of ReportActions List other than with this patch. If we want to get it fixed quick, we can merge this and I'll keep working on it in the meantime - the patch that allows carousel to scroll manually will probably be here for a while, so we can go with this change. |
Yeah I think we get this fixed now and then try to think of some other solution so we do not have to maintain patch forever |
@DylanDylann feel free to review |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-04-21.at.17.15.00.movAndroid: mWeb ChromeScreen.Recording.2025-04-21.at.17.13.19.moviOS: NativeScreen.Recording.2025-04-21.at.17.15.15.moviOS: mWeb SafariScreen.Recording.2025-04-21.at.17.11.32.movMacOS: Chrome / SafariScreen.Recording.2025-04-21.at.17.09.57.movMacOS: DesktopScreen.Recording.2025-04-21.at.17.12.35.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, great job
✋ 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.32-0 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.32-8 🚀
|
Explanation of Change
PR ensures that web implementation of inverted FlatList scrolls in only one direction at a time by comparing X and Y deltas, then choosing the direction with the bigger delta.
Fixed Issues
$ #60276 (comment)
PROPOSAL:
Tests
Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2025-04-16.at.12.28.32.mov
MacOS: Desktop
Screen.Recording.2025-04-16.at.17.55.23.mov