Repository navigation
SelectionList ListItem decomposition - sweep and flag deletion - #100954
Conversation
|
@codex review |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b08459eaa
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
|
@MelvinBot review |
ReviewThe refactor looks good. Deleting I ran Equivalences I checked and confirmed are safe
1. The RBR indicator is silently dropped for seven rows
I could not find a consumer that sets 2.
|
|
@MelvinBot updated 2 & 4. 1 intended, 3 list limit rows rendered, I think can be left as is |
|
Agreed on 1 and 3. 4 is resolved. The fix for 2 introduced one regression, though. Moving the avatar into
|
heyjennahay
left a comment
There was a problem hiding this comment.
Product review not required
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-09-18.at.14.33.52.movAndroid: mWeb ChromeScreen.Recording.2026-09-18.at.14.35.40.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2026-09-18.at.14.45.38.movMacOS: Chrome / SafariScreen.Recording.2026-09-18.at.14.29.47.mov |
|
@jmusial Could you resolve the AI's comments above? |
| value?: string; | ||
|
|
||
| /** Should display tag 'Recommended' */ | ||
| isRecommended?: boolean; |
There was a problem hiding this comment.
NAB because it's an existing prop, but this is a configuration prop used for conditional rendering of a child component - it should be removed and replaced with composition. @MelvinBot create a follow-up issue for this specific decomposition. Assign @jmusial and @roryabraham. Label it Weekly
There was a problem hiding this comment.
Created [SelectionList ListItem] Replace the isRecommended config prop on TravelDomainListItem with composition — assigned jmusial and roryabraham, labeled Weekly.
Scoped it to just this prop: the flag exists only to gate a Badge at TravelDomainListItem.tsx:68, and there's exactly one consumer (DomainSelectorStep.tsx:40), so it's a small change. The issue lays out two options — compose the row at the call site and drop the travel-specific Badge/copy out of src/components/SelectionList/, or pass the badge element via the existing actionElement/rightElement slots — and links it back to [$1000] [Tracking] SelectionList ListItem - Composition over Configuration.
|
🚧 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.4.92-0 🚀
|
|
No help site changes are required for this PR, so I did not create a docs draft PR. This is an internal component refactor. Nothing a user sees, clicks, or reads changed. What I checkedThe PR is structural only. It consolidates the remaining No copy changed. Every
No The plan type lock is preserved, not changed. In Articles I checked and left alone, because the flows they document are unchanged:
One caveat: I could not drive NewDot in a browser on this run, so this is based on code review rather than live UI verification. The diff contains no string change and no changed conditional to verify — the copy and the gating logic are identical before and after. Since no docs PR was needed, there is nothing for you to review or mark |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.92-8 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
BaseListItemwrappers (SpendCategorySelectorListItem,SpendRuleListItem,SplitListItem) ontoListItemComposed, deleteBaseListItem.rightHandSideComponentrender prop withitem.actionElement.shouldDisplayRBR, prop-levelshouldShowRightCaret,FooterComponent.BaseSelectListItemrendersitem.leftElement, falling back to a compact avatar fromitem.icons.ListItem/types.ts(dropBaseListItemPropsand per-row aliases) and add tests for the migrated rows.Fixed Issues
$ #98362
PROPOSAL:
Tests
Offline tests
Same as 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
Screen.Recording.2026-09-17.at.10.50.25.mov
Android: mWeb Chrome
Screen.Recording.2026-09-17.at.10.51.51.mov
iOS: Native
Screen.Recording.2026-09-17.at.10.43.50.mov
iOS: mWeb Safari
Screen.Recording.2026-09-17.at.10.44.26.mov
MacOS: Chrome / Safari
Screen.Recording.2026-09-17.at.10.34.17.mov