Repository navigation
Conversation
…g it Co-authored-by: Jayesh Mangwani <jayeshmangwani@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.
|
|
🤖 Android test steps verification results: The PR's Tests section is still a TODO, so I used the reproduction steps from Native share - Android - No sharing flow/app freezes when share image signed out. Sharing while signed out and the onboarding flow both worked: no setup-questions error and no share-modal flash. After onboarding, the share page showed "Failed to read the shared file". A control share sent while already signed in failed the same way, so the cause is probably how the test sends the share with |
|
@MelvinBot solution doesn’t work at all. Right now, when we log in with a new public domain user and press the Join button, we briefly see the “Share to Expensify” page, then the OnboardingWorkEmail page. After completing onboarding, we land on the home page. Can we handle this flow differently? When a new user comes from Share and logs in, they should go directly to the onboarding pages instead of briefly seeing the “Share to Expensify” page. After completing onboarding, they should land on the “Share to Expensify” page instead of the home page. PR-flow.mov |
… page once onboarding finishes - Drop the signed-out share link from react-navigation so the share page no longer flashes after sign-in. - Park the share once at sign-in. useOnboardingFlowRouter opens it right away for users who don't onboard, or holds it for onboarding. - navigateAfterOnboarding opens the held share after OnboardingGuard sees the completed onboarding NVP, so the guard redirect can't drop it and leave the user on Home. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
I changed the flow as you asked and pushed it in 65e4fdb. A new user who shares while signed out now goes straight to onboarding, and the share page opens when onboarding finishes. Why the last version failed
What changed
Tests
Please retest the Android flow from your video. view run · no recording available |
|
@MelvinBot latest changes don't work at all. I think something might be missing here |
IS_LOADING_APP is kept across sign-out, so right after a new user signs in on a device that was used before, the router saw "not loading" and an empty onboarding NVP, which reads as completed. It opened the share at once (the flash) and used it up, so onboarding then ended on Home. HAS_LOADED_APP is cleared on sign-out and set once OpenApp lands, so gate on it. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
You were right, something was missing. I pushed the fix in 6a0ccb3. What broke: Fix: the router now opens the saved share only after
Tests
Please retest on a device that was signed in before you shared, since that's the case this fixes. view run · no recording available |
|
@MelvinBot please merge latest main. |
…erOnboarding Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com> # Conflicts: # src/libs/navigateAfterOnboarding.ts # tests/unit/navigateAfterOnboardingTest.ts
|
I merged the latest I resolved two conflicts. Checks:
|
|
@MelvinBot please merge latest main. |
…erOnboarding Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com> # Conflicts: # src/libs/navigateAfterOnboarding.ts # tests/unit/navigateAfterOnboardingTest.ts
…erOnboarding Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
I merged the latest Conflicts resolved:
Checks: The three related unit test suites pass (23 tests). Typecheck and lint pass on the resolved files. |
|
MelvinBot has made 6 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
@MelvinBot , please close this PR. The changes don’t solve the issue. |
|
I tried to close this PR, but closing PRs isn't permitted in my current environment, so it is still open. Please close it manually, |








Explanation of Change
When a signed-out user shares a file to the Android app and then signs in as a new user, the share deep link is replayed after sign-in. Onboarding then either drops it, tears down the share modal, or shows "Please finish the setup questions" even though the user never pressed back.
This PR:
openReportFromDeepLinkmarks a pending share (newpendingShareIntenthelper) instead of dropping the link or showing the back-button error.navigateAfterOnboardingandnavigateToSubmitWorkspaceAfterOnboardingconsume it once and openshare/root. The shared file is still in the native shared preferences, so the share page reloads it.JSON.parsethrew inside the callback and the page stayed on the skeleton forever. It now shows the attachment error alert and returns to Inbox.Share_Rootfrom the last visited path, so a later cold start does not restore an empty share page.AI Tests (run by MelvinBot):
npm run typecheck: passednpm run linton changed files: passednpm run spell-changed: passednpm run react-compiler-compliance-check check src/pages/Share/ShareRootPage.tsx: passed (check-changedcould not resolve the base ref in this environment)npm test -- tests/unit/navigateAfterOnboardingTest.ts tests/unit/LinkTest.ts tests/unit/DeepLinkHandlerTest.tsx: 40/40 passed, including a new test for resuming a parked share onceoxfmt(formatter command was blocked in this environment). The fix is Android native-share only, so it was not verified on a device.Fixed Issues
$ #101425
PROPOSAL: #101425 (comment)
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: These must be filled out, or the issue title must include "[No QA]."
// 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 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