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.

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

[Navigation Reboot] Implement UP press #18402

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
dcd9711
Fix flatlist glitch for android (#9)
staszekscp Apr 24, 2023
3b5e279
remove close button
staszekscp Apr 25, 2023
f200f22
Merge branch 'navigation-refactor' into remove-close-button
staszekscp Apr 25, 2023
19a9513
fix linter
staszekscp Apr 26, 2023
a86fdc6
rename storybook file
staszekscp Apr 26, 2023
9aaa242
Merge branch 'navigation-refactor' into remove-close-button
staszekscp Apr 27, 2023
0a5008e
simplify code
staszekscp Apr 27, 2023
ab605b0
fixes
staszekscp Apr 27, 2023
ad65cec
fixes and improvements
staszekscp Apr 27, 2023
9b82116
remove ModalHeader
staszekscp Apr 27, 2023
8ee7d09
feat: some work done
WoLewicki Apr 28, 2023
1e456f6
move to goback
staszekscp May 2, 2023
71d6fed
add workspaces
staszekscp May 2, 2023
00f1f82
fixes
staszekscp May 4, 2023
7e0365d
fixes for yearpicker
staszekscp May 4, 2023
9748448
fix animation
staszekscp May 4, 2023
619cce2
remove some code
staszekscp May 4, 2023
d6da21d
fixes
staszekscp May 4, 2023
281a25f
chore: merge current navigation-refactor
WoLewicki May 4, 2023
b143205
feat: some changes
WoLewicki May 4, 2023
588b401
Merge branch 'navigation-refactor' into implement-up-press
WoLewicki May 4, 2023
db4430d
fix: remove unused import
WoLewicki May 4, 2023
bafd3bb
add improvements from previous pr
staszekscp May 5, 2023
44f1f1b
fix: app not loading
WoLewicki May 5, 2023
e64177d
fix for narrow screens
staszekscp May 8, 2023
6236f7f
fix: freeze on swipe
WoLewicki May 8, 2023
dcf5f75
fix: lint
WoLewicki May 8, 2023
2395ac3
fix: add patch for web too
WoLewicki May 8, 2023
fb4968f
fix patch
staszekscp May 10, 2023
5175d8b
improvements
staszekscp May 15, 2023
99214de
add prettier for easier resolving conflicts
staszekscp May 15, 2023
67a3a2c
merge after prettier
staszekscp May 15, 2023
8374575
fixes
staszekscp May 15, 2023
043994f
fix remaining lint problems
staszekscp May 15, 2023
489e5a0
Merge branch 'navigation-refactor' into implement-up-press
staszekscp May 15, 2023
eee8f27
fix
staszekscp May 15, 2023
b2293a7
fix wrong focus
staszekscp May 15, 2023
01f20aa
lint
staszekscp May 15, 2023
cc216e9
address comment issues
staszekscp May 16, 2023
bd07f59
fix
staszekscp May 17, 2023
2162b81
Merge branch 'navigation-refactor' into implement-up-press
staszekscp May 17, 2023
b49cc6d
podfile.lock update
staszekscp May 17, 2023
a9585be
fix not working back button after resize
staszekscp May 18, 2023
8a8ebbf
performance test
staszekscp May 18, 2023
e96e7d6
fix test
staszekscp May 19, 2023
450136b
fix: test from main
WoLewicki May 19, 2023
e5d8bdc
add last line to package.json
staszekscp May 19, 2023
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
6 changes: 3 additions & 3 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ EXTERNAL SOURCES:
SPEC CHECKSUMS:
Airship: c70eed50e429f97f5adb285423c7291fb7a032ae
AirshipFrameworkProxy: 2eefb77bb77b5120b0f48814b0d44439aa3ad415
boost: 57d2868c099736d80fcd648bf211b4431e51a558
boost: a7c83b31436843459a1961bfd74b96033dc77234
CocoaAsyncSocket: 065fd1e645c7abab64f7a6a2007a48038fdc6a99
DoubleConversion: 5189b271737e1565bdce30deb4a08d647e3f5f54
FBLazyVector: ff54429f0110d3c722630a98096ba689c39f6d5f
Expand Down Expand Up @@ -1062,7 +1062,7 @@ SPEC CHECKSUMS:
Permission-LocationWhenInUse: 3ba99e45c852763f730eabecec2870c2382b7bd4
Plaid: 7d340abeadb46c7aa1a91f896c5b22395a31fcf2
PromisesObjC: 09985d6d70fbe7878040aa746d78236e6946d2ef
RCT-Folly: 424b8c9a7a0b9ab2886ffe9c3b041ef628fd4fb1
RCT-Folly: 0080d0a6ebf2577475bda044aa59e2ca1f909cda
RCTRequired: e9e7b8b45aa9bedb2fdad71740adf07a7265b9be
RCTTypeSafety: 9ae0e9206625e995f0df4d5b9ddc94411929fb30
React: a71c8e1380f07e01de721ccd52bcf9c03e81867d
Expand Down Expand Up @@ -1135,4 +1135,4 @@ SPEC CHECKSUMS:

PODFILE CHECKSUM: 5feaab251246f42f41e69c8a28fa331b4899e814

COCOAPODS: 1.12.0
COCOAPODS: 1.11.3
91 changes: 91 additions & 0 deletions patches/@react-navigation+stack+6.3.16.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
diff --git a/node_modules/@react-navigation/stack/src/views/Stack/CardContainer.tsx b/node_modules/@react-navigation/stack/src/views/Stack/CardContainer.tsx
index 1e9ee0e..d85c7b4 100644
--- a/node_modules/@react-navigation/stack/src/views/Stack/CardContainer.tsx
+++ b/node_modules/@react-navigation/stack/src/views/Stack/CardContainer.tsx
@@ -105,14 +105,14 @@ function CardContainer({
const handleOpen = () => {
const { route } = scene.descriptor;

- onTransitionEnd({ route }, false);
+ onTransitionEnd({ route }, false, scene.descriptor.navigation.getState());
onOpenRoute({ route });
};

const handleClose = () => {
const { route } = scene.descriptor;

- onTransitionEnd({ route }, true);
+ onTransitionEnd({ route }, true, scene.descriptor.navigation.getState());
onCloseRoute({ route });
};

@@ -120,7 +120,7 @@ function CardContainer({
const { route } = scene.descriptor;

onPageChangeStart();
- onGestureStart({ route });
+ onGestureStart({ route }, scene.descriptor.navigation.getState());
};

const handleGestureCanceled = () => {
diff --git a/node_modules/@react-navigation/stack/src/views/Stack/StackView.tsx b/node_modules/@react-navigation/stack/src/views/Stack/StackView.tsx
index 6bbce10..7f2eed3 100644
--- a/node_modules/@react-navigation/stack/src/views/Stack/StackView.tsx
+++ b/node_modules/@react-navigation/stack/src/views/Stack/StackView.tsx
@@ -375,29 +375,51 @@ export default class StackView extends React.Component<Props, State> {

private handleTransitionStart = (
{ route }: { route: Route<string> },
- closing: boolean
- ) =>
+ closing: boolean,
+ ) => {
this.props.navigation.emit({
type: 'transitionStart',
data: { closing },
target: route.key,
});
+ }

private handleTransitionEnd = (
{ route }: { route: Route<string> },
- closing: boolean
- ) =>
+ closing: boolean,
+ state: StackNavigationState<ParamListBase>
+ ) => {
this.props.navigation.emit({
type: 'transitionEnd',
data: { closing },
target: route.key,
});
+ // Patch introduced to pass information about events to screens lower in the stack, so they could be safely frozen
+ if (state?.index > 1) {
+ this.props.navigation.emit({
+ type: 'transitionEnd',
+ data: { closing: !closing },
+ target: state.routes[state.index - 2].key,
+ });
+ }
Comment on lines +63 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add comment what's this for. Is this known issue in upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not really a bug that would be fixed in react-navigation, but it has to be introduced specifically for the architecture of nested navigators and usage of Freeze component in the Expensify app. In here we just make sure that the second to last screen in the stack gets information that it cannot be seen. It is necessary, because otherwise a frozen screen could be seen below ie. while using the swipe gesture on iOS.

+ }

- private handleGestureStart = ({ route }: { route: Route<string> }) => {
+ private handleGestureStart = (
+ { route }: { route: Route<string> },
+ state: StackNavigationState<ParamListBase>
+ ) => {
+ console.log(state)
this.props.navigation.emit({
type: 'gestureStart',
target: route.key,
});
+ // Patch introduced to pass information about events to screens lower in the stack, so they could be safely frozen
+ if (state?.index > 1) {
+ this.props.navigation.emit({
+ type: 'gestureStart',
+ target: state.routes[state.index - 2].key,
+ });
+ }
Comment on lines +83 to +88
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Is there PR raised in upstream?

};

private handleGestureEnd = ({ route }: { route: Route<string> }) => {
7 changes: 5 additions & 2 deletions src/components/AttachmentModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import themeColors from '../styles/themes/default';
import compose from '../libs/compose';
import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions';
import Button from './Button';
import HeaderWithCloseButton from './HeaderWithCloseButton';
import HeaderWithBackButton from './HeaderWithBackButton';
import fileDownload from '../libs/fileDownload';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import ConfirmModal from './ConfirmModal';
Expand Down Expand Up @@ -270,11 +270,14 @@ class AttachmentModal extends PureComponent {
propagateSwipe
>
{this.props.isSmallScreenWidth && <HeaderGap />}
<HeaderWithCloseButton
<HeaderWithBackButton
title={this.props.headerTitle || this.props.translate('common.attachment')}
shouldShowBorderBottom
shouldShowDownloadButton={this.props.allowDownload}
onDownloadButtonPress={() => this.downloadAttachment(source)}
shouldShowCloseButton={!this.props.isSmallScreenWidth}
shouldShowBackButton={this.props.isSmallScreenWidth}
onBackButtonPress={() => this.setState({isModalOpen: false})}
onCloseButtonPress={() => this.setState({isModalOpen: false})}
/>
<View style={styles.imageModalImageCenterContainer}>
Expand Down
6 changes: 3 additions & 3 deletions src/components/AvatarCropModal/AvatarCropModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import compose from '../../libs/compose';
import styles from '../../styles/styles';
import themeColors from '../../styles/themes/default';
import Button from '../Button';
import HeaderWithCloseButton from '../HeaderWithCloseButton';
import HeaderWithBackButton from '../HeaderWithBackButton';
import Icon from '../Icon';
import * as Expensicons from '../Icon/Expensicons';
import Modal from '../Modal';
Expand Down Expand Up @@ -361,9 +361,9 @@ const AvatarCropModal = (props) => {
onModalHide={resetState}
>
{props.isSmallScreenWidth && <HeaderGap />}
<HeaderWithCloseButton
<HeaderWithBackButton
title={props.translate('avatarCropModal.title')}
onCloseButtonPress={props.onClose}
onBackButtonPress={props.onClose}
/>
<Text style={[styles.mh5]}>{props.translate('avatarCropModal.description')}</Text>
<GestureHandlerRootView
Expand Down
2 changes: 1 addition & 1 deletion src/components/BlockingViews/BlockingView.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const BlockingView = (props) => (
<Text style={[styles.textAlignCenter]}>{props.subtitle}</Text>
{props.shouldShowBackHomeLink ? (
<TextLink
onPress={() => Navigation.dismissModal(true)}
onPress={Navigation.goBack}
style={[styles.link, styles.mt2]}
>
{props.link}
Expand Down
19 changes: 3 additions & 16 deletions src/components/BlockingViews/FullPageNotFoundView.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {View} from 'react-native';
import BlockingView from './BlockingView';
import * as Illustrations from '../Icon/Illustrations';
import withLocalize, {withLocalizePropTypes} from '../withLocalize';
import HeaderWithCloseButton from '../HeaderWithCloseButton';
import HeaderWithBackButton from '../HeaderWithBackButton';
import Navigation from '../../libs/Navigation/Navigation';
import variables from '../../styles/variables';
import styles from '../../styles/styles';
Expand All @@ -25,12 +25,6 @@ const propTypes = {
/** The key in the translations file to use for the subtitle */
subtitleKey: PropTypes.string,

/** Whether we should show a back icon */
shouldShowBackButton: PropTypes.bool,

/** Whether we should show a close button */
shouldShowCloseButton: PropTypes.bool,

/** Whether we should show a go back home link */
shouldShowBackHomeLink: PropTypes.bool,

Expand All @@ -47,23 +41,16 @@ const defaultProps = {
titleKey: 'notFound.notHere',
subtitleKey: 'notFound.pageNotFound',
linkKey: 'notFound.goBackHome',
shouldShowBackButton: true,
shouldShowBackHomeLink: false,
shouldShowCloseButton: true,
onBackButtonPress: () => Navigation.dismissModal(),
onBackButtonPress: Navigation.goBack,
};

// eslint-disable-next-line rulesdir/no-negated-variables
const FullPageNotFoundView = (props) => {
if (props.shouldShow) {
return (
<>
<HeaderWithCloseButton
shouldShowBackButton={props.shouldShowBackButton}
Copy link
Contributor

Choose a reason for hiding this comment

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

Removal of shouldShowBackButton in this PR caused this issue.

Issue: Back Button is shown on desktop in Report Screen

shouldShowCloseButton={props.shouldShowCloseButton}
onBackButtonPress={props.onBackButtonPress}
onCloseButtonPress={() => Navigation.dismissModal()}
/>
<HeaderWithBackButton onBackButtonPress={props.onBackButtonPress} />
<View style={[styles.flex1, styles.blockingViewContainer]}>
<BlockingView
icon={Illustrations.ToddBehindCloud}
Expand Down
53 changes: 20 additions & 33 deletions src/components/FlatList/index.android.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, {forwardRef} from 'react';
import React, {forwardRef, useCallback, useState} from 'react';
import {FlatList} from 'react-native';
import PropTypes from 'prop-types';
import {useFocusEffect} from '@react-navigation/native';

const propTypes = {
/** Same as for FlatList */
Expand Down Expand Up @@ -31,56 +32,42 @@ const defaultProps = {
// FlatList wrapped with the freeze component will lose its scroll state when frozen (only for Android).
// CustomFlatList saves the offset and use it for scrollToOffset() when unfrozen.
function CustomFlatList(props) {
const contentOffsetRef = React.useRef(null);
const isHidden = React.useRef(false);
const [ready, setReady] = React.useState(true);
const [scrollPosition, setScrollPosition] = useState({});

const handleOnScroll = (event) => {
props.onScroll(event);

// The last onScroll event happens after freezing the FlatList and it's called with offset: 0.
// Don't save this value because it's incorrect.
if (!isHidden.current) {
contentOffsetRef.current = event.nativeEvent.contentOffset;
const onScreenFocus = useCallback(() => {
if (!props.innerRef.current || !scrollPosition.offset) {
return;
}
};
const handleOnLayout = (event) => {
props.onLayout(event);

if (event.nativeEvent.layout.height === 0) {
// If the layout height is equal to 0, we can assume that this flatList is frozen.
isHidden.current = true;

// The maintainVisibleContentPosition prop causes glitches with animations and scrollToOffset.
// Use ready state to decide if this prop should be undefined to avoid glitching.
setReady(false);
} else {
isHidden.current = false;
if (props.innerRef.current && contentOffsetRef.current) {
props.innerRef.current.scrollToOffset({offset: contentOffsetRef.current.y, animated: false});
setReady(true);
}
if (props.innerRef.current && scrollPosition.offset) {
props.innerRef.current.scrollToOffset({offset: scrollPosition.offset, animated: false});
}
};
}, [scrollPosition.offset, props.innerRef]);

useFocusEffect(
useCallback(() => {
onScreenFocus();
}, [onScreenFocus]),
);

return (
<FlatList
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
onScroll={(event) => props.onScroll(event)}
onMomentumScrollEnd={(event) => {
setScrollPosition({offset: event.nativeEvent.contentOffset.y});
}}
ref={props.innerRef}
onScroll={handleOnScroll}
onLayout={handleOnLayout}
maintainVisibleContentPosition={ready ? props.maintainVisibleContentPosition : undefined}
/>
);
}

CustomFlatList.propTypes = propTypes;
CustomFlatList.defaultProps = defaultProps;

// eslint-disable-next-line react/jsx-props-no-spreading
export default forwardRef((props, ref) => (
<CustomFlatList
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
innerRef={ref}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ const propTypes = {
/** Method to trigger when pressing more options button of the header */
onThreeDotsButtonPress: PropTypes.func,

/** Whether we should show a back icon */
shouldShowBackButton: PropTypes.bool,

/** Whether we should show a border on the bottom of the Header */
shouldShowBorderBottom: PropTypes.bool,

Expand All @@ -67,6 +64,9 @@ const propTypes = {
/** Whether we should show a close button */
shouldShowCloseButton: PropTypes.bool,

/** Whether we should show a back button */
shouldShowBackButton: PropTypes.bool,

/** Whether we should show the step counter */
shouldShowStepCounter: PropTypes.bool,

Expand Down Expand Up @@ -103,16 +103,16 @@ const defaultProps = {
title: '',
subtitle: '',
onDownloadButtonPress: () => {},
onCloseButtonPress: () => {},
onBackButtonPress: () => {},
onBackButtonPress: Navigation.goBack,
onCloseButtonPress: Navigation.dismissModal,
onThreeDotsButtonPress: () => {},
shouldShowBackButton: false,
shouldShowBorderBottom: false,
shouldShowDownloadButton: false,
shouldShowGetAssistanceButton: false,
shouldShowThreeDotsButton: false,
shouldShowCloseButton: true,
shouldShowStepCounter: true,
shouldShowCloseButton: false,
shouldShowBackButton: true,
shouldShowAvatarWithDisplay: false,
report: null,
policies: {},
Expand All @@ -126,7 +126,7 @@ const defaultProps = {
},
};

class HeaderWithCloseButton extends Component {
class HeaderWithBackButton extends Component {
constructor(props) {
super(props);

Expand Down Expand Up @@ -238,7 +238,7 @@ class HeaderWithCloseButton extends Component {
}
}

HeaderWithCloseButton.propTypes = propTypes;
HeaderWithCloseButton.defaultProps = defaultProps;
HeaderWithBackButton.propTypes = propTypes;
HeaderWithBackButton.defaultProps = defaultProps;

export default compose(withLocalize, withDelayToggleButtonState, withKeyboardState)(HeaderWithCloseButton);
export default compose(withLocalize, withDelayToggleButtonState, withKeyboardState)(HeaderWithBackButton);
6 changes: 4 additions & 2 deletions src/components/KeyboardShortcutsModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
import {View, ScrollView} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import HeaderWithCloseButton from './HeaderWithCloseButton';
import HeaderWithBackButton from './HeaderWithBackButton';
import Text from './Text';
import Modal from './Modal';
import CONST from '../CONST';
Expand Down Expand Up @@ -159,8 +159,10 @@ class KeyboardShortcutsModal extends React.Component {
innerContainerStyle={{...styles.keyboardShortcutModalContainer, ...StyleUtils.getKeyboardShortcutsModalWidth(this.props.isSmallScreenWidth)}}
onClose={KeyboardShortcutsActions.hideKeyboardShortcutModal}
>
<HeaderWithCloseButton
<HeaderWithBackButton
title={this.props.translate('keyboardShortcutModal.title')}
shouldShowCloseButton
shouldShowBackButton={false}
onCloseButtonPress={KeyboardShortcutsActions.hideKeyboardShortcutModal}
/>
<ScrollView style={[styles.p5, styles.pt0]}>
Expand Down
Loading