Fix: preserve prev/next carousel arrows when opening an expense from an expense report#96556
Fix: preserve prev/next carousel arrows when opening an expense from an expense report#96556MelvinBot wants to merge 1 commit into
Conversation
…port The MoneyRequestReportView mount effect guarded against taking over a descriptor-backed carousel (Home Recently added) but not a snapshot-backed one (the Spend page seeds the carousel with a snapshot hash and no descriptors). It overwrote the Spend-page carousel and its unmount cleanup then wiped it, so the prev/next arrows disappeared after navigating back. Extend getActiveTransactionIDs to expose the snapshot hash and bail out of the report-view effect when a snapshot-backed carousel is already active, mirroring the existing descriptor guard. Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
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.
|
|
🤖 I looked into the failing checks — there's no code fix to push. The only failing check is
That's a human-only checklist — it needs the author/reviewer to check the boxes, so I can't complete it for you. Every actual CI check is green: Next step: tick the boxes in the PR Author Checklist section of the description and the |
Explanation of Change
The prev/next carousel arrows in the expense RHP are driven by a single shared Onyx key (
TRANSACTION_THREAD_NAVIGATION_TRANSACTION_IDS). When the full expense-report view (MoneyRequestReportView) mounts on top of an existing transaction thread, its effect inMoneyRequestReportTransactionListseeds that key with the report's own transactions and registers an unmount cleanup that callsclearActiveTransactionIDs().That effect already guards against taking over a descriptor-backed carousel (the Home "Recently added" flow) so it doesn't clobber the carousel of the thread sitting underneath it. But the Spend page seeds the carousel with a search snapshot hash and no descriptors (
setActiveTransactionIDs(siblingTransactionIDs, hash)), so this guard didn't cover it. As a result, opening an expense from the Spend page → clicking the report subtitle link → backing out overwrote and then cleared the Spend-page carousel, dropping the list below 2 items so the arrows disappeared.This fixes it by extending the existing guard to also bail when a snapshot-backed carousel is active, mirroring the descriptor handling:
getActiveTransactionIDs()now also returns the activesnapshotHash.MoneyRequestReportTransactionListeffect returns early (registering no cleanup) when either a snapshot hash or descriptors are present, so it never clobbers the underlying Spend-page carousel. Row presses inside the report still re-seed the correct siblings lazily viauseNavigateToTransactionThread.Fixed Issues
$ #96545
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari