Repository navigation
[CP Staging] Revert "Reapply "Add a More menu option to switch single-expense reports between expense view and table view"" - #104661
Conversation
…eExpenseReportViewToggle" This reverts commit 7cb9aa2, reversing changes made to 7255f7a. Co-authored-by: Vit Horacek <[email protected]>
melvin-bot
Bot
added
Melvin-Test-Android
Melvin-Test-Web
Codex Review SummaryThis 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. |
mountiny
changed the title
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.
|
![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: 3249384825
ℹ️ 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".
| return ( | ||
| <ReportSubmitToPopoverRoot reportID={reportID}> | ||
| <SelectionToolbar | ||
| reportID={reportID} | ||
| transactions={transactions} | ||
| reportActions={reportActions} | ||
| /> | ||
| </ReportSubmitToPopoverRoot> | ||
| <SelectionToolbar | ||
| reportID={reportID} | ||
| transactions={transactions} | ||
| reportActions={reportActions} |
There was a problem hiding this comment.
When a Submit-policy report has multiple rows but only one selectable row (for example, another row is disabled by a reject error), isEveryReportTransactionSelected() deliberately exposes the report-level Submit action and the one selected transaction does not trigger the multi-selection submit block. useSelectionModeReportActions() calls useLifecycleActions() while rendering SelectionToolbar, so after this wrapper is removed it reads the default no-op useOpenReportSubmitToPopover() value; the ReportSubmitToPopoverAnchor rendered later around the narrow dropdown cannot provide context to its parent hook, and the wide toolbar has no provider at all. Consequently, pressing the offered Submit action does nothing instead of opening the submit-to picker.
Useful? React with 👍 / 👎.
|
🤖 Android test steps verification results: ✅ pass On Android, the single-expense report's More menu no longer has the table view option. Tapping the expense preview opens the transaction thread. Deleting the expense returns straight to the workspace chat. No errors appeared during the run. The PR's only documented step is "no console errors," so I also tested the flows from the linked deploy blockers.
|
|
🤖 Web test steps verification results: On web, the "Switch to table view" option is gone from the More menu. The expense preview opens the transaction thread, and deleting the expense returns straight to the workspace chat. I couldn't check the JS console because the web test tool can't read console logs, so the PR's one documented step is unverified.
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 mountiny 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! 🧪🧪
|
…eportViewToggle [CP Staging] Revert "Reapply "Add a More menu option to switch single-expense reports between expense view and table view"" (cherry picked from commit ebf3e87) (cherry-picked to staging by mountiny)
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.5.16-1 🚀
|
|
No help site update is needed. This PR reverts the Switch to table view / Switch to expense view More menu option, and no article on Note: the open draft Docs updates for E/A#103703 Add single-expense view switch to More menu table documents this feature. It should not be merged while the feature is reverted. |
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.5.18-0 🚀
|
|
No help site update is needed. This PR reverts the single-expense view switch, and no published article under One follow-up: the open draft docs PR Docs updates for E/A#103703 Add single-expense view switch to More menu table adds a Switch to table view or Switch to expense view row to |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.18-1 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.5.20-0 🚀
|
|
No help site update is needed. This PR reverts the Switch to table view / Switch to expense view option, and no merged article on One open draft docs PR still adds that option to the More menu table: Docs updates for E/A#103703 Add single-expense view switch to More menu table. Don't merge it while the feature is reverted. Hold or close it until the feature ships again. |
|
Do we need QA this? |
|
@IuliiaHerets straight revert so no thanks, thanks for asking! |
Explanation of Change
Reverts Reapply "Add a More menu option to switch single-expense reports between expense view and table view" because QA found new deploy blockers, listed under Fixed Issues.
I made this with
git revert -m 1 7cb9aa2eaa61481ca71789877b95a36265e0d49don top of the latestmain.Conflicts
There was one conflict, in the import block of
src/libs/ReportSecondaryActionUtils.ts:import {isSingleTransactionReport} from './MoneyRequestReportUtils';.import {getLoginByAccountID} from './PersonalDetailsUtils';on the next line.I removed the
isSingleTransactionReportimport, which only the reverted feature used. I kept thegetLoginByAccountIDimport, which code from that later PR still uses. Every other file reverted cleanly.Checks after the revert
src/ortests/, for exampleuseSingleExpenseReportView,TOGGLE_SINGLE_EXPENSE_VIEW,NVP_SINGLE_EXPENSE_REPORT_VIEW, orswitchToTableView.npm run typecheck, lint, or Jest in my environment, so CI needs to confirm them.Fixed Issues
$ #104645
$ #104649
$ #104651
$ #104653
$ #104654
$ #104657
$ #104676
$ #104678
$ #104681
$ #104682
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)Avatar, 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
Not yet tested — needs manual QA.
Android: mWeb Chrome
Not yet tested — needs manual QA.
iOS: Native
Not yet tested — needs manual QA.
iOS: mWeb Safari
Not yet tested — needs manual QA.
MacOS: Chrome / Safari
Not yet tested — needs manual QA.