Scan - In multiple receipt modal animation is not shown#92239
Conversation
|
@mkhutornyi 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] |
Reviewer Checklist
Screenshots/VideosAndroid: mWeb ChromeNative only iOS: mWeb SafariNative only MacOS: Chrome / SafariNative only |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3320ce982
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| imageHeight="auto" | ||
| imageWidth="auto" | ||
| modalInnerContainerStyle={styles.pt0} | ||
| illustrationOuterContainerStyle={styles.multiScanEducationalPopupImage} |
There was a problem hiding this comment.
Set the modal illustration aspect ratio
When the SVG now falls back to ImageSVG's 100% width/height, FeatureTrainingModal still wraps it in a 4:3 inner viewport because illustrationAspectRatio is omitted (the default is the video ratio, 1280/960). This modal’s outer container uses styles.multiScanEducationalPopupImage with aspectRatio: 1.7, matching the multi-scan SVG viewBox (375/220), so the 4:3 inner view becomes taller than the clipped outer view and the centered SVG is cropped at the bottom on the same Android/iOS path this fixes. Pass the SVG’s aspect ratio here so the image renders at the intended size instead of being partially hidden.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
makes sense, added the original approach
| // if (!showEducationalPopup || !dismissEducationalPopup) { | ||
| // return null; | ||
| // } |
|
@rinej please merge main again. Lots of checks failing |
…ceipt-modal-animation-is-not-shown
|
updated, tests passed |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @chuckdries 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/chuckdries in version: 9.3.97-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review: no changes required. This PR is a pure rendering fix. It replaces Nothing about the feature's behavior, workflow, labels, buttons, or limits changed — only an illustration that previously failed to render now renders. The help article that covers this flow ( Because there's no user-facing behavior change to document, I did not create a draft help site PR.
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.97-1 🚀
|



Explanation of Change
Fix multi-scan educational modal illustration not rendering on Android/iOS native. The modal was passing imageWidth="auto" / imageHeight="auto" - it collapses the SVG to zero size. Removed the two props so ImageSVG falls back to its "100%" default.
Fixed Issues
$ #92206
PROPOSAL:
Tests
Offline tests
QA Steps
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))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.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