[CP Staging] Revert report fields InitialListValueSelector dynamic route migration (#91627, #93153)#93191
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@ShridharGoel 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53f52d8113
ℹ️ 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".
|
No checklist, straight revert of two PRs |
|
🚧 @Julesssss 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! 🧪🧪
|
…301102343-1 🍒 Cherry pick PR #93191 to staging 🍒
|
🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.4.4-2 🚀
Bundle Size Analysis (Sentry): |
Help site review — no documentation changes requiredI reviewed the changes in this PR against the help articles under Why: This PR is a purely internal navigation/routing revert. It restores the modal-based
The end-user experience of setting an Initial value for a list-type report field is functionally unchanged — same picker, same labels, same subtitle copy. No feature was added or removed, no UI labels/tabs/settings were renamed, and no product copy changed. Help articles document user-facing product behavior, none of which is affected here. @mountiny — flagging for your awareness; no linked help site PR was created because no documentation changes are required. Let me know if you'd like me to document anything specific about the report-field initial value flow regardless. |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.4-3 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.4.5-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.5-6 🚀
|
Explanation of Change
Clean revert of two merged PRs (in reverse merge order), requested by
mountinyto clear the DeployBlockerCash where the report-field initial value error persists after selecting an initial value:Expensify/App#93153[CP Staging] Fix report field Initial value: refresh redirect + status bar overlap #93153 — [CP Staging] Fix report field Initial value: refresh redirect + status bar overlap (follow-up fix). Reverted cleanly with no conflicts.Expensify/App#91627Migrate InitialListValueSelectorModal to dynamic @react-navigation route #91627 — Migrate InitialListValueSelectorModal to dynamic @react-navigation route. This restores the modal-basedInitialListValueSelectorand removes theDYNAMIC_REPORT_FIELDS_INITIAL_LIST_VALUEroute/screen and its navigation wiring.Each revert is its own commit so the original authorship/history is preserved.
Conflict resolution notes (#91627 revert)
#91627's 11 files were almost entirely auto-reverted. Two files needed manual resolution becausemainmoved since the original merge:src/libs/Navigation/types.ts—mainadded an unrelatedINVOICE_FIELDS_CREATEentry adjacent to#91627'sDYNAMIC_REPORT_FIELDS_INITIAL_LIST_VALUEentry. Resolved by removing only theDYNAMIC_REPORT_FIELDS_INITIAL_LIST_VALUEentry and keepingINVOICE_FIELDS_CREATE.src/pages/workspace/reports/CreateReportFieldsPage.tsx— since#91627merged, this file's inline form was extracted into the sharedsrc/pages/workspace/fields/CreateFieldsPage.tsxcomponent (a separate, later refactor). A mechanical revert would have restored the old inline form and undone that unrelated refactor, so I keptmain's current refactored version of this file.#91627's only change here had been the removal of a single cosmeticsubtitle={translate('workspace.reportFields.listValuesInputSubtitle')}prop on the initial-valueInputWrapper. That form now lives in the sharedCreateFieldsPage.tsx(also used by invoice fields), so I did not re-add the cosmetic subtitle there to avoid touching the shared component / invoice-field behavior.mountiny— flagging this one residual cosmetic difference for you to confirm; everything functional from#91627is reverted.Local AI checks (run by MelvinBot)
npm run typecheck(tsc) — ✅ passnpm run lint-changed— ✅ passnpm run prettier --check(changed files) — ✅ passreact-compiler-compliance-check(changed.tsx) — ✅ all compiledFixed Issues
$ #93186
$ #93141
$ #93138
PROPOSAL:
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 changed worked.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
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./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari