Skip to content

Add type-safety checks in navigation functions#5478

Merged
roryabraham merged 1 commit into
mainfrom
Rory-NavigationTypeSafety
Sep 24, 2021
Merged

Add type-safety checks in navigation functions#5478
roryabraham merged 1 commit into
mainfrom
Rory-NavigationTypeSafety

Conversation

@roryabraham

Copy link
Copy Markdown
Contributor

Details

Makes sure that the navigationRef is ready before trying to access current.

Fixed Issues

Slack threads:

Tests / QA Steps

  1. Go to https://staging.expensify.com/ on a mobile device.
  2. Sign into an account.
  3. Press Set up my company for free
  4. Verify that you are taken to the workspace modal in NewDot (no white screen or infinite loading spinner). If you did not already have a workspace for this account, a new one should be created. Otherwise, a new workspace should not be created.
  5. Sign out from NewDot.
  6. Go to https://staging.expensify.com/ on a mobile device.
  7. Sign into an account.
  8. Press Split bills & chat with friends
  9. Verify that you are taken to the NewDot homepage without any issues.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@roryabraham roryabraham self-assigned this Sep 23, 2021
@roryabraham roryabraham requested a review from a team as a code owner September 23, 2021 23:40
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@MelvinBot MelvinBot requested review from mountiny and removed request for a team September 23, 2021 23:40
@roryabraham

Copy link
Copy Markdown
Contributor Author

Note: due to this bug, we're unable to successfully defer these navigation events to happen after the navigation ref is ready. So a side-effect of this is that the workspace modal will not be consistently opened when routing to NewDot from OldDot via Set up my company for free.

However, in the events when we previously had a white-screen error, we'll now just take the user to the NewDot homepage instead of crashing the app. Seems like an improvement to me.

@roryabraham

Copy link
Copy Markdown
Contributor Author

Actually we don't need to CP Staging because we've got an unlocked checklist atm.

@luacmartins luacmartins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@roryabraham roryabraham merged commit 3be5cfc into main Sep 24, 2021
@roryabraham roryabraham deleted the Rory-NavigationTypeSafety branch September 24, 2021 00:57
@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.1.1-10 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @timszot in version: 1.1.2-7 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants