From 3e91a327acda23c27c2f8a4e1cbe140ca7d1f493 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 2 Apr 2021 22:10:31 +0300 Subject: [PATCH 01/46] refactor: convert ReportScreen to class component --- src/pages/home/ReportScreen.js | 119 +++++++++++++++++++++++++-------- 1 file changed, 90 insertions(+), 29 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 4689b41c95e8..ca435c0bfb75 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -9,46 +9,107 @@ import HeaderView from './HeaderView'; import Navigation from '../../libs/Navigation/Navigation'; import ROUTES from '../../ROUTES'; import ONYXKEYS from '../../ONYXKEYS'; +import {fetchActions} from '../../libs/actions/Report'; const propTypes = { - // id of the most recently viewed report - currentlyViewedReportID: PropTypes.string, + /* The ID of the report this screen should display */ + reportID: PropTypes.string.isRequired, }; -const defaultProps = { - currentlyViewedReportID: '0', -}; +class ReportScreen extends React.Component { + constructor(props) { + super(props); -const ReportScreen = (props) => { - const activeReportID = parseInt(props.currentlyViewedReportID, 10); - if (!activeReportID) { - return null; + this.state = { + isLoading: true, + error: null, + drawerOpen: false, + }; } - return ( - - Navigation.navigate(ROUTES.HOME)} - /> - - - - - ); -}; + componentDidMount() { + if (this.reportID) { + this.fetchReport(); + } + } + + componentDidUpdate(prevProps) { + // Reports changed, reset and load new data + if (this.props.reportID !== prevProps.reportID) { + if (this.reportID) { + this.fetchReport(); + } + } + } + + // Todo: ask why getters aren't on top? + get canRenderMainContent() { + return !this.state.isLoading && !this.state.error && !this.state.drawerOpen; + } + + get reportID() { + return parseInt(this.props.reportID, 10); + } + + fetchReport() { + console.debug('[ReportScreen] Fetch started: '); + this.setState({isLoading: true, error: null}); + + /* Todo: it might be good if this method resolves with needed data here + * when online resolve with data from the server + * when offline resolve with local data + * E.cash strategy is optimistic response though. Maybe fetchAction can internally + * resolve instantly when it can (some onyx data available) and then + * push an update to onyx data after actual data is fetched + * Another issue for optimistic loading - when data is available + * the Chat will try to render immediately which is what's causing the + * transition issues in the first place */ + fetchActions(this.reportID) + .catch((error) => { + // Todo: what should we do if fetching fails for some reason + // Todo: see what we currently do and do the same here + this.setState({error}); + console.error(error); + }) + .finally(() => { + console.debug('[ReportScreen] Fetch complete: '); + this.setState({isLoading: false}); + }); + } + + render() { + const activeReportID = parseInt(this.props.reportID, 10); + + return ( + + Navigation.navigate(ROUTES.HOME)} + /> + { + this.canRenderMainContent + ? ( + + + + ) + : null // Todo: add loader here :) + } + + ); + } +} ReportScreen.displayName = 'ReportScreen'; ReportScreen.propTypes = propTypes; -ReportScreen.defaultProps = defaultProps; export default withOnyx({ - currentlyViewedReportID: { + reportID: { key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, }, })(ReportScreen); From 455a707c28cbf809b7cbe35b11bcfed5f716490e Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 2 Apr 2021 22:16:26 +0300 Subject: [PATCH 02/46] refactor: ReportScreen added drawer state This should be moved to HOC though --- src/pages/home/ReportScreen.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index ca435c0bfb75..d1811dbfb73d 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -14,6 +14,12 @@ import {fetchActions} from '../../libs/actions/Report'; const propTypes = { /* The ID of the report this screen should display */ reportID: PropTypes.string.isRequired, + + /* Todo: extract to `withDrawer` HOC */ + navigation: PropTypes.shape({ + addListener: PropTypes.func.isRequired, + }).isRequired, + }; class ReportScreen extends React.Component { @@ -31,6 +37,16 @@ class ReportScreen extends React.Component { if (this.reportID) { this.fetchReport(); } + + // Todo: extract to `withDrawer` HOC + this.openListener = this.props.navigation.addListener('drawerOpen', () => { + this.setState({drawerOpen: true}); + }); + + // Todo: extract to `withDrawer` HOC + this.closeListener = this.props.navigation.addListener('drawerClose', () => { + this.setState({drawerOpen: false}); + }); } componentDidUpdate(prevProps) { @@ -42,6 +58,11 @@ class ReportScreen extends React.Component { } } + componentWillUnmount() { + this.openListener(); + this.closeListener(); + } + // Todo: ask why getters aren't on top? get canRenderMainContent() { return !this.state.isLoading && !this.state.error && !this.state.drawerOpen; From 53b65f9ce9bb98c9dde80bf768d99e5452549abf Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 2 Apr 2021 23:11:39 +0300 Subject: [PATCH 03/46] feat: Loading components --- src/components/Loading/FullscreenLoading.js | 21 ++++ src/components/Loading/LoadingIndicator.js | 100 ++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 src/components/Loading/FullscreenLoading.js create mode 100644 src/components/Loading/LoadingIndicator.js diff --git a/src/components/Loading/FullscreenLoading.js b/src/components/Loading/FullscreenLoading.js new file mode 100644 index 000000000000..d274b7540bc5 --- /dev/null +++ b/src/components/Loading/FullscreenLoading.js @@ -0,0 +1,21 @@ +import React from 'react'; +import {StyleSheet, View} from 'react-native'; +import LoadingIndicator from './LoadingIndicator'; + +const FullScreenLoadingIndicator = () => ( + + + +); + +const styles = StyleSheet.create({ + container: { + ...StyleSheet.absoluteFillObject, + justifyContent: 'center', + alignItems: 'center', + backgroundColor: '#eeeeeecc', + zIndex: 10, + }, +}); + +export default FullScreenLoadingIndicator; diff --git a/src/components/Loading/LoadingIndicator.js b/src/components/Loading/LoadingIndicator.js new file mode 100644 index 000000000000..f64a0c4b7075 --- /dev/null +++ b/src/components/Loading/LoadingIndicator.js @@ -0,0 +1,100 @@ +import React, {Component} from 'react'; +import PropTypes from 'prop-types'; +import { + Animated, + Easing, + View, + StyleSheet, +} from 'react-native'; +import Svg, {Circle} from 'react-native-svg'; + +const propTypes = { + useNativeDriver: PropTypes.bool, + size: PropTypes.number, +}; + +const defaultProps = { + useNativeDriver: true, + size: 40, +}; + +class LoadingIndicator extends Component { + constructor(props) { + super(props); + + this.spinValue = new Animated.Value(0); + + this.spinTransform = this.spinValue.interpolate({ + inputRange: [0, 1], + outputRange: ['0deg', '360deg'], + }); + + this.circleProps = { + style: StyleSheet.absoluteFillObject, + cx: 0, + cy: 0, + r: 40, + fill: 'transparent', + stroke: 'rgb(142, 152, 158)', + strokeWidth: 8, + strokeLinecap: 'butt', + strokeDasharray: 0, + strokeDashoffset: 0, + }; + } + + componentDidMount() { + this.spin(); + } + + spin() { + Animated.loop( + Animated.timing( + this.spinValue, + { + toValue: 1, + duration: 690, + easing: Easing.linear, + useNativeDriver: this.props.useNativeDriver, + }, + ), + ).start(); + } + + render() { + const size = this.props.size; + + return ( + + + + + + + + + ); + } +} + +const styles = StyleSheet.create({ + container: { + alignItems: 'center', + justifyContent: 'center', + }, +}); + +LoadingIndicator.propTypes = propTypes; +LoadingIndicator.defaultProps = defaultProps; + +export default LoadingIndicator; From 339559395c8cd2c029fcb28888a4a18eb10926e3 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Fri, 2 Apr 2021 23:50:18 +0300 Subject: [PATCH 04/46] feat: ReportScreen add loading animation for content --- src/pages/home/ReportScreen.js | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index d1811dbfb73d..9291f9a8bd5a 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -5,11 +5,12 @@ import {View} from 'react-native'; import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; +import FullScreenLoadingIndicator from '../../components/Loading/FullscreenLoading'; import HeaderView from './HeaderView'; import Navigation from '../../libs/Navigation/Navigation'; +import {fetchActions} from '../../libs/actions/Report'; import ROUTES from '../../ROUTES'; import ONYXKEYS from '../../ONYXKEYS'; -import {fetchActions} from '../../libs/actions/Report'; const propTypes = { /* The ID of the report this screen should display */ @@ -29,7 +30,7 @@ class ReportScreen extends React.Component { this.state = { isLoading: true, error: null, - drawerOpen: false, + drawerOpen: true, }; } @@ -74,7 +75,10 @@ class ReportScreen extends React.Component { fetchReport() { console.debug('[ReportScreen] Fetch started: '); - this.setState({isLoading: true, error: null}); + this.setState({ + isLoading: true, + error: null, + }); /* Todo: it might be good if this method resolves with needed data here * when online resolve with data from the server @@ -113,15 +117,14 @@ class ReportScreen extends React.Component { reportID={activeReportID} onNavigationMenuButtonClicked={() => Navigation.navigate(ROUTES.HOME)} /> - { - this.canRenderMainContent - ? ( - - - - ) - : null // Todo: add loader here :) - } + + + { + this.canRenderMainContent + ? + : + } + ); } From 0c30b0645dca89bd45780879dbdafbbf48e5190b Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 01:21:48 +0300 Subject: [PATCH 05/46] refactor: ReportScreen move loading animation to ReportView --- src/pages/home/ReportScreen.js | 28 ++++++------------ src/pages/home/report/ReportView.js | 44 +++++++++++++++-------------- 2 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 9291f9a8bd5a..327a12d954bb 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -5,7 +5,6 @@ import {View} from 'react-native'; import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; -import FullScreenLoadingIndicator from '../../components/Loading/FullscreenLoading'; import HeaderView from './HeaderView'; import Navigation from '../../libs/Navigation/Navigation'; import {fetchActions} from '../../libs/actions/Report'; @@ -66,7 +65,7 @@ class ReportScreen extends React.Component { // Todo: ask why getters aren't on top? get canRenderMainContent() { - return !this.state.isLoading && !this.state.error && !this.state.drawerOpen; + return !this.state.isLoading && !this.state.error; } get reportID() { @@ -80,15 +79,6 @@ class ReportScreen extends React.Component { error: null, }); - /* Todo: it might be good if this method resolves with needed data here - * when online resolve with data from the server - * when offline resolve with local data - * E.cash strategy is optimistic response though. Maybe fetchAction can internally - * resolve instantly when it can (some onyx data available) and then - * push an update to onyx data after actual data is fetched - * Another issue for optimistic loading - when data is available - * the Chat will try to render immediately which is what's causing the - * transition issues in the first place */ fetchActions(this.reportID) .catch((error) => { // Todo: what should we do if fetching fails for some reason @@ -97,14 +87,12 @@ class ReportScreen extends React.Component { console.error(error); }) .finally(() => { - console.debug('[ReportScreen] Fetch complete: '); + console.debug('[ReportScreen] Stop loading: '); this.setState({isLoading: false}); }); } render() { - const activeReportID = parseInt(this.props.reportID, 10); - return ( Navigation.navigate(ROUTES.HOME)} /> - { - this.canRenderMainContent - ? - : - } + ); diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index f9125a472c1a..d1908393f1cf 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -7,36 +7,38 @@ import {addAction} from '../../../libs/actions/Report'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; +import FullScreenLoadingIndicator from '../../../components/Loading/FullscreenLoading'; const propTypes = { - // The ID of the report actions will be created for + /* The ID of the report the selected report */ reportID: PropTypes.number.isRequired, -}; -// This is a PureComponent so that it only re-renders when the reportID changes or when the report changes from -// active to inactive (or vice versa). This should greatly reduce how often comments are re-rendered. -class ReportView extends React.Component { - shouldComponentUpdate(prevProps) { - return this.props.reportID !== prevProps.reportID; - } + /* Is the view ready to be displayed */ + loaded: PropTypes.bool.isRequired, + + /* Is the report view covered by the drawer */ + drawerOpen: PropTypes.bool.isRequired, +}; - render() { - return ( - - +function ReportView({reportID, drawerOpen, loaded}) { + return ( + + { + loaded + ? + : + } + {!drawerOpen && ( Keyboard.dismiss()}> addAction(this.props.reportID, text)} - reportID={this.props.reportID} - key={this.props.reportID} + onSubmit={text => addAction(reportID, text)} + reportID={reportID} /> - - - ); - } + )} + + + ); } ReportView.propTypes = propTypes; From 82529971d4131e3f9476882a0f1f5031dfd29b16 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 01:23:10 +0300 Subject: [PATCH 06/46] refactor: ReportActionsView remove extra calls to initial fetch --- src/pages/home/report/ReportActionsView.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 708130ac689f..a5b5af0421e9 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -90,7 +90,6 @@ class ReportActionsView extends React.Component { subscribeToReportTypingEvents(this.props.reportID); this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); - fetchActions(this.props.reportID); } shouldComponentUpdate(nextProps, nextState) { @@ -189,9 +188,6 @@ class ReportActionsView extends React.Component { unsubscribeFromReportChannel(oldReportID); subscribeToReportTypingEvents(this.props.reportID); Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); - - // Fetch the new set of actions - fetchActions(this.props.reportID); } /** From 0dd170bb58d152f5c6bd4fe9c1a2316e625e0c17 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 02:06:29 +0300 Subject: [PATCH 07/46] refactor: ReportScreen extract withDrawerState HOC --- src/components/withDrawerState.js | 49 +++++++++++++++++++++++++++++ src/pages/home/ReportScreen.js | 29 ----------------- src/pages/home/report/ReportView.js | 9 +++--- 3 files changed, 54 insertions(+), 33 deletions(-) create mode 100644 src/components/withDrawerState.js diff --git a/src/components/withDrawerState.js b/src/components/withDrawerState.js new file mode 100644 index 000000000000..150be8ad7622 --- /dev/null +++ b/src/components/withDrawerState.js @@ -0,0 +1,49 @@ +import React, {Component} from 'react'; +import PropTypes from 'prop-types'; +import {NavigationContext} from '@react-navigation/native'; +import getComponentDisplayName from '../libs/getComponentDisplayName'; + +export const withDrawerPropTypes = { + isDrawerOpen: PropTypes.bool.isRequired, +}; + +export default function withDrawerState(WrappedComponent) { + class HOC_Wrapper extends Component { + constructor(props) { + super(props); + + this.state = { + isDrawerOpen: true, + }; + } + + componentDidMount() { + this.removeOpenListener = this.context.addListener('drawerOpen', () => { + this.setState({isDrawerOpen: true}); + }); + + this.removeCloseListener = this.context.addListener('drawerClose', () => { + this.setState({isDrawerOpen: false}); + }); + } + + componentWillUnmount() { + this.removeOpenListener(); + this.removeCloseListener(); + } + + render() { + return ( + + ); + } + } + + HOC_Wrapper.contextType = NavigationContext; + HOC_Wrapper.displayName = `withWindowDimensions(${getComponentDisplayName(WrappedComponent)})`; + return HOC_Wrapper; +} diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 327a12d954bb..fa0cbf08ad77 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -14,12 +14,6 @@ import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { /* The ID of the report this screen should display */ reportID: PropTypes.string.isRequired, - - /* Todo: extract to `withDrawer` HOC */ - navigation: PropTypes.shape({ - addListener: PropTypes.func.isRequired, - }).isRequired, - }; class ReportScreen extends React.Component { @@ -29,26 +23,9 @@ class ReportScreen extends React.Component { this.state = { isLoading: true, error: null, - drawerOpen: true, }; } - componentDidMount() { - if (this.reportID) { - this.fetchReport(); - } - - // Todo: extract to `withDrawer` HOC - this.openListener = this.props.navigation.addListener('drawerOpen', () => { - this.setState({drawerOpen: true}); - }); - - // Todo: extract to `withDrawer` HOC - this.closeListener = this.props.navigation.addListener('drawerClose', () => { - this.setState({drawerOpen: false}); - }); - } - componentDidUpdate(prevProps) { // Reports changed, reset and load new data if (this.props.reportID !== prevProps.reportID) { @@ -58,11 +35,6 @@ class ReportScreen extends React.Component { } } - componentWillUnmount() { - this.openListener(); - this.closeListener(); - } - // Todo: ask why getters aren't on top? get canRenderMainContent() { return !this.state.isLoading && !this.state.error; @@ -109,7 +81,6 @@ class ReportScreen extends React.Component { diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index d1908393f1cf..40df924f09d0 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -8,6 +8,7 @@ import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; import FullScreenLoadingIndicator from '../../../components/Loading/FullscreenLoading'; +import withDrawerState from '../../../components/withDrawerState'; const propTypes = { /* The ID of the report the selected report */ @@ -17,10 +18,10 @@ const propTypes = { loaded: PropTypes.bool.isRequired, /* Is the report view covered by the drawer */ - drawerOpen: PropTypes.bool.isRequired, + isDrawerOpen: PropTypes.bool.isRequired, }; -function ReportView({reportID, drawerOpen, loaded}) { +function ReportView({reportID, isDrawerOpen, loaded}) { return ( { @@ -28,7 +29,7 @@ function ReportView({reportID, drawerOpen, loaded}) { ? : } - {!drawerOpen && ( + {!isDrawerOpen && ( Keyboard.dismiss()}> addAction(reportID, text)} @@ -42,4 +43,4 @@ function ReportView({reportID, drawerOpen, loaded}) { } ReportView.propTypes = propTypes; -export default ReportView; +export default withDrawerState(ReportView); From 8a6df1ba02e2f09fb44517373e6515207eb79b33 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 02:11:20 +0300 Subject: [PATCH 08/46] refactor: withDrawerState implement with useIsDrawerOpen for simplicity --- src/components/withDrawerState.js | 46 ++++++++----------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/src/components/withDrawerState.js b/src/components/withDrawerState.js index 150be8ad7622..4e2880130f62 100644 --- a/src/components/withDrawerState.js +++ b/src/components/withDrawerState.js @@ -1,6 +1,6 @@ -import React, {Component} from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; -import {NavigationContext} from '@react-navigation/native'; +import {useIsDrawerOpen} from '@react-navigation/drawer'; import getComponentDisplayName from '../libs/getComponentDisplayName'; export const withDrawerPropTypes = { @@ -8,42 +8,18 @@ export const withDrawerPropTypes = { }; export default function withDrawerState(WrappedComponent) { - class HOC_Wrapper extends Component { - constructor(props) { - super(props); + const HOC_Wrapper = (props) => { + const isDrawerOpen = useIsDrawerOpen(); - this.state = { - isDrawerOpen: true, - }; - } - - componentDidMount() { - this.removeOpenListener = this.context.addListener('drawerOpen', () => { - this.setState({isDrawerOpen: true}); - }); - - this.removeCloseListener = this.context.addListener('drawerClose', () => { - this.setState({isDrawerOpen: false}); - }); - } - - componentWillUnmount() { - this.removeOpenListener(); - this.removeCloseListener(); - } - - render() { - return ( - - ); - } - } + {...props} + isDrawerOpen={isDrawerOpen} + /> + ); + }; - HOC_Wrapper.contextType = NavigationContext; HOC_Wrapper.displayName = `withWindowDimensions(${getComponentDisplayName(WrappedComponent)})`; return HOC_Wrapper; } From 8d89162a61ca97fa08b31af1c7f3eee5e7bdbf91 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 02:18:22 +0300 Subject: [PATCH 09/46] refactor: ReportView improve transition using drawer state --- src/pages/home/report/ReportView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index 40df924f09d0..e9ebecc70bda 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -25,7 +25,7 @@ function ReportView({reportID, isDrawerOpen, loaded}) { return ( { - loaded + !isDrawerOpen && loaded ? : } From 283e6c035b0ab42a0ce8e405d72ec4dbc51f1c22 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 02:33:20 +0300 Subject: [PATCH 10/46] refactor: ReportActionCompose remove drawer check This check is no longer needed 1. It didn't work accurately 2. The compose component is unmounted when the drawer is expanded --- src/pages/home/report/ReportActionCompose.js | 6 ------ src/pages/home/report/ReportView.js | 1 + 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 85eb443286c1..ddc08d74ee6f 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -17,7 +17,6 @@ import AttachmentModal from '../../../components/AttachmentModal'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; import compose from '../../../libs/compose'; import CreateMenu from '../../../components/CreateMenu'; -import Navigation from '../../../libs/Navigation/Navigation'; const propTypes = { // A method to call when the form is submitted @@ -175,10 +174,6 @@ class ReportActionCompose extends React.Component { } render() { - // We want to make sure to disable on small screens because in iOS safari the keyboard up/down buttons will - // focus this from the chat switcher. - // https://github.com/Expensify/Expensify.cash/issues/1228 - const inputDisable = this.props.isSmallScreenWidth && Navigation.isDrawerOpen(); // eslint-disable-next-line no-unused-vars const hasMultipleParticipants = lodashGet(this.props.report, 'participants.length') > 1; @@ -277,7 +272,6 @@ class ReportActionCompose extends React.Component { onPasteFile={file => displayFileInModal({file})} shouldClear={this.state.textInputShouldClear} onClear={() => this.setTextInputShouldClear(false)} - isDisabled={inputDisable} /> diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index e9ebecc70bda..75ace673fb1c 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -34,6 +34,7 @@ function ReportView({reportID, isDrawerOpen, loaded}) { addAction(reportID, text)} reportID={reportID} + key={reportID} /> )} From 4cf35fdee2dfc6b5fce6473ee4118a6cf79addd7 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 3 Apr 2021 04:37:34 +0300 Subject: [PATCH 11/46] refactor: ReportScreen handle initial state --- src/components/Loading/LoadingIndicator.js | 1 + src/pages/home/ReportScreen.js | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/components/Loading/LoadingIndicator.js b/src/components/Loading/LoadingIndicator.js index f64a0c4b7075..708d59ba44ba 100644 --- a/src/components/Loading/LoadingIndicator.js +++ b/src/components/Loading/LoadingIndicator.js @@ -1,3 +1,4 @@ +/* eslint-disable react/jsx-props-no-spreading */ import React, {Component} from 'react'; import PropTypes from 'prop-types'; import { diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index fa0cbf08ad77..36eee9c4ae94 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -13,7 +13,11 @@ import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { /* The ID of the report this screen should display */ - reportID: PropTypes.string.isRequired, + reportID: PropTypes.string, +}; + +const defaultProps = { + reportID: '0', }; class ReportScreen extends React.Component { @@ -26,18 +30,20 @@ class ReportScreen extends React.Component { }; } + componentDidMount() { + this.fetchReport(); + } + componentDidUpdate(prevProps) { // Reports changed, reset and load new data if (this.props.reportID !== prevProps.reportID) { - if (this.reportID) { - this.fetchReport(); - } + this.fetchReport(); } } // Todo: ask why getters aren't on top? get canRenderMainContent() { - return !this.state.isLoading && !this.state.error; + return this.reportID && !this.state.isLoading && !this.state.error; } get reportID() { @@ -45,6 +51,8 @@ class ReportScreen extends React.Component { } fetchReport() { + if (!this.reportID) { return; } + console.debug('[ReportScreen] Fetch started: '); this.setState({ isLoading: true, @@ -91,6 +99,8 @@ class ReportScreen extends React.Component { ReportScreen.displayName = 'ReportScreen'; ReportScreen.propTypes = propTypes; +ReportScreen.defaultProps = defaultProps; + export default withOnyx({ reportID: { key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, From 7fa89be020e070867ce9426fb02697bba8e21a3b Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 11:37:23 +0300 Subject: [PATCH 12/46] refactor: update copy pasted display name --- src/components/withDrawerState.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/withDrawerState.js b/src/components/withDrawerState.js index 4e2880130f62..3bcc81756e62 100644 --- a/src/components/withDrawerState.js +++ b/src/components/withDrawerState.js @@ -13,13 +13,13 @@ export default function withDrawerState(WrappedComponent) { return ( ); }; - HOC_Wrapper.displayName = `withWindowDimensions(${getComponentDisplayName(WrappedComponent)})`; + HOC_Wrapper.displayName = `withDrawerState(${getComponentDisplayName(WrappedComponent)})`; return HOC_Wrapper; } From 96ff600d5d61e71c4493e0d7bbd7b4256d4db75d Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 11:49:06 +0300 Subject: [PATCH 13/46] refactor: ReportScreen use reportID from route params `reportID` comes with a delay when the prop comes from onyx The user first selects a route in the drawer The route immediately hides the drawer and reviews the report screen `reportID` have not yet updated and the old report is visible for a moment Switching to route params changes the `reportID` the moment the user selects one of the sidebar chat links --- src/pages/home/ReportScreen.js | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 36eee9c4ae94..b7b334b8740f 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -9,15 +9,16 @@ import HeaderView from './HeaderView'; import Navigation from '../../libs/Navigation/Navigation'; import {fetchActions} from '../../libs/actions/Report'; import ROUTES from '../../ROUTES'; -import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { - /* The ID of the report this screen should display */ - reportID: PropTypes.string, -}; - -const defaultProps = { - reportID: '0', + /* Navigation route context info */ + route: PropTypes.shape({ + /* Route specific parameters used on this screen */ + params: PropTypes.shape({ + /* The ID of the report this screen should display */ + reportID: PropTypes.string, + }).isRequired, + }).isRequired, }; class ReportScreen extends React.Component { @@ -36,7 +37,7 @@ class ReportScreen extends React.Component { componentDidUpdate(prevProps) { // Reports changed, reset and load new data - if (this.props.reportID !== prevProps.reportID) { + if (this.reportID !== prevProps.route.params.reportID) { this.fetchReport(); } } @@ -47,7 +48,8 @@ class ReportScreen extends React.Component { } get reportID() { - return parseInt(this.props.reportID, 10); + const params = this.props.route.params; + return params.reportID; } fetchReport() { @@ -97,12 +99,5 @@ class ReportScreen extends React.Component { } } -ReportScreen.displayName = 'ReportScreen'; ReportScreen.propTypes = propTypes; -ReportScreen.defaultProps = defaultProps; - -export default withOnyx({ - reportID: { - key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID, - }, -})(ReportScreen); +export default ReportScreen; From b2177a608c7a67c697b735cd3981f954b2253e6c Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 11:53:13 +0300 Subject: [PATCH 14/46] refactor: ReportScreen expand the drawer on back navigation Don't trigger a navigation event as this causes a few unintended re-renders We don't need to trigger a navigation when can review the drawer from any chat screen we're currently on --- src/pages/home/ReportScreen.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index b7b334b8740f..ff14336c88cb 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -6,12 +6,16 @@ import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; import HeaderView from './HeaderView'; -import Navigation from '../../libs/Navigation/Navigation'; import {fetchActions} from '../../libs/actions/Report'; -import ROUTES from '../../ROUTES'; const propTypes = { - /* Navigation route context info */ + /* Navigation api provided by react navigation */ + navigation: PropTypes.shape({ + /* Display the drawer programmatically */ + openDrawer: PropTypes.func.isRequired, + }).isRequired, + + /* Navigation route context info provided by react navigation */ route: PropTypes.shape({ /* Route specific parameters used on this screen */ params: PropTypes.shape({ @@ -85,7 +89,7 @@ class ReportScreen extends React.Component { > Navigation.navigate(ROUTES.HOME)} + onNavigationMenuButtonClicked={this.props.navigation.openDrawer} /> From 2f4a7ea80d31648dd9db2daefe9996b531bcecef Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 11:57:23 +0300 Subject: [PATCH 15/46] feat: ReportActionCompose allow autoFocus to be configurable from parents --- src/pages/home/report/ReportActionCompose.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index ddc08d74ee6f..df9208facb1e 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -28,6 +28,9 @@ const propTypes = { // The ID of the report actions will be created for reportID: PropTypes.number.isRequired, + // Should the input focus on mount + autoFocus: PropTypes.bool.isRequired, + // Details about any modals being used modal: PropTypes.shape({ // Indicates if there is a modal currently visible or not @@ -62,7 +65,7 @@ class ReportActionCompose extends React.Component { this.comment = props.comment; this.state = { - isFocused: true, + isFocused: this.props.autoFocus, textInputShouldClear: false, isCommentEmpty: props.comment.length === 0, isMenuVisible: false, @@ -243,7 +246,7 @@ class ReportActionCompose extends React.Component { )} this.textInput = el} textAlignVertical="top" From 073249ee329e273dba4a07cc57972adae5fb246d Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 12:05:40 +0300 Subject: [PATCH 16/46] refactor: ReportView optimizations Keep the view mounted when the report doesn't change Unmount the input when the drawer covers the view Set autoFocus only for bigger screens as example --- src/pages/home/report/ReportView.js | 33 ++++++++++++++++++----------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index 75ace673fb1c..25c8cf92e119 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -4,11 +4,13 @@ import PropTypes from 'prop-types'; import ReportActionsView from './ReportActionsView'; import ReportActionCompose from './ReportActionCompose'; import {addAction} from '../../../libs/actions/Report'; +import compose from '../../../libs/compose'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; import FullScreenLoadingIndicator from '../../../components/Loading/FullscreenLoading'; import withDrawerState from '../../../components/withDrawerState'; +import withWindowDimensions from '../../../components/withWindowDimensions'; const propTypes = { /* The ID of the report the selected report */ @@ -19,22 +21,26 @@ const propTypes = { /* Is the report view covered by the drawer */ isDrawerOpen: PropTypes.bool.isRequired, + + /* Is the window width narrow, like on a mobile device */ + isSmallScreenWidth: PropTypes.bool.isRequired, }; -function ReportView({reportID, isDrawerOpen, loaded}) { +function ReportView(props) { + const isDrawerOutOfTheWay = !props.isDrawerOpen || !props.isSmallScreenWidth; + return ( - - { - !isDrawerOpen && loaded - ? - : - } - {!isDrawerOpen && ( + + {!props.loaded && } + + + + {isDrawerOutOfTheWay && ( Keyboard.dismiss()}> addAction(reportID, text)} - reportID={reportID} - key={reportID} + onSubmit={text => addAction(props.reportID, text)} + reportID={props.reportID} + autoFocus={!props.isSmallScreenWidth} /> )} @@ -44,4 +50,7 @@ function ReportView({reportID, isDrawerOpen, loaded}) { } ReportView.propTypes = propTypes; -export default withDrawerState(ReportView); +export default compose( + withWindowDimensions, + withDrawerState, +)(ReportView); From f3fea9b844dfa819ac38db78ed836d57d1bb66ea Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 12:13:19 +0300 Subject: [PATCH 17/46] refactor: ReportActionsView remove unnecessary logic around report switching This is now handled by re-mounting Remove action sorting performed on each render - apply only when actions change --- src/pages/home/report/ReportActionsView.js | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index a5b5af0421e9..f9068474d32a 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -76,6 +76,7 @@ class ReportActionsView extends React.Component { // Helper variable that keeps track of the unread action count before it updates to zero this.unreadActionCount = 0; + // Todo: this should come from props and depend on isDrawerOutOfTheWay // Helper variable that prevents the unread indicator to show up for new messages // received while the report is still active this.shouldShowUnreadActionIndicator = true; @@ -83,6 +84,8 @@ class ReportActionsView extends React.Component { this.state = { isLoadingMoreChats: false, }; + + this.updateSortedReportActions(props.reportActions); } componentDidMount() { @@ -93,11 +96,8 @@ class ReportActionsView extends React.Component { } shouldComponentUpdate(nextProps, nextState) { - if (nextProps.reportID !== this.props.reportID) { - return true; - } - if (!_.isEqual(nextProps.reportActions, this.props.reportActions)) { + this.updateSortedReportActions(nextProps.reportActions); return true; } @@ -109,12 +109,6 @@ class ReportActionsView extends React.Component { } componentDidUpdate(prevProps) { - // We have switched to a new report - if (prevProps.reportID !== this.props.reportID) { - this.reset(prevProps.reportID); - return; - } - // The last sequenceNumber of the same report has changed. const previousLastSequenceNumber = lodashGet(lastItem(prevProps.reportActions), 'sequenceNumber'); const currentLastSequenceNumber = lodashGet(lastItem(this.props.reportActions), 'sequenceNumber'); @@ -215,9 +209,11 @@ class ReportActionsView extends React.Component { /** * Updates and sorts the report actions by sequence number + * + * @param {Array<{sequenceNumber, actionName}>} reportActions */ - updateSortedReportActions() { - this.sortedReportActions = _.chain(this.props.reportActions) + updateSortedReportActions(reportActions) { + this.sortedReportActions = _.chain(reportActions) .sortBy('sequenceNumber') .filter(action => action.actionName === 'ADDCOMMENT' || action.actionName === 'IOU') .map((item, index) => ({action: item, index})) @@ -361,7 +357,6 @@ class ReportActionsView extends React.Component { } this.setUpUnreadActionIndicator(); - this.updateSortedReportActions(); return ( this.actionListElement = el} From 5d2f079a3d90ec29412abb74b9acb6bc252d6ea7 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 12:14:39 +0300 Subject: [PATCH 18/46] refactor: ReportActionsView remove `needsLayoutCalculation` This prop no longer exists The calculation seems to be handled differently and this is leftover --- src/pages/home/report/ReportActionsView.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index f9068474d32a..dd803f9dda2a 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -311,7 +311,6 @@ class ReportActionsView extends React.Component { * @param {Object} args.item * @param {Number} args.index * @param {Function} args.onLayout - * @param {Boolean} args.needsLayoutCalculation * * @returns {React.Component} */ @@ -319,7 +318,6 @@ class ReportActionsView extends React.Component { item, index, onLayout, - needsLayoutCalculation, }) { return ( @@ -335,7 +333,6 @@ class ReportActionsView extends React.Component { action={item.action} displayAsGroup={this.isConsecutiveActionMadeByPreviousActor(index)} onLayout={onLayout} - needsLayoutCalculation={needsLayoutCalculation} /> ); From 199fb116969c43ebece0c3b27708dbf87b6a036e Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 13:28:48 +0300 Subject: [PATCH 19/46] refactor: ReportActionCompose remove `withWindowDimensions` - unused This is no longer needed - functionality extracted outside --- src/pages/home/report/ReportActionCompose.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index df9208facb1e..aa4bff1a0f26 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -14,7 +14,6 @@ import AttachmentPicker from '../../../components/AttachmentPicker'; import {addAction, saveReportComment, broadcastUserIsTyping} from '../../../libs/actions/Report'; import ReportTypingIndicator from './ReportTypingIndicator'; import AttachmentModal from '../../../components/AttachmentModal'; -import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions'; import compose from '../../../libs/compose'; import CreateMenu from '../../../components/CreateMenu'; @@ -43,8 +42,6 @@ const propTypes = { // participants associated with current report participants: PropTypes.arrayOf(PropTypes.string), }).isRequired, - - ...windowDimensionsPropTypes, }; const defaultProps = { @@ -312,5 +309,4 @@ export default compose( key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, }, }), - withWindowDimensions, )(ReportActionCompose); From 7003ad804191eab8da121347cc684030432a2986 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 13:54:36 +0300 Subject: [PATCH 20/46] feat: ReportScreen loading view layout transitions Nicer transition on iOS. I can't see any difference on Android --- src/pages/home/ReportScreen.js | 8 ++++++-- src/setup/index.android.js | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 src/setup/index.android.js diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index ff14336c88cb..c8c35ab46b02 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {withOnyx} from 'react-native-onyx'; -import {View} from 'react-native'; +import {LayoutAnimation, View} from 'react-native'; import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; @@ -59,6 +58,9 @@ class ReportScreen extends React.Component { fetchReport() { if (!this.reportID) { return; } + // This adds a small fade in transition when the loader appears + LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); + console.debug('[ReportScreen] Fetch started: '); this.setState({ isLoading: true, @@ -73,6 +75,8 @@ class ReportScreen extends React.Component { console.error(error); }) .finally(() => { + // This adds a small fadeout transition when the loader disappears + LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); console.debug('[ReportScreen] Stop loading: '); this.setState({isLoading: false}); }); diff --git a/src/setup/index.android.js b/src/setup/index.android.js new file mode 100644 index 000000000000..a1869b33d239 --- /dev/null +++ b/src/setup/index.android.js @@ -0,0 +1,8 @@ +import {Platform, UIManager} from 'react-native'; + +export default function () { + // To use layout animations in Android: https://reactnative.dev/docs/layoutanimation + if (Platform.OS === 'android' && UIManager.setLayoutAnimationEnabledExperimental) { + UIManager.setLayoutAnimationEnabledExperimental(true); + } +} From 33960741407b025f14a3acf59a2be40628794ee5 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 13:58:52 +0300 Subject: [PATCH 21/46] refactor: ReportScreen clean up comments and console.logs --- src/pages/home/ReportScreen.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index c8c35ab46b02..404402c71a8e 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -45,7 +45,6 @@ class ReportScreen extends React.Component { } } - // Todo: ask why getters aren't on top? get canRenderMainContent() { return this.reportID && !this.state.isLoading && !this.state.error; } @@ -61,7 +60,6 @@ class ReportScreen extends React.Component { // This adds a small fade in transition when the loader appears LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); - console.debug('[ReportScreen] Fetch started: '); this.setState({ isLoading: true, error: null, @@ -69,15 +67,15 @@ class ReportScreen extends React.Component { fetchActions(this.reportID) .catch((error) => { - // Todo: what should we do if fetching fails for some reason - // Todo: see what we currently do and do the same here + /* Todo: do something here if the user is going to be left with an empty view + * E.g. Alert like "Oops, we couldn't load this conversation. Please Try again" + * button Retry -> this.fetchReport() */ this.setState({error}); console.error(error); }) .finally(() => { // This adds a small fadeout transition when the loader disappears LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); - console.debug('[ReportScreen] Stop loading: '); this.setState({isLoading: false}); }); } From fc371c0b47ae629eab39afba1cce7ef6a6a686a9 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Apr 2021 15:30:34 +0300 Subject: [PATCH 22/46] refactor: ReportScreen restore reportID as number There are many prop type warnings when this is string. --- src/pages/home/ReportScreen.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 404402c71a8e..a57f183fd805 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -40,18 +40,18 @@ class ReportScreen extends React.Component { componentDidUpdate(prevProps) { // Reports changed, reset and load new data - if (this.reportID !== prevProps.route.params.reportID) { + if (this.props.route.params.reportID !== prevProps.route.params.reportID) { this.fetchReport(); } } get canRenderMainContent() { - return this.reportID && !this.state.isLoading && !this.state.error; + return Boolean(this.reportID) && !this.state.isLoading && !this.state.error; } get reportID() { const params = this.props.route.params; - return params.reportID; + return parseInt(params.reportID, 10) || 0; } fetchReport() { From b3414b69c45ba7d38763cda7c60fa16585f6a959 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 14:16:21 +0300 Subject: [PATCH 23/46] Remove todo note, already addressed elsewhere and could cause confusion --- src/pages/home/report/ReportActionsView.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index dd803f9dda2a..194b37a21551 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -76,7 +76,6 @@ class ReportActionsView extends React.Component { // Helper variable that keeps track of the unread action count before it updates to zero this.unreadActionCount = 0; - // Todo: this should come from props and depend on isDrawerOutOfTheWay // Helper variable that prevents the unread indicator to show up for new messages // received while the report is still active this.shouldShowUnreadActionIndicator = true; From 5ce7b33eb92e43a23920a0ab8b70801a0a08da97 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 14:22:12 +0300 Subject: [PATCH 24/46] refactor: ReportScreen remove error state --- src/pages/home/ReportScreen.js | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index a57f183fd805..e7d001a58201 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -30,7 +30,6 @@ class ReportScreen extends React.Component { this.state = { isLoading: true, - error: null, }; } @@ -46,7 +45,7 @@ class ReportScreen extends React.Component { } get canRenderMainContent() { - return Boolean(this.reportID) && !this.state.isLoading && !this.state.error; + return Boolean(this.reportID) && !this.state.isLoading; } get reportID() { @@ -60,19 +59,10 @@ class ReportScreen extends React.Component { // This adds a small fade in transition when the loader appears LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); - this.setState({ - isLoading: true, - error: null, - }); + this.setState({isLoading: true}); fetchActions(this.reportID) - .catch((error) => { - /* Todo: do something here if the user is going to be left with an empty view - * E.g. Alert like "Oops, we couldn't load this conversation. Please Try again" - * button Retry -> this.fetchReport() */ - this.setState({error}); - console.error(error); - }) + .catch(console.error) .finally(() => { // This adds a small fadeout transition when the loader disappears LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); From a3df62a9caf53f8a55d12c522d012a2fbe9c567b Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 14:32:42 +0300 Subject: [PATCH 25/46] refactor: ReportScreen replace computed getters with methods --- src/pages/home/ReportScreen.js | 30 +++++++++++++++++++---------- src/pages/home/report/ReportView.js | 4 ++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index e7d001a58201..abf05733e43a 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -44,24 +44,34 @@ class ReportScreen extends React.Component { } } - get canRenderMainContent() { - return Boolean(this.reportID) && !this.state.isLoading; + /** + * Get the currently viewed report ID as number + * + * @returns {Number} + */ + getReportID() { + const params = this.props.route.params; + return Number.parseInt(params.reportID, 10); } - get reportID() { - const params = this.props.route.params; - return parseInt(params.reportID, 10) || 0; + /** + * When reports change there's a brief time content is not ready to be displayed + * + * @returns {Boolean} + */ + isReadyToDisplayReport() { + return !this.state.isLoading && Boolean(this.getReportID()); } fetchReport() { - if (!this.reportID) { return; } + if (!this.getReportID()) { return; } // This adds a small fade in transition when the loader appears LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); this.setState({isLoading: true}); - fetchActions(this.reportID) + fetchActions(this.getReportID()) .catch(console.error) .finally(() => { // This adds a small fadeout transition when the loader disappears @@ -80,14 +90,14 @@ class ReportScreen extends React.Component { ]} > diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index 25c8cf92e119..abad1a5cfd97 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -17,7 +17,7 @@ const propTypes = { reportID: PropTypes.number.isRequired, /* Is the view ready to be displayed */ - loaded: PropTypes.bool.isRequired, + isReady: PropTypes.bool.isRequired, /* Is the report view covered by the drawer */ isDrawerOpen: PropTypes.bool.isRequired, @@ -31,7 +31,7 @@ function ReportView(props) { return ( - {!props.loaded && } + {!props.isReady && } From 809d0413b6b24f99bab95afb30092949064ed1ad Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 14:51:08 +0300 Subject: [PATCH 26/46] Revert "feat: ReportActionCompose allow autoFocus to be configurable from parents" This reverts commit 2f4a7ea8 --- src/pages/home/report/ReportActionCompose.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index aa4bff1a0f26..6c141d1c6ffe 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -27,9 +27,6 @@ const propTypes = { // The ID of the report actions will be created for reportID: PropTypes.number.isRequired, - // Should the input focus on mount - autoFocus: PropTypes.bool.isRequired, - // Details about any modals being used modal: PropTypes.shape({ // Indicates if there is a modal currently visible or not @@ -62,7 +59,7 @@ class ReportActionCompose extends React.Component { this.comment = props.comment; this.state = { - isFocused: this.props.autoFocus, + isFocused: true, textInputShouldClear: false, isCommentEmpty: props.comment.length === 0, isMenuVisible: false, @@ -243,7 +240,7 @@ class ReportActionCompose extends React.Component { )} this.textInput = el} textAlignVertical="top" From 0d50195913fd79d33ad66c56347fac3a9ebb0714 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 14:54:45 +0300 Subject: [PATCH 27/46] refactor: Address autoFocus usage and add documentation --- src/components/TextInputFocusable/index.js | 5 +++++ src/components/TextInputFocusable/index.native.js | 5 +++++ src/pages/home/report/ReportView.js | 1 - 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/components/TextInputFocusable/index.js b/src/components/TextInputFocusable/index.js index bef213b31d27..764ca1984ba1 100644 --- a/src/components/TextInputFocusable/index.js +++ b/src/components/TextInputFocusable/index.js @@ -37,6 +37,10 @@ const propTypes = { // Whether or not this TextInput is disabled. isDisabled: PropTypes.bool, + + /* Set focus to this component the first time it renders. Override this in case you need to set focus on one + * field out of many, or when you want to disable autoFocus */ + autoFocus: PropTypes.bool, }; const defaultProps = { @@ -49,6 +53,7 @@ const defaultProps = { onDragLeave: () => {}, onDrop: () => {}, isDisabled: false, + autoFocus: false, }; const IMAGE_EXTENSIONS = { diff --git a/src/components/TextInputFocusable/index.native.js b/src/components/TextInputFocusable/index.native.js index a7d571949d02..ba3db4fc297b 100644 --- a/src/components/TextInputFocusable/index.native.js +++ b/src/components/TextInputFocusable/index.native.js @@ -17,11 +17,16 @@ const propTypes = { // When the input has cleared whoever owns this input should know about it onClear: PropTypes.func, + + /* Set focus to this component the first time it renders. Override this in case you need to set focus on one + * field out of many, or when you want to disable autoFocus */ + autoFocus: PropTypes.bool, }; const defaultProps = { shouldClear: false, onClear: () => {}, + autoFocus: false, }; class TextInputFocusable extends React.Component { diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index abad1a5cfd97..3471108b8018 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -40,7 +40,6 @@ function ReportView(props) { addAction(props.reportID, text)} reportID={props.reportID} - autoFocus={!props.isSmallScreenWidth} /> )} From a03f8baa944d010bf1132befb16b34e19ca56c46 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 15:07:53 +0300 Subject: [PATCH 28/46] refactor: extract FullscreenLoading styles --- src/components/Loading/FullscreenLoading.js | 13 ++----------- src/styles/styles.js | 8 ++++++++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/components/Loading/FullscreenLoading.js b/src/components/Loading/FullscreenLoading.js index d274b7540bc5..d2b380a43e6d 100644 --- a/src/components/Loading/FullscreenLoading.js +++ b/src/components/Loading/FullscreenLoading.js @@ -1,21 +1,12 @@ import React from 'react'; import {StyleSheet, View} from 'react-native'; import LoadingIndicator from './LoadingIndicator'; +import styles from '../../styles/styles'; const FullScreenLoadingIndicator = () => ( - + ); -const styles = StyleSheet.create({ - container: { - ...StyleSheet.absoluteFillObject, - justifyContent: 'center', - alignItems: 'center', - backgroundColor: '#eeeeeecc', - zIndex: 10, - }, -}); - export default FullScreenLoadingIndicator; diff --git a/src/styles/styles.js b/src/styles/styles.js index 22b13cd5e8db..dbf548d24cd2 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -1259,6 +1259,14 @@ const styles = { noScrollbars: { scrollbarWidth: 'none', }, + + fullScreenLoading: { + backgroundColor: themeColors.modalBackdrop, + opacity: 0.8, + justifyContent: 'center', + alignItems: 'center', + zIndex: 10, + }, }; const baseCodeTagStyles = { From 8184e0f4792535a0e76d11c5c3efa4306d20f383 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 15:19:56 +0300 Subject: [PATCH 29/46] Drop: custom LoadingIndicator and use the built in ActivityIndicator --- src/components/FullscreenLoading.js | 17 ++++ src/components/Loading/FullscreenLoading.js | 12 --- src/components/Loading/LoadingIndicator.js | 101 -------------------- src/pages/home/report/ReportView.js | 2 +- 4 files changed, 18 insertions(+), 114 deletions(-) create mode 100644 src/components/FullscreenLoading.js delete mode 100644 src/components/Loading/FullscreenLoading.js delete mode 100644 src/components/Loading/LoadingIndicator.js diff --git a/src/components/FullscreenLoading.js b/src/components/FullscreenLoading.js new file mode 100644 index 000000000000..1630253c8a56 --- /dev/null +++ b/src/components/FullscreenLoading.js @@ -0,0 +1,17 @@ +import React from 'react'; +import {ActivityIndicator, StyleSheet, View} from 'react-native'; +import styles from '../styles/styles'; +import themeColors from '../styles/themes/default'; + +/** + * Loading indication component intended to cover the whole page, while the page prepares for initial render + * + * @returns {JSX.Element} + */ +const FullScreenLoadingIndicator = () => ( + + + +); + +export default FullScreenLoadingIndicator; diff --git a/src/components/Loading/FullscreenLoading.js b/src/components/Loading/FullscreenLoading.js deleted file mode 100644 index d2b380a43e6d..000000000000 --- a/src/components/Loading/FullscreenLoading.js +++ /dev/null @@ -1,12 +0,0 @@ -import React from 'react'; -import {StyleSheet, View} from 'react-native'; -import LoadingIndicator from './LoadingIndicator'; -import styles from '../../styles/styles'; - -const FullScreenLoadingIndicator = () => ( - - - -); - -export default FullScreenLoadingIndicator; diff --git a/src/components/Loading/LoadingIndicator.js b/src/components/Loading/LoadingIndicator.js deleted file mode 100644 index 708d59ba44ba..000000000000 --- a/src/components/Loading/LoadingIndicator.js +++ /dev/null @@ -1,101 +0,0 @@ -/* eslint-disable react/jsx-props-no-spreading */ -import React, {Component} from 'react'; -import PropTypes from 'prop-types'; -import { - Animated, - Easing, - View, - StyleSheet, -} from 'react-native'; -import Svg, {Circle} from 'react-native-svg'; - -const propTypes = { - useNativeDriver: PropTypes.bool, - size: PropTypes.number, -}; - -const defaultProps = { - useNativeDriver: true, - size: 40, -}; - -class LoadingIndicator extends Component { - constructor(props) { - super(props); - - this.spinValue = new Animated.Value(0); - - this.spinTransform = this.spinValue.interpolate({ - inputRange: [0, 1], - outputRange: ['0deg', '360deg'], - }); - - this.circleProps = { - style: StyleSheet.absoluteFillObject, - cx: 0, - cy: 0, - r: 40, - fill: 'transparent', - stroke: 'rgb(142, 152, 158)', - strokeWidth: 8, - strokeLinecap: 'butt', - strokeDasharray: 0, - strokeDashoffset: 0, - }; - } - - componentDidMount() { - this.spin(); - } - - spin() { - Animated.loop( - Animated.timing( - this.spinValue, - { - toValue: 1, - duration: 690, - easing: Easing.linear, - useNativeDriver: this.props.useNativeDriver, - }, - ), - ).start(); - } - - render() { - const size = this.props.size; - - return ( - - - - - - - - - ); - } -} - -const styles = StyleSheet.create({ - container: { - alignItems: 'center', - justifyContent: 'center', - }, -}); - -LoadingIndicator.propTypes = propTypes; -LoadingIndicator.defaultProps = defaultProps; - -export default LoadingIndicator; diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index 3471108b8018..d2d292683762 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -8,7 +8,7 @@ import compose from '../../../libs/compose'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; -import FullScreenLoadingIndicator from '../../../components/Loading/FullscreenLoading'; +import FullScreenLoadingIndicator from '../../../components/FullscreenLoading'; import withDrawerState from '../../../components/withDrawerState'; import withWindowDimensions from '../../../components/withWindowDimensions'; From a3f92f5dc1a05358fdcefce1130048b77628b4b4 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 15:28:54 +0300 Subject: [PATCH 30/46] drop: Remove layout animations Additional weight on th PR - These can be proposed as a follow up improvement --- src/pages/home/ReportScreen.js | 11 ++--------- src/setup/index.android.js | 8 -------- 2 files changed, 2 insertions(+), 17 deletions(-) delete mode 100644 src/setup/index.android.js diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index abf05733e43a..9b5367d09b58 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {LayoutAnimation, View} from 'react-native'; +import {View} from 'react-native'; import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; @@ -66,18 +66,11 @@ class ReportScreen extends React.Component { fetchReport() { if (!this.getReportID()) { return; } - // This adds a small fade in transition when the loader appears - LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); - this.setState({isLoading: true}); fetchActions(this.getReportID()) .catch(console.error) - .finally(() => { - // This adds a small fadeout transition when the loader disappears - LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); - this.setState({isLoading: false}); - }); + .finally(() => this.setState({isLoading: false})); } render() { diff --git a/src/setup/index.android.js b/src/setup/index.android.js deleted file mode 100644 index a1869b33d239..000000000000 --- a/src/setup/index.android.js +++ /dev/null @@ -1,8 +0,0 @@ -import {Platform, UIManager} from 'react-native'; - -export default function () { - // To use layout animations in Android: https://reactnative.dev/docs/layoutanimation - if (Platform.OS === 'android' && UIManager.setLayoutAnimationEnabledExperimental) { - UIManager.setLayoutAnimationEnabledExperimental(true); - } -} From 0a96fa8168139f11b6711b29c218bce9fa11e0b3 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 15:44:37 +0300 Subject: [PATCH 31/46] feat: ReportScreen don't wait for fetch just show a brief loading --- src/pages/home/ReportScreen.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 9b5367d09b58..a266f0dae8d3 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -44,6 +44,10 @@ class ReportScreen extends React.Component { } } + componentWillUnmount() { + clearTimeout(this.loadingTimerId); + } + /** * Get the currently viewed report ID as number * @@ -63,14 +67,19 @@ class ReportScreen extends React.Component { return !this.state.isLoading && Boolean(this.getReportID()); } + /** + * Load initial data for the current report + * Configures a small loading transition of fixed time and proceeds with rendering available data + */ fetchReport() { if (!this.getReportID()) { return; } this.setState({isLoading: true}); - fetchActions(this.getReportID()) - .catch(console.error) - .finally(() => this.setState({isLoading: false})); + fetchActions(this.getReportID()).catch(console.error); + + clearTimeout(this.loadingTimerId); + this.loadingTimerId = setTimeout(() => this.setState({isLoading: false}), 300); } render() { From 3b4a2c3c2144f187c810d8659322e597fc75aa8f Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 15:51:43 +0300 Subject: [PATCH 32/46] refactor: ReportActionsView remove `reset` method - no longer needed Involved logic is already covered on mount and on unmount --- src/pages/home/report/ReportActionsView.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 194b37a21551..aae50ecb2336 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -92,6 +92,7 @@ class ReportActionsView extends React.Component { subscribeToReportTypingEvents(this.props.reportID); this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); + Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } shouldComponentUpdate(nextProps, nextState) { @@ -172,17 +173,6 @@ class ReportActionsView extends React.Component { this.shouldShowUnreadActionIndicator = false; } - /** - * Actions to run when the report has been updated - * @param {Number} oldReportID - */ - reset(oldReportID) { - // Unsubscribe from previous report and resubscribe - unsubscribeFromReportChannel(oldReportID); - subscribeToReportTypingEvents(this.props.reportID); - Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); - } - /** * Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently * displaying. From f0d7bb56afee07160956b2c3fea8c065097efe5f Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 15:55:37 +0300 Subject: [PATCH 33/46] refactor: ReportScreen move `fetchActions` call back to ReportActionsView ReportActionsView is already handling all the required fetching so it's better to just encapsulate that logic in a single place While the Report Screen can just setup a small transition --- src/pages/home/ReportScreen.js | 10 +++------- src/pages/home/report/ReportActionsView.js | 1 + 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index a266f0dae8d3..4d5032a35e0c 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -5,7 +5,6 @@ import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; import HeaderView from './HeaderView'; -import {fetchActions} from '../../libs/actions/Report'; const propTypes = { /* Navigation api provided by react navigation */ @@ -34,13 +33,13 @@ class ReportScreen extends React.Component { } componentDidMount() { - this.fetchReport(); + this.prepareTransition(); } componentDidUpdate(prevProps) { // Reports changed, reset and load new data if (this.props.route.params.reportID !== prevProps.route.params.reportID) { - this.fetchReport(); + this.prepareTransition(); } } @@ -68,16 +67,13 @@ class ReportScreen extends React.Component { } /** - * Load initial data for the current report * Configures a small loading transition of fixed time and proceeds with rendering available data */ - fetchReport() { + prepareTransition() { if (!this.getReportID()) { return; } this.setState({isLoading: true}); - fetchActions(this.getReportID()).catch(console.error); - clearTimeout(this.loadingTimerId); this.loadingTimerId = setTimeout(() => this.setState({isLoading: false}), 300); } diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index aae50ecb2336..9151c8747a5d 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -92,6 +92,7 @@ class ReportActionsView extends React.Component { subscribeToReportTypingEvents(this.props.reportID); this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); + fetchActions(this.props.reportID).catch(console.error); Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } From c08d14e4a92019ed0b09db3f7833a6c1b876f908 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 16:43:11 +0300 Subject: [PATCH 34/46] refactor: ReportScreen replace comment with self explanatory code --- src/pages/home/ReportScreen.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 4d5032a35e0c..572ee68aa10a 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -37,8 +37,9 @@ class ReportScreen extends React.Component { } componentDidUpdate(prevProps) { - // Reports changed, reset and load new data - if (this.props.route.params.reportID !== prevProps.route.params.reportID) { + const reportChanged = this.props.route.params.reportID !== prevProps.route.params.reportID; + + if (reportChanged) { this.prepareTransition(); } } From 7847ffa0b1330f5656ebdb91859eb2cf51412f4e Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 21:55:40 +0300 Subject: [PATCH 35/46] refactor: TextInputFocusable add isDisabled prop to the native component too --- src/components/TextInputFocusable/index.native.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/components/TextInputFocusable/index.native.js b/src/components/TextInputFocusable/index.native.js index ba3db4fc297b..f54704f2669b 100644 --- a/src/components/TextInputFocusable/index.native.js +++ b/src/components/TextInputFocusable/index.native.js @@ -21,12 +21,16 @@ const propTypes = { /* Set focus to this component the first time it renders. Override this in case you need to set focus on one * field out of many, or when you want to disable autoFocus */ autoFocus: PropTypes.bool, + + /* Prevent edits and interactions like focus for this input. */ + isDisabled: PropTypes.bool, }; const defaultProps = { shouldClear: false, onClear: () => {}, autoFocus: false, + isDisabled: false, }; class TextInputFocusable extends React.Component { @@ -53,6 +57,7 @@ class TextInputFocusable extends React.Component { ref={el => this.textInput = el} maxHeight={116} rejectResponderTermination={false} + editable={!this.props.isDisabled} /* eslint-disable-next-line react/jsx-props-no-spreading */ {...this.props} /> From a9ed45b0a58d5a1053054d82dd675a38c994ff3a Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 7 Apr 2021 21:56:44 +0300 Subject: [PATCH 36/46] refactor: ReportActionCompose disable while app focus is away from the input --- src/pages/home/report/ReportActionCompose.js | 4 ++++ src/pages/home/report/ReportView.js | 17 ++++++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 6c141d1c6ffe..e6bb7f7150fd 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -39,6 +39,9 @@ const propTypes = { // participants associated with current report participants: PropTypes.arrayOf(PropTypes.string), }).isRequired, + + // Disables the input field. E.g. disable when the drawer is covering the chat + isDisabled: PropTypes.bool.isRequired, }; const defaultProps = { @@ -269,6 +272,7 @@ class ReportActionCompose extends React.Component { onPasteFile={file => displayFileInModal({file})} shouldClear={this.state.textInputShouldClear} onClear={() => this.setTextInputShouldClear(false)} + isDisabled={this.props.isDisabled} /> diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index d2d292683762..fbd5977964c1 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -27,7 +27,7 @@ const propTypes = { }; function ReportView(props) { - const isDrawerOutOfTheWay = !props.isDrawerOpen || !props.isSmallScreenWidth; + const isComposeDisabled = props.isDrawerOpen && props.isSmallScreenWidth; return ( @@ -35,14 +35,13 @@ function ReportView(props) { - {isDrawerOutOfTheWay && ( - Keyboard.dismiss()}> - addAction(props.reportID, text)} - reportID={props.reportID} - /> - - )} + Keyboard.dismiss()}> + addAction(props.reportID, text)} + reportID={props.reportID} + /> + ); From c5dacac4387fb873a506f62f53b33abd235df53f Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 17:15:08 +0300 Subject: [PATCH 37/46] refactor: rename FullscreenLoadingIndicator --- .../{FullscreenLoading.js => FullscreenLoadingIndicator.js} | 0 src/pages/home/report/ReportView.js | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename src/components/{FullscreenLoading.js => FullscreenLoadingIndicator.js} (100%) diff --git a/src/components/FullscreenLoading.js b/src/components/FullscreenLoadingIndicator.js similarity index 100% rename from src/components/FullscreenLoading.js rename to src/components/FullscreenLoadingIndicator.js diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index fbd5977964c1..f51d2ee57629 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -8,7 +8,7 @@ import compose from '../../../libs/compose'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; -import FullScreenLoadingIndicator from '../../../components/FullscreenLoading'; +import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import withDrawerState from '../../../components/withDrawerState'; import withWindowDimensions from '../../../components/withWindowDimensions'; From 4646f1cd9d22962e0b7fd39941e25019d7d1423d Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 17:27:23 +0300 Subject: [PATCH 38/46] refactor: ReportScreen move the full screen loader to this screen --- src/pages/home/ReportScreen.js | 12 ++++++------ src/pages/home/report/ReportView.js | 6 ------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 572ee68aa10a..2449e80d685d 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -5,6 +5,7 @@ import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; import HeaderView from './HeaderView'; +import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator'; const propTypes = { /* Navigation api provided by react navigation */ @@ -63,8 +64,8 @@ class ReportScreen extends React.Component { * * @returns {Boolean} */ - isReadyToDisplayReport() { - return !this.state.isLoading && Boolean(this.getReportID()); + shouldShowLoader() { + return this.state.isLoading || !this.getReportID(); } /** @@ -93,11 +94,10 @@ class ReportScreen extends React.Component { onNavigationMenuButtonClicked={this.props.navigation.openDrawer} /> + {this.shouldShowLoader() && } + - + ); diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index f51d2ee57629..636ec6908cd5 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -8,7 +8,6 @@ import compose from '../../../libs/compose'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; -import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator'; import withDrawerState from '../../../components/withDrawerState'; import withWindowDimensions from '../../../components/withWindowDimensions'; @@ -16,9 +15,6 @@ const propTypes = { /* The ID of the report the selected report */ reportID: PropTypes.number.isRequired, - /* Is the view ready to be displayed */ - isReady: PropTypes.bool.isRequired, - /* Is the report view covered by the drawer */ isDrawerOpen: PropTypes.bool.isRequired, @@ -31,8 +27,6 @@ function ReportView(props) { return ( - {!props.isReady && } - Keyboard.dismiss()}> From 74eb0084adbfef53479525bf969e694eab352869 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 17:51:32 +0300 Subject: [PATCH 39/46] refactor: add `visible` prop to loader --- src/components/FullscreenLoadingIndicator.js | 10 +++++++++- src/pages/home/ReportScreen.js | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/FullscreenLoadingIndicator.js b/src/components/FullscreenLoadingIndicator.js index 1630253c8a56..56bea1f0acef 100644 --- a/src/components/FullscreenLoadingIndicator.js +++ b/src/components/FullscreenLoadingIndicator.js @@ -1,17 +1,25 @@ import React from 'react'; +import PropTypes from 'prop-types'; import {ActivityIndicator, StyleSheet, View} from 'react-native'; import styles from '../styles/styles'; import themeColors from '../styles/themes/default'; +const propTypes = { + /* Controls whether the loader is mounted and displayed */ + visible: PropTypes.bool.isRequired, +}; + /** * Loading indication component intended to cover the whole page, while the page prepares for initial render * * @returns {JSX.Element} */ -const FullScreenLoadingIndicator = () => ( +const FullScreenLoadingIndicator = ({visible}) => visible && ( ); +FullScreenLoadingIndicator.propTypes = propTypes; + export default FullScreenLoadingIndicator; diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 2449e80d685d..f72674df4456 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -94,7 +94,7 @@ class ReportScreen extends React.Component { onNavigationMenuButtonClicked={this.props.navigation.openDrawer} /> - {this.shouldShowLoader() && } + From 7072ed6231952d37eae55733cd33647f5d284658 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 18:13:14 +0300 Subject: [PATCH 40/46] refactor: ReportView cleanup unused styles --- src/pages/home/ReportScreen.js | 13 ++----------- src/pages/home/report/ReportView.js | 6 +++--- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index f72674df4456..ef252b8713e3 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -1,6 +1,5 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {View} from 'react-native'; import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; @@ -82,13 +81,7 @@ class ReportScreen extends React.Component { render() { return ( - + - - - + ); } diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index 636ec6908cd5..4427f075505a 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -22,11 +22,11 @@ const propTypes = { isSmallScreenWidth: PropTypes.bool.isRequired, }; -function ReportView(props) { +const ReportView = (props) => { const isComposeDisabled = props.isDrawerOpen && props.isSmallScreenWidth; return ( - + Keyboard.dismiss()}> @@ -39,7 +39,7 @@ function ReportView(props) { ); -} +}; ReportView.propTypes = propTypes; export default compose( From 9234aa889cd82cf37b4c3841762caded56548838 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 18:23:37 +0300 Subject: [PATCH 41/46] refactor: ReportScreen restore Navigation usage --- src/pages/home/ReportScreen.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index ef252b8713e3..1b580623b685 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -4,6 +4,7 @@ import styles from '../../styles/styles'; import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; import HeaderView from './HeaderView'; +import Navigation from '../../libs/Navigation/Navigation'; import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator'; const propTypes = { @@ -84,7 +85,7 @@ class ReportScreen extends React.Component { From f762e415d19e17ca42fe3b0dc0feb5322156f0ff Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 19:07:25 +0300 Subject: [PATCH 42/46] refactor: ReportScreen remove unneeded check --- src/pages/home/ReportScreen.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 1b580623b685..671a6dbd2a54 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -72,8 +72,6 @@ class ReportScreen extends React.Component { * Configures a small loading transition of fixed time and proceeds with rendering available data */ prepareTransition() { - if (!this.getReportID()) { return; } - this.setState({isLoading: true}); clearTimeout(this.loadingTimerId); From 328f1ba4356fe445135ec9d2a970009f01ceba91 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 19:08:14 +0300 Subject: [PATCH 43/46] refactor: ReportActionsView setup unread indicator during mount --- src/pages/home/report/ReportActionsView.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 9151c8747a5d..d41e7a3c0d86 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -76,10 +76,6 @@ class ReportActionsView extends React.Component { // Helper variable that keeps track of the unread action count before it updates to zero this.unreadActionCount = 0; - // Helper variable that prevents the unread indicator to show up for new messages - // received while the report is still active - this.shouldShowUnreadActionIndicator = true; - this.state = { isLoadingMoreChats: false, }; @@ -92,7 +88,8 @@ class ReportActionsView extends React.Component { subscribeToReportTypingEvents(this.props.reportID); this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); - fetchActions(this.props.reportID).catch(console.error); + fetchActions(this.props.reportID); + this.setUpUnreadActionIndicator(); Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } @@ -155,10 +152,6 @@ class ReportActionsView extends React.Component { * a flag to not show it again if the report is still open */ setUpUnreadActionIndicator() { - if (!this.shouldShowUnreadActionIndicator) { - return; - } - this.unreadActionCount = this.props.report.unreadActionCount; if (this.unreadActionCount > 0) { @@ -170,8 +163,6 @@ class ReportActionsView extends React.Component { }).start(); }, 3000)); } - - this.shouldShowUnreadActionIndicator = false; } /** From 3e5b5991c50127b0d18c8fae383ba24942372537 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 19:11:10 +0300 Subject: [PATCH 44/46] refactor: ReportScreen remove navigation prop - unused --- src/pages/home/ReportScreen.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 671a6dbd2a54..d9f0c932bb66 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -8,12 +8,6 @@ import Navigation from '../../libs/Navigation/Navigation'; import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator'; const propTypes = { - /* Navigation api provided by react navigation */ - navigation: PropTypes.shape({ - /* Display the drawer programmatically */ - openDrawer: PropTypes.func.isRequired, - }).isRequired, - /* Navigation route context info provided by react navigation */ route: PropTypes.shape({ /* Route specific parameters used on this screen */ From a89877cdbd15718431f879d9183101ec7877cd87 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 19:26:14 +0300 Subject: [PATCH 45/46] refactor: ReportActionCompose move logic related to disabling the input here This will: - remove focus / hide the keyboard when we go back to the LHN - prevent focusing on the input while it's covered by the LHN - enable the field when LHN is collapsed (or permanent) --- src/pages/home/report/ReportActionCompose.js | 16 ++++++-- src/pages/home/report/ReportView.js | 43 ++++++-------------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index e6bb7f7150fd..ac0f66fe8da8 100644 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -16,6 +16,8 @@ import ReportTypingIndicator from './ReportTypingIndicator'; import AttachmentModal from '../../../components/AttachmentModal'; import compose from '../../../libs/compose'; import CreateMenu from '../../../components/CreateMenu'; +import withWindowDimensions from '../../../components/withWindowDimensions'; +import withDrawerState from '../../../components/withDrawerState'; const propTypes = { // A method to call when the form is submitted @@ -40,8 +42,11 @@ const propTypes = { participants: PropTypes.arrayOf(PropTypes.string), }).isRequired, - // Disables the input field. E.g. disable when the drawer is covering the chat - isDisabled: PropTypes.bool.isRequired, + /* Is the report view covered by the drawer */ + isDrawerOpen: PropTypes.bool.isRequired, + + /* Is the window width narrow, like on a mobile device */ + isSmallScreenWidth: PropTypes.bool.isRequired, }; const defaultProps = { @@ -177,6 +182,9 @@ class ReportActionCompose extends React.Component { // eslint-disable-next-line no-unused-vars const hasMultipleParticipants = lodashGet(this.props.report, 'participants.length') > 1; + // Prevents focusing and showing the keyboard while the drawer is covering the chat. + const isComposeDisabled = this.props.isDrawerOpen && this.props.isSmallScreenWidth; + return ( displayFileInModal({file})} shouldClear={this.state.textInputShouldClear} onClear={() => this.setTextInputShouldClear(false)} - isDisabled={this.props.isDisabled} + isDisabled={isComposeDisabled} /> @@ -299,6 +307,8 @@ ReportActionCompose.propTypes = propTypes; ReportActionCompose.defaultProps = defaultProps; export default compose( + withWindowDimensions, + withDrawerState, withOnyx({ comment: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, diff --git a/src/pages/home/report/ReportView.js b/src/pages/home/report/ReportView.js index 4427f075505a..b00a8fb50628 100644 --- a/src/pages/home/report/ReportView.js +++ b/src/pages/home/report/ReportView.js @@ -4,45 +4,28 @@ import PropTypes from 'prop-types'; import ReportActionsView from './ReportActionsView'; import ReportActionCompose from './ReportActionCompose'; import {addAction} from '../../../libs/actions/Report'; -import compose from '../../../libs/compose'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import styles from '../../../styles/styles'; import SwipeableView from '../../../components/SwipeableView'; -import withDrawerState from '../../../components/withDrawerState'; -import withWindowDimensions from '../../../components/withWindowDimensions'; const propTypes = { /* The ID of the report the selected report */ reportID: PropTypes.number.isRequired, - - /* Is the report view covered by the drawer */ - isDrawerOpen: PropTypes.bool.isRequired, - - /* Is the window width narrow, like on a mobile device */ - isSmallScreenWidth: PropTypes.bool.isRequired, }; -const ReportView = (props) => { - const isComposeDisabled = props.isDrawerOpen && props.isSmallScreenWidth; +const ReportView = ({reportID}) => ( + + - return ( - - - - Keyboard.dismiss()}> - addAction(props.reportID, text)} - reportID={props.reportID} - /> - - - - ); -}; + Keyboard.dismiss()}> + addAction(reportID, text)} + reportID={reportID} + /> + + + +); ReportView.propTypes = propTypes; -export default compose( - withWindowDimensions, - withDrawerState, -)(ReportView); +export default ReportView; From 64c048fdcaa50c65e3713d358912d5a2fc3eb13c Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Thu, 8 Apr 2021 19:53:53 +0300 Subject: [PATCH 46/46] refactor: ReportScreen restore original navigation handling --- src/pages/home/ReportScreen.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index d9f0c932bb66..0438c2f6b910 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -5,6 +5,7 @@ import ReportView from './report/ReportView'; import ScreenWrapper from '../../components/ScreenWrapper'; import HeaderView from './HeaderView'; import Navigation from '../../libs/Navigation/Navigation'; +import ROUTES from '../../ROUTES'; import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator'; const propTypes = { @@ -77,7 +78,7 @@ class ReportScreen extends React.Component { Navigation.navigate(ROUTES.HOME)} />