Share extension: pre-mount destination report and create expense before reveal transition - #97242
Share extension: pre-mount destination report and create expense before reveal transition#97242jmgraa wants to merge 37 commits into
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
JakubKorytko
left a comment
JakubKorytko
left a comment
There was a problem hiding this comment.
the pre-mount idea is right, and the refactor is clean 🎉
Main thing: this re-does what the manual flow already has (getSubmitExpensePreMountDestinationRoute + handleReportPreInsert + usePreMountDestination) and drops the stability guard, which I think breaks the pre-mount. Details on the preMountDestinationRoute line.
On telemetry: ManualSubmitToDestinationVisible can't regress here - thank you for that haha. Share never calls startTracking, so the span is never started and every touchpoint is behind isTracking(). But that also means the whole flow is unmeasured, so the win this PR claims isn't verifiable, added a more precise comment in code.
Three behaviour changes also look inherited rather than decided: Search tab, wide layout, and report-already-in-RHP.
…ountDestinationRoute
JakubKorytko
left a comment
JakubKorytko
left a comment
There was a problem hiding this comment.
the pre-insert gates and optimisticChatReportID close most of the last round, so what's left is mainly the pending-navigation branch (still the one that can park the modal in a loading state with nothing to resolve it) plus a few leftovers from round one. Otherwise, LGTM!
|
@MelvinBot create a new issue for this and attach it as a sub-issue of #95335 |
|
Created a new issue for this and attached it as a sub-issue of #95335: 👉 #97667 — Share extension: eliminate Inbox flash when dismissing share modal after submit It captures the Inbox-flash-during-dismiss problem this PR addresses and links back to both this PR and the parent performance issue. The parent now tracks 2 sub-issues. |
|
@ChavdaSachin 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] |
![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: 19d8139deb
ℹ️ 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".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…reate-expense # Conflicts: # src/pages/Share/SubmitDetailsPage.tsx
4 participants
Explanation of Change
Before this change, submitting a shared receipt could briefly show the Inbox behind the share modal before the destination chat appeared on top - a visible flash during dismiss.
SubmitDetailsPagenow handles two navigation strategies based on layout and report state:Narrow layout: Pre-mounts the destination report behind the modal (when it already exists in Onyx) so the correct chat is ready when the modal closes.
Wide layout (tablets/desktop): Reveals the destination report on top of the modal first, then dismisses the modal after the animation completes.
Both strategies keep the confirm button in loading state until navigation is safe. When the destination report doesn't exist yet (new recipient), the page waits for the optimistic report to land in Onyx before navigating - with fallback timers (500ms if ID changes, 5s absolute timeout) to prevent indefinite hangs.
The expense is created immediately on submit (not after the reveal animation), and cleanup skips a second navigation (shouldNavigate: false) since reveal/pre-mount already handles routing. Stale tracking spans are cancelled before creation to avoid lifecycle conflicts, improving telemetry accuracy.
Fixed Issues
$ #97667
PROPOSAL:
Tests
Share extension with existing recipient
Share extension with new recipient
Share extension with destination already topmost
Repeat tests 1-3 both on narrow and wide layout
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
iOS: Native
ios.mov