Skip to content
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.

Sign up for GitHub

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

Jump to bottom

iOS - Login - The splash screen glitches and flashes on launch #35185

Merged
merged 6 commits into from
Feb 13, 2024
Merged

iOS - Login - The splash screen glitches and flashes on launch #35185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c512dd0
iOS - Login - The splash screen glitches and flashes on launch
ikevin127 Feb 13, 2024
f8d689d
Merge branch 'main' of https://github.com/ikevin127/Expensify into fi…
ikevin127 Feb 13, 2024
2f9aadf
fixed reassure tests
ikevin127 Feb 13, 2024
0186486
Merge branch 'main' of https://github.com/Expensify/App into fix/34696
ikevin127 Feb 13, 2024
cae96fa
reasurre tests fix - optional chaining
ikevin127 Feb 13, 2024
b3be5cd
Merge branch 'main' of https://github.com/Expensify/App into fix/34696
ikevin127 Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/hooks/useIsSplashHidden.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {useContext} from 'react';
import {SplashScreenHiddenContext} from '@src/Expensify';

type SplashScreenHiddenContextType = {isSplashHidden: boolean};

export default function useIsSplashHidden() {
const {isSplashHidden} = useContext(SplashScreenHiddenContext) as SplashScreenHiddenContextType;
return isSplashHidden;
}

export type {SplashScreenHiddenContextType};
2 changes: 1 addition & 1 deletion src/libs/Navigation/getTopmostReportId.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function getTopmostReportId(state: NavigationState | NavigationState<RootStackPa
return;
}

const topmostCentralPane = state.routes.filter((route) => route.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR).at(-1);
const topmostCentralPane = state.routes?.filter((route) => route.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR).at(-1);
if (!topmostCentralPane) {
return;
}
Expand Down
10 changes: 10 additions & 0 deletions src/pages/signin/SignInHeroImage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import PropTypes from 'prop-types';
import React from 'react';
import {View} from 'react-native';
import Lottie from '@components/Lottie';
import LottieAnimations from '@components/LottieAnimations';
import withWindowDimensions, {windowDimensionsPropTypes} from '@components/withWindowDimensions';
import useIsSplashHidden from '@hooks/useIsSplashHidden';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';

Expand Down Expand Up @@ -36,6 +38,14 @@ function SignInHeroImage(props) {
};
}

const isSplashHidden = useIsSplashHidden();
// Prevents rendering of the Lottie animation until the splash screen is hidden
// by returning an empty view of the same size as the animation.
// See issue: https://github.com/Expensify/App/issues/34696
if (!isSplashHidden) {
return <View style={[styles.alignSelfCenter, imageSize]} />;
}

return (
<Lottie
source={LottieAnimations.Hands}
Expand Down
34 changes: 29 additions & 5 deletions src/pages/signin/SignInPageLayout/BackgroundImage/index.ios.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import {Image} from 'expo-image';
import PropTypes from 'prop-types';
import React, {useMemo} from 'react';
import Reanimated, {Easing, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import DesktopBackgroundImage from '@assets/images/home-background--desktop.svg';
import MobileBackgroundImage from '@assets/images/home-background--mobile-new.svg';
import useIsSplashHidden from '@hooks/useIsSplashHidden';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import defaultPropTypes from './propTypes';

const defaultProps = {
Expand All @@ -21,12 +24,33 @@ function BackgroundImage(props) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const src = useMemo(() => (props.isSmallScreen ? MobileBackgroundImage : DesktopBackgroundImage), [props.isSmallScreen]);

const opacity = useSharedValue(0);
const animatedStyle = useAnimatedStyle(() => ({opacity: opacity.value}));
// This sets the opacity animation for the background image once it has loaded.
function setOpacityAnimation() {
opacity.value = withTiming(1, {
duration: CONST.MICROSECONDS_PER_MS,
easing: Easing.ease,
});
}

const isSplashHidden = useIsSplashHidden();
// Prevent rendering the background image until the splash screen is hidden.
// See issue: https://github.com/Expensify/App/issues/34696
if (!isSplashHidden) {
return;
}

return (
<Image
source={src}
style={[styles.signInBackground, StyleUtils.getWidthStyle(props.width)]}
transition={props.transitionDuration}
/>
<Reanimated.View style={[styles.signInBackground, StyleUtils.getWidthStyle(props.width), animatedStyle]}>
<Image
source={src}
onLoadEnd={() => setOpacityAnimation()}
style={[styles.signInBackground, StyleUtils.getWidthStyle(props.width)]}
transition={props.transitionDuration}
/>
</Reanimated.View>
);
}

Expand Down
1 change: 1 addition & 0 deletions tests/perf-test/SignInPage.perf-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jest.mock('@react-navigation/native', () => {
removeListener: () => jest.fn(),
isReady: () => jest.fn(),
getCurrentRoute: () => jest.fn(),
getState: () => jest.fn(),
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

perf tests failing if this was added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More context on Slack: https://expensify.slack.com/archives/C01GTK53T8Q/p1706732738472299

SignInPage reassure test was failing with this error:

  ● SignInPage › [SignInPage] should add username and click continue button

    TypeError: _Navigation.navigationRef.isReady is not a function

      37 |
      38 | const triggerUnreadUpdate = () => {
    > 39 |     const currentReportID = navigationRef.isReady() ? Navigation.getTopmostReportId() ?? '' : '';
         |                                           ^
      40 |
      41 |     // We want to keep notification count consistent with what can be accessed from the LHN list
      42 |     const unreadReports = getUnreadReportsForUnreadIndicator(allReports, currentReportID);

And I was told to remove this part as:

I recently changed it to solve another issue, but it looks like we don't need it at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On recent sync w/ main the createNavigationContainerRef was added back, but tests on this PR were still failing since the navigationRef.getState() mock was missing, I added the mock and they still failed because of this line:

App/src/libs/Navigation/getTopmostReportId.ts

Line 19 in 3a702ce

const topmostCentralPane = state.routes.filter((route) => route.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR).at(-1);

where state.routes was undefined and .filter was failing. I added optional chaining there and they pass now.

} as typeof NativeNavigation;
});
Expand Down
Loading