-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
perf: Freeze screens and unmount Browser and Transactions screens when unfocused #15246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
perf: Freeze screens and unmount Browser and Transactions screens when unfocused #15246
Conversation
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Let's also provide some vids on before and after showing that the screens are frozen
@@ -1,5 +1,8 @@ | |||
import './shim.js'; | |||
|
|||
import { enableFreeze } from 'react-native-screens'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're on React Navigation v5, it looks like we also need to enable screens via enableScreens(true)
and then passing detachInactiveScreens
option to stack / tabs / etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -508,6 +508,7 @@ const HomeTabs = () => { | |||
); | |||
}, | |||
rootScreenName: Routes.BROWSER_VIEW, | |||
unmountOnBlur: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since React Navigation v6 removes this option, let's go with the useIsFocus
implementation for now since that will still work once we upgrade to v6 - https://reactnavigation.org/docs/upgrading-from-6.x/#the-unmountonblur-option-is-removed-in-favor-of-poptotoponblur-in-bottom-tab-navigator-and-drawer-navigator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point Cal.
I like the high order component proposal presented on the docs
function UnmountOnBlur({ children }) {
const isFocused = useIsFocused();
if (!isFocused) {
return null;
}
return children;
}
Then we can use it in the layout
prop of the screen
<Tab.Screen
name={Routes.BROWSER.HOME}
options={options.browser}
component={BrowserFlow}
layout={({ children }) => <UnmountOnBlur>{children}</UnmountOnBlur>}
/>
@andrepimenta Let's also check if this breaks deeplink behavior cc @tommasini |
Description
This PR improves the performance of the app by:
Note: Unmounting the Browser also fixes this Browser issue: https://github.com/MetaMask/mobile-planning/issues/1247
Related issues
Fixes: https://github.com/MetaMask/mobile-planning/issues/1247
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist