Skip to content

Commit

Permalink
[native] Fix crash in NUX
Browse files Browse the repository at this point in the history
Summary:
issue: [[https://linear.app/comm/issue/ENG-9375/button-should-be-registered-with-nuxtipcontext-error | ENG-9375]]
If a dev kills the app while it is showing the NUX, then after they reopen they will get this error: "button should be registered with nuxTipContext"
This happens because for dev we remember the navigation state. When the navigation state is rehydrated, the buttons have not yet been mounted, so they are not registered with the context, resulting in the error.
Because this is for dev only, and because we expect the NUX tips to only be shown at the start of the app over the chat list, I think we should just reset the nav state if NUX tips are present.

Test Plan:
Created a new user -> saw the NUX tips -> navigated to a tip that points to a button -> closed the app -> opened the app
Before this resulted in an error on the screen. After this change the error doesn't appear. The user sees the chat list.

Reviewers: tomek, kamil, ashoat

Reviewed By: ashoat

Subscribers: ashoat

Differential Revision: https://phab.comm.dev/D13520
  • Loading branch information
InkaAlicja committed Oct 1, 2024
1 parent 369199f commit d17d71b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
18 changes: 18 additions & 0 deletions native/navigation/navigation-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
import invariant from 'invariant';

import {
NUXTipOverlayBackdropRouteName,
ComposeSubchannelRouteName,
AppRouteName,
threadRoutes,
Expand Down Expand Up @@ -167,6 +168,22 @@ function getChildRouteFromNavigatorRoute(
return childRoute;
}

// isShowingNUXTips assumes nav state is valid
function isShowingNUXTips(state: PossiblyStaleNavigationState): boolean {
const [appRoute] = state.routes;
const appSubroutes = appRoute?.state?.routes;
if (!appSubroutes) {
return false;
}
for (const route of appSubroutes) {
if (route.name === NUXTipOverlayBackdropRouteName) {
return true;
}
}

return false;
}

export {
getStateFromNavigatorRoute,
getThreadIDFromParams,
Expand All @@ -176,4 +193,5 @@ export {
removeScreensFromStack,
validNavState,
getChildRouteFromNavigatorRoute,
isShowingNUXTips,
};
7 changes: 5 additions & 2 deletions native/root.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ import {
type NavContextType,
} from './navigation/navigation-context.js';
import NavigationHandler from './navigation/navigation-handler.react.js';
import { validNavState } from './navigation/navigation-utils.js';
import {
validNavState,
isShowingNUXTips,
} from './navigation/navigation-utils.js';
import OrientationHandler from './navigation/orientation-handler.react.js';
import { navStateAsyncStorageKey } from './navigation/persistance.js';
import RootNavigator from './navigation/root-navigator.react.js';
Expand Down Expand Up @@ -158,7 +161,7 @@ function Root() {
);
if (navStateString) {
const savedState = JSON.parse(navStateString);
if (validNavState(savedState)) {
if (validNavState(savedState) && !isShowingNUXTips(savedState)) {
loadedState = savedState;
}
}
Expand Down

0 comments on commit d17d71b

Please sign in to comment.