Fix mobile web PDF pinch zoom#95226
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
melvin-bot
Bot
requested review from
JmillsExpensify and
nyomanjyotisa
and removed request for
a team
|
@nyomanjyotisa 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] |
melvin-bot
Bot
requested review from
a team and
puneetlath
and removed request for
a team
|
@puneetlath 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }); | ||
| }; | ||
|
|
||
| const scaleChange = useCallback( |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-0 (docs)
React Compiler is enabled in this codebase (babel-plugin-react-compiler) and automatically memoizes closures based on their captured variables, so wrapping scaleChange in useCallback is redundant and interferes with the compiler's own caching model. It also adds a dependency array ([actions, onScaleChanged]) that must be maintained.
Remove the manual memoization and use a plain function so the compiler can handle it:
const scaleChange = (scale: number) => {
onScaleChanged?.(scale);
actions?.onScaleChanged?.(scale);
};Reviewed at: e3c53f3 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
…pinch-zoom-comparison # Conflicts: # src/components/PDFView/index.tsx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cea2f8e38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2 similar comments
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
It looks like I ran into the same codex issue as the one mentioned here: https://expensify.slack.com/archives/C02NK2DQWUX/p1782486912897689 @puneetlath could you please help trigger the codex review? thanks! |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
|
@KJ21-ENG When a carousel page gains/loses focus, the iOS-mWeb.Safari.mp4 |
@nyomanjyotisa Fixed in 5f6e902, it was caused by 3dabc5a that i made for the codex's review findings Screen.Recording.2026-07-08.at.12.19.42.AM.mov |
|
@KJ21-ENG I found another issue with multi page PDF. When we zoom in on page 1 of a multi page PDF, we can't scroll/pan down to page 2. I guess because gesture canvas only pans within one viewport iOS-mWeb.Safari.mp4Also, please check all the items in the checklist and include recordings for all platforms to make sure we don't break anything |
|
@nyomanjyotisa Does this looks good to you ? or when scroll/pan down to page 2 then it should zoom out to default ? ezyZip.74.mp4 |
|
@KJ21-ENG Yeah, I think that's good. We can follow the native app behavior iOS-Native.mp4 |
|
@nyomanjyotisa Done, added in 9221811 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid-HybridApp.mp4Android: mWeb ChromeAndroid-mWeb.Chrome.mp4iOS: HybridAppiOS-Native.mp4iOS: mWeb SafariiOS-mWeb.Safari.mp4MacOS: Chrome / SafariMacOS-Chrome.mp4 |
|
@KJ21-ENG Could you also please check this comment? Thanks
|
|
@nyomanjyotisa strange, idk why but some recordings got failed to uplaod, may be i forgot to compress video. no worries i have uploaded agaian. Appologies for inconvenience. |
|
@puneetlath All yours now, thanks! |
|
✋ 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/puneetlath in version: 9.4.33-0 🚀
|
|
🤖 Help site review: no changes required. This PR adds mobile-web pinch / double-tap / pan zoom gestures for PDF attachments opened in the full-screen attachment viewer (reusing I checked
Since the help site documents features and workflows rather than gesture mechanics, no article needs updating and no draft docs PR was created. If you'd like a note added somewhere (e.g. a "you can pinch-to-zoom PDFs on mobile web" tip in an attachment article), let me know and I'll draft it. @KJ21-ENG, please confirm you agree no help site update is needed here. If you'd prefer a docs PR anyway, reply and I'll create one. |
6 participants
Explanation of Change
Adds in-app pinch, double-tap, and pan zoom support for mobile-web PDF attachments opened in the full-screen attachment viewer. The web PDF viewer now reuses
MultiGestureCanvas, forwards scale changes to the attachment carousel pager, and keeps inline chat PDF previews and desktop PDF rendering unchanged. While the PDF is zoomed, the PDF scroll surface stops handling browser touch scrolling so one-finger panning stays driven by the gesture canvas; at 1x, normal PDF scrolling remains enabled. The shared pan gesture also gets an explicit PDF-only opt-out for swipe-down-to-close so existing Lightbox behavior stays unchanged.Fixed Issues
$ #92109
PROPOSAL: #92109 (comment)
Tests
Offline tests
No dedicated offline behavior is changed. The PDF gesture handling is local UI behavior and does not enqueue network requests.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)MultiGestureCanvashooks.StyleUtilsdoes not cover touch-action gesture arbitration.MultiGestureCanvasis shared; image Lightbox regression coverage is included in the manual steps.main, I reran targeted checks and updated the PR for the remaining mobile-web gesture issue.Screenshots/Videos
Android: Native
1783595012338249.mp4
Android: mWeb Chrome
Record_2026-07-04-15-47-16.mp4
iOS: Native
ezyZip.76.mp4
iOS: mWeb Safari
VIDEO-2026-07-06-01-32-40.mp4
MacOS: Chrome / Safari
Not applicable. The gesture wrapper is gated to mobile web.