Repository navigation
Migrate attachment carousel and MultiGestureCanvas gestures to Gesture Handler 3 hooks - #103461
Conversation
…e Handler 3 hooks Move the attachment carousel (web pan and native pager), the Android PDF pager pan and MultiGestureCanvas (pinch, pan, single and double tap) from the deprecated builder API to the hook API. Gesture refs don't exist in the hook API, so the web carousel passes its pan gesture through the pager context as pagerGesture instead of the gesture ref in pagerRef. pagerRef now points to the pager itself and only tells the attachment it is rendered inside a carousel. In MultiGestureCanvas the pan gesture is created before the taps, because the single tap waits for it to fail; the taps declare the simultaneous relation with the pan instead, which is symmetric. Without the refs read during render, AttachmentCarouselView and MultiGestureCanvas now compile with React Compiler, so the React.memo workaround is dropped and the seatbelt baseline is tightened.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
OSBotify tightens the baseline after merge, so editing it here only causes conflicts between the migration PRs.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@QichenZhu 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] |
mikolajpochec
left a comment
There was a problem hiding this comment.
LGTM! I'm leaving two comments. Nothing blocking
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
|
@QichenZhu could you please test this one too? 🙏 |
This comment was marked as resolved.
This comment was marked as resolved.
|
I found no accessibility issues in this PR. It only changes how the gestures are wired up (the Gesture Handler 3 hook API). It does not touch any accessibility labels, roles, focus order, keyboard handling, or the carousel arrow buttons. What I checked
Already on I reviewed only the code. I didn't run the changes in the app. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-native.mp4android-native.webmAndroid: mWeb Chromeandroid-web.mp4android-web.webmiOS: HybridAppios-native.mp4ios-native.moviOS: mWeb Safariios-web.mp4ios-web.movMacOS: Chrome / Safarimac-web.mov |
|
NAB: on Android mWeb, in a chat with at least two image attachments, open the second one, double tap to zoom in, then immediately swipe right. The carousel stops responding to tapping and panning. nab-1.mov |
|
NAB: on Android mWeb, you need to swipe right on a PDF several times to switch to the previous attachment. nab-2.webm |
|
Please let me know when this is ready for re-review. |
|
@QichenZhu, the PR is ready for re-review. The problem was that the new hooks API changed the order of applying gestures on the web, which is why the Pan onDeactivated wasn't applied on mWeb iOS. The fix was to declare pitch before pan, and now everything works. I also added videos for mWeb iOS and Android 😄 |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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.5.20-0 🚀
|
|
No help site update is needed. This PR is an internal migration of attachment carousel and zoom gestures to the Gesture Handler 3 hook API, and it doesn't change any feature, label, or workflow that users see. |
Explanation of Change
Part of the migration from the deprecated Gesture Handler 3 builder API (
Gesture.*,GestureType,Legacy*types) to the hook API (#102782). The migration is split into independent PRs by feature area. This one covers the attachment carousel andMultiGestureCanvas.Mechanical changes:
Gesture.*()builders →use*Gesture()hooks,onStart/onChange/onEnd→onActivate/onUpdate/onDeactivate,state.activate()/fail()→GestureStateManager.activate()/fail(evt.handlerTag),successinonFinalize→!event.canceled.Non-obvious parts:
pagerGesture(used byLightbox,PDFView,MultiGestureIcon).pagerRefnow only tells an attachment it's inside a carousel. On native it never carried a real relation (PagerViewhas nohandlerTag).MultiGestureCanvas. A gesture can only reference gestures created before it, so the pan is created first and the taps declare the simultaneous relation with it (simultaneous relations are symmetric). The relations and theSimultaneous(pinch, Race(taps, pan))composition are unchanged.usePanGesture/usePinchGesture/useTapGesturesinMultiGestureCanvasare nowuseCanvasPanGesture/useCanvasPinchGesture/useCanvasTapGestures, so they no longer shadow the Gesture Handler hooks. The Gesture Handler hooks are called by their own names, and the worklets Babel plugin turns their callbacks into worklets automatically (checked on the transformed native output: all 14 callbacks are worklets).MultiGestureCanvasandAttachmentCarouselViewnow compile with both Babel and OXC, so theReact.memoworkaround onAttachmentCarouselViewis removed.Fixed Issues
$ #102782
PROPOSAL: N/A
Tests
iOS/Android:
iOS/Android:
.txtfile at once. In the preview, verify swiping switches between the files.IOS/Android:
Android:
Web:
4. open an image attachment with siblings and verify the arrows and the keyboard arrow keys switch attachments.
Note: pinch freezing at the max/min zoom on Android is a known bug on
main(#103648), not caused by this PR.Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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_attachment_compressed.mp4
Android: mWeb Chrome
Screen.Recording.2026-10-22.at.15.46.34.mov
iOS: Native
ios_attachmentCarousel_compressed.mp4
iOS: mWeb Safari
Uploading Screen Recording 2026-10-22 at 15.32.39.mov…
MacOS: Chrome / Safari
web_attachment.mov