Repository navigation
MenuItem decomposition: migrate seven illustrated menu rows - #106773
Conversation
Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
Codex Review Summary: ✅ CompletedThis 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. |
|
@roryabraham 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] |
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.
|
|
🤖 Android test steps verification results: ❌ fail Sign-in recovery and onboarding succeeded on standalone Android NewDot. Wallet → Add bank account then required an emailed six-digit security code, blocking testing; no real bank accounts were configured or cards issued.
|
|
🤖 Web test steps verification results: ❌ fail Testing on dev NewDot web was blocked by mandatory email-code verification when opening Wallet → Add bank account. Testing stopped at this authentication blocker; no bank configuration or card issuance occurred, and these results indicate unverified coverage—not demonstrated PR regressions.
view run · no recording available |
|
oxfmt failing. Working on preventing that going forward in https://github.com/Expensify/melvin/pull/607 (that is merged and it should be fixed now) |
|
PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself. |
|
@MelvinBot fix oxfmt failing check |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb Safari |
Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
|
|
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|







Explanation of Change
Migrate the seven requested legacy MenuItem JSX sites to the existing compound API, without adding presets or props.
MenuItem.Root/Row, with plainIconillustrations inLeading. Leading icons have nofillprop, preserving each asset's own colors.popoverMenuIconwrapper plus the existing margins or bank-specific icon styles. Move wrapper styles to Root and compose Title/Description/Trailing Chevron explicitly.AI Tests
npm run fmtproduced no further diff; staged whitespace checks passed. Typecheck, changed-file lint, spelling, and all 22 tests in the two related Jest suites passed.prettierscript is absent; the actual CI formatter (npm run fmt) passed instead. Inline review found no formatting-introduced violations; existing static-config composition findings and an unrelated suppression were left unchanged because restructuring those is outside this formatting-only request. No UI behavior was changed or re-tested.check-changedwrapper timed out. The configured matrix'sprettierscript is absent; a targeted formatter check was denied by the execution policy.Fixed Issues
$ #100312
PROPOSAL: #100312 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review".
// Please describe what tests you performed that validates your changes worked.
Executed automated checks and browser limitations are recorded in AI Tests. Ready-for-review status was explicitly requested; visual parity remains a manual QA item.
Offline tests
QA Steps
Use staging/test fixtures only. Do not issue a real card or configure a real bank account.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Browser verification blocked by Control-plan gating. This is setup evidence, not successful visual verification of the migrated rows.