-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Prevent Lottie from running in the background after navigating to other pages #48444
Merged
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
fd10e5b
Prevent Lottie from running in the background after navigating to oth…
QichenZhu b0d577f
Merge remote-tracking branch 'upstream/main' into fix/47273-2nd-try
QichenZhu fc9fa94
Rename isHidden to hasNavigatedAway
QichenZhu b79df74
Check the platform
QichenZhu 3bc4b18
Fix lint errors
QichenZhu 7f5a158
Add useEffect dependencies
QichenZhu 2e03f4b
Merge remote-tracking branch 'upstream/main' into fix/47273-2nd-try
QichenZhu 10af7f9
Exclude left side modal
QichenZhu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,13 +1,16 @@ | ||||||
import {NavigationContainerRefContext, NavigationContext} from '@react-navigation/native'; | ||||||
import type {AnimationObject, LottieViewProps} from 'lottie-react-native'; | ||||||
import LottieView from 'lottie-react-native'; | ||||||
import type {ForwardedRef} from 'react'; | ||||||
import React, {forwardRef, useEffect, useState} from 'react'; | ||||||
import React, {forwardRef, useContext, useEffect, useState} from 'react'; | ||||||
import {View} from 'react-native'; | ||||||
import type DotLottieAnimation from '@components/LottieAnimations/types'; | ||||||
import useAppState from '@hooks/useAppState'; | ||||||
import useNetwork from '@hooks/useNetwork'; | ||||||
import useThemeStyles from '@hooks/useThemeStyles'; | ||||||
import * as Browser from '@libs/Browser'; | ||||||
import CONST from '@src/CONST'; | ||||||
import NAVIGATORS from '@src/NAVIGATORS'; | ||||||
import {useSplashScreenStateContext} from '@src/SplashScreenStateContext'; | ||||||
|
||||||
type Props = { | ||||||
|
@@ -30,11 +33,40 @@ function Lottie({source, webStyle, ...props}: Props, ref: ForwardedRef<LottieVie | |||||
|
||||||
const aspectRatioStyle = styles.aspectRatioLottie(source); | ||||||
|
||||||
// If the image fails to load, app is in background state, animation file isn't ready, or the splash screen isn't hidden yet, | ||||||
const browser = Browser.getBrowser(); | ||||||
const [hasNavigatedAway, setHasNavigatedAway] = React.useState(false); | ||||||
const navigationContainerRef = useContext(NavigationContainerRefContext); | ||||||
const navigator = useContext(NavigationContext); | ||||||
|
||||||
useEffect(() => { | ||||||
if (!browser || !navigationContainerRef || !navigator) { | ||||||
return; | ||||||
} | ||||||
const unsubscribeNavigationFocus = navigator.addListener('focus', () => { | ||||||
setHasNavigatedAway(false); | ||||||
}); | ||||||
return unsubscribeNavigationFocus; | ||||||
}, [browser, navigationContainerRef, navigator]); | ||||||
|
||||||
useEffect(() => { | ||||||
if (!browser || !navigationContainerRef || !navigator) { | ||||||
return; | ||||||
} | ||||||
const unsubscribeNavigationBlur = navigator.addListener('blur', () => { | ||||||
const state = navigationContainerRef.getRootState(); | ||||||
const targetRouteName = state?.routes?.[state?.index ?? 0]?.name; | ||||||
if (targetRouteName !== NAVIGATORS.RIGHT_MODAL_NAVIGATOR) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That will be better. Thanks for pointing it out! |
||||||
setHasNavigatedAway(true); | ||||||
} | ||||||
}); | ||||||
return unsubscribeNavigationBlur; | ||||||
}, [browser, navigationContainerRef, navigator]); | ||||||
|
||||||
// If the page navigates to another screen, the image fails to load, app is in background state, animation file isn't ready, or the splash screen isn't hidden yet, | ||||||
// we'll just render an empty view as the fallback to prevent | ||||||
// 1. memory leak, see issue: https://github.com/Expensify/App/issues/36645 | ||||||
// 2. heavy rendering, see issue: https://github.com/Expensify/App/issues/34696 | ||||||
if (isError || appState.isBackground || !animationFile || splashScreenState !== CONST.BOOT_SPLASH_STATE.HIDDEN) { | ||||||
// 2. heavy rendering, see issues: https://github.com/Expensify/App/issues/34696 and https://github.com/Expensify/App/issues/47273 | ||||||
if (hasNavigatedAway || isError || appState.isBackground || !animationFile || splashScreenState !== CONST.BOOT_SPLASH_STATE.HIDDEN) { | ||||||
return <View style={[aspectRatioStyle, props.style]} />; | ||||||
} | ||||||
|
||||||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we need to check browser?
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.
Because the issue only occurs in browsers, it seems unnecessary to apply this fix to native platforms.
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.
OK, let change the PR status