From 830b95eb2038d7dc15b1e9d3b7ad2d0797c65063 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Mon, 17 May 2021 01:53:41 +0530 Subject: [PATCH 01/31] old Animation changes --- src/components/ScreenWrapper.js | 40 ++++- .../Navigation/AppNavigator/AuthScreens.js | 8 +- .../modalCardStyleInterpolator.js | 18 ++- src/pages/NewChatPage.js | 73 +++++---- src/pages/NewGroupPage.js | 117 +++++++------- src/pages/SearchPage.js | 75 ++++----- src/pages/home/sidebar/SidebarScreen.js | 2 +- src/pages/iou/IOUBillPage.js | 11 +- src/pages/iou/IOUModal.js | 144 ++++++++++-------- src/pages/iou/IOURequestPage.js | 11 +- 10 files changed, 279 insertions(+), 220 deletions(-) diff --git a/src/components/ScreenWrapper.js b/src/components/ScreenWrapper.js index 9a44a4a11f72..7ab7d5f8ff43 100644 --- a/src/components/ScreenWrapper.js +++ b/src/components/ScreenWrapper.js @@ -24,9 +24,15 @@ const propTypes = { /** Whether to include padding top */ includePaddingTop: PropTypes.bool, - /** react-navigation object that will allow us to goBack() */ + // Called when navigated Screen's transition is finished. + onTransitionEnd: PropTypes.func, + + // react-navigation navigation object available to screen components navigation: PropTypes.shape({ - /** Returns to the previous navigation state e.g. if this is inside a Modal we will dismiss it */ + // Method to attach listner to Navigaton state. + addListener: PropTypes.func.isRequired, + + // Returns to the previous navigation state e.g. if this is inside a Modal we will dismiss it goBack: PropTypes.func, }), }; @@ -35,24 +41,39 @@ const defaultProps = { style: [], includePaddingBottom: true, includePaddingTop: true, + onTransitionEnd: () => {}, navigation: { + addListener: () => {}, goBack: () => {}, }, }; class ScreenWrapper extends React.Component { + constructor(props) { + super(props); + this.state = { + didScreenTransitionEnd: false, + }; + } + componentDidMount() { - this.unsubscribe = KeyboardShortcut.subscribe('Escape', () => { + this.unsubscribeEscapeKey = KeyboardShortcut.subscribe('Escape', () => { this.props.navigation.goBack(); }, [], true); + + this.unsubscribeTransitionEnd = this.props.navigation.addListener('transitionEnd', () => { + this.setState({didScreenTransitionEnd: true}); + this.props.onTransitionEnd(); + }); } componentWillUnmount() { - if (!this.unsubscribe) { - return; + if (this.unsubscribeEscapeKey) { + this.unsubscribeEscapeKey(); + } + if (this.unsubscribeTransitionEnd) { + this.unsubscribeTransitionEnd(); } - - this.unsubscribe(); } render() { @@ -80,7 +101,10 @@ class ScreenWrapper extends React.Component { {// If props.children is a function, call it to provide the insets to the children. _.isFunction(this.props.children) - ? this.props.children(insets) + ? this.props.children({ + insets, + didScreenTransitionEnd: this.state.didScreenTransitionEnd, + }) : this.props.children } diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 7017dcdca8e0..69a9f41b3534 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -68,10 +68,12 @@ Onyx.connect({ const RootStack = createCustomModalStackNavigator(); -// When modal screen gets focused, update modal visibility in Onyx +// We want to delay the re-rendering for components(e.g. ReportActionCompose) +// that depends on modal visibility until Modal is completely closed or its transition has ended +// When modal screen is focused and animation transition is ended, update modal visibility in Onyx // https://reactnavigation.org/docs/navigation-events/ const modalScreenListeners = { - focus: () => { + transitionEnd: () => { setModalVisibility(true); }, beforeRemove: () => { @@ -159,7 +161,7 @@ class AuthScreens extends React.Component { const modalScreenOptions = { headerShown: false, cardStyle: getNavigationModalCardStyle(this.props.isSmallScreenWidth), - cardStyleInterpolator: modalCardStyleInterpolator, + cardStyleInterpolator: props => modalCardStyleInterpolator(this.props.isSmallScreenWidth, props), animationEnabled: true, gestureDirection: 'horizontal', cardOverlayEnabled: true, diff --git a/src/libs/Navigation/AppNavigator/modalCardStyleInterpolator.js b/src/libs/Navigation/AppNavigator/modalCardStyleInterpolator.js index 28b85c68b7ec..8ad5b1603cdd 100644 --- a/src/libs/Navigation/AppNavigator/modalCardStyleInterpolator.js +++ b/src/libs/Navigation/AppNavigator/modalCardStyleInterpolator.js @@ -1,15 +1,19 @@ import {Animated} from 'react-native'; +import variables from '../../../styles/variables'; -export default ({ - current: {progress}, - inverted, - layouts: { - screen, +export default ( + isSmallScreen, + { + current: {progress}, + inverted, + layouts: { + screen, + }, }, -}) => { +) => { const translateX = Animated.multiply(progress.interpolate({ inputRange: [0, 1], - outputRange: [screen.width, 0], + outputRange: [isSmallScreen ? screen.width : variables.sideBarWidth, 0], extrapolate: 'clamp', }), inverted); diff --git a/src/pages/NewChatPage.js b/src/pages/NewChatPage.js index b87a755f288f..535e51b4bdad 100755 --- a/src/pages/NewChatPage.js +++ b/src/pages/NewChatPage.js @@ -12,6 +12,7 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../components/wit import HeaderWithCloseButton from '../components/HeaderWithCloseButton'; import Navigation from '../libs/Navigation/Navigation'; import ScreenWrapper from '../components/ScreenWrapper'; +import FullScreenLoadingIndicator from '../components/FullscreenLoadingIndicator'; import withLocalize, {withLocalizePropTypes} from '../components/withLocalize'; import compose from '../libs/compose'; @@ -52,7 +53,6 @@ class NewChatPage extends Component { super(props); this.createNewChat = this.createNewChat.bind(this); - const { personalDetails, userToInvite, @@ -117,38 +117,45 @@ class NewChatPage extends Component { return ( - Navigation.dismissModal(true)} - /> - - { - const { - personalDetails, - userToInvite, - } = getNewChatOptions( - this.props.reports, - this.props.personalDetails, - searchValue, - ); - this.setState({ - searchValue, - userToInvite, - personalDetails, - }); - }} - headerMessage={headerMessage} - hideSectionHeaders - disableArrowKeysActions - hideAdditionalOptionStates - forceTextUnreadStyle - /> - - + {({didScreenTransitionEnd}) => ( + <> + Navigation.dismissModal(true)} + /> + + + {didScreenTransitionEnd && ( + { + const { + personalDetails, + userToInvite, + } = getNewChatOptions( + this.props.reports, + this.props.personalDetails, + searchValue, + ); + this.setState({ + searchValue, + userToInvite, + personalDetails, + }); + }} + headerMessage={headerMessage} + hideSectionHeaders + disableArrowKeysActions + hideAdditionalOptionStates + forceTextUnreadStyle + /> + )} + + + + )} ); } diff --git a/src/pages/NewGroupPage.js b/src/pages/NewGroupPage.js index 0531248da993..a8a2898b62de 100755 --- a/src/pages/NewGroupPage.js +++ b/src/pages/NewGroupPage.js @@ -14,6 +14,7 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../components/wit import HeaderWithCloseButton from '../components/HeaderWithCloseButton'; import ScreenWrapper from '../components/ScreenWrapper'; import Navigation from '../libs/Navigation/Navigation'; +import FullScreenLoadingIndicator from '../components/FullscreenLoadingIndicator'; import withLocalize, {withLocalizePropTypes} from '../components/withLocalize'; import compose from '../libs/compose'; @@ -55,7 +56,6 @@ class NewGroupPage extends Component { this.toggleOption = this.toggleOption.bind(this); this.createGroup = this.createGroup.bind(this); - const { recentReports, personalDetails, @@ -185,60 +185,69 @@ class NewGroupPage extends Component { ); return ( - Navigation.dismissModal(true)} - /> - - { - const { - recentReports, - personalDetails, - userToInvite, - } = getNewGroupOptions( - this.props.reports, - this.props.personalDetails, - searchValue, - [], - ); - this.setState({ - searchValue, - userToInvite, - recentReports, - personalDetails, - }); - }} - headerMessage={headerMessage} - disableArrowKeysActions - hideAdditionalOptionStates - forceTextUnreadStyle - shouldFocusOnSelectRow - /> - {this.state.selectedOptions?.length > 0 && ( - - [ - styles.button, - styles.buttonSuccess, - styles.w100, - hovered && styles.buttonSuccessHovered, - ]} - > - - {this.props.translate('newGroupPage.createGroup')} - - + {({didScreenTransitionEnd}) => ( + <> + Navigation.dismissModal(true)} + /> + + + {didScreenTransitionEnd && ( + <> + { + const { + recentReports, + personalDetails, + userToInvite, + } = getNewGroupOptions( + this.props.reports, + this.props.personalDetails, + searchValue, + [], + ); + this.setState({ + searchValue, + userToInvite, + recentReports, + personalDetails, + }); + }} + headerMessage={headerMessage} + disableArrowKeysActions + hideAdditionalOptionStates + forceTextUnreadStyle + shouldFocusOnSelectRow + /> + {this.state.selectedOptions?.length > 0 && ( + + [ + styles.button, + styles.buttonSuccess, + styles.w100, + hovered && styles.buttonSuccessHovered, + ]} + > + + {this.props.translate('newGroupPage.createGroup')} + + + + )} + + )} - )} - - + + + )} ); } diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index dff19ddea251..72e28b86d3e9 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -15,6 +15,7 @@ import HeaderWithCloseButton from '../components/HeaderWithCloseButton'; import ScreenWrapper from '../components/ScreenWrapper'; import Timing from '../libs/actions/Timing'; import CONST from '../CONST'; +import FullScreenLoadingIndicator from '../components/FullscreenLoadingIndicator'; import withLocalize, {withLocalizePropTypes} from '../components/withLocalize'; import compose from '../libs/compose'; @@ -60,7 +61,6 @@ class SearchPage extends Component { Timing.start(CONST.TIMING.SEARCH_RENDER); this.selectReport = this.selectReport.bind(this); - const { recentReports, personalDetails, @@ -141,39 +141,46 @@ class SearchPage extends Component { ); return ( - Navigation.dismissModal(true)} - /> - - { - const { - recentReports, - personalDetails, - userToInvite, - } = getSearchOptions( - this.props.reports, - this.props.personalDetails, - searchValue, - ); - this.setState({ - searchValue, - userToInvite, - recentReports, - personalDetails, - }); - }} - headerMessage={headerMessage} - hideSectionHeaders - hideAdditionalOptionStates - showTitleTooltip - /> - - + {({didScreenTransitionEnd}) => ( + <> + Navigation.dismissModal(true)} + /> + + + {didScreenTransitionEnd && ( + { + const { + recentReports, + personalDetails, + userToInvite, + } = getSearchOptions( + this.props.reports, + this.props.personalDetails, + searchValue, + ); + this.setState({ + searchValue, + userToInvite, + recentReports, + personalDetails, + }); + }} + headerMessage={headerMessage} + hideSectionHeaders + hideAdditionalOptionStates + showTitleTooltip + /> + )} + + + + )} ); } diff --git a/src/pages/home/sidebar/SidebarScreen.js b/src/pages/home/sidebar/SidebarScreen.js index dabf849b677e..5d03df697d74 100755 --- a/src/pages/home/sidebar/SidebarScreen.js +++ b/src/pages/home/sidebar/SidebarScreen.js @@ -80,7 +80,7 @@ class SidebarScreen extends Component { includePaddingBottom={false} style={[styles.sidebar]} > - {insets => ( + {({insets}) => ( <> ( - - {() => ( - // eslint-disable-next-line react/jsx-props-no-spreading - - )} - -); +// eslint-disable-next-line react/jsx-props-no-spreading +export default props => ; diff --git a/src/pages/iou/IOUModal.js b/src/pages/iou/IOUModal.js index 94280ed35365..52cc53fa1efa 100755 --- a/src/pages/iou/IOUModal.js +++ b/src/pages/iou/IOUModal.js @@ -16,6 +16,8 @@ import ONYXKEYS from '../../ONYXKEYS'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import compose from '../../libs/compose'; import {getPersonalDetailsForLogins} from '../../libs/OptionsListUtils'; +import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator'; +import ScreenWrapper from '../../components/ScreenWrapper'; /** * IOU modal for requesting money and splitting bills. @@ -37,6 +39,9 @@ const propTypes = { /** Whether or not transaction creation has resulted to error */ error: PropTypes.bool, + + // is loading + loading: PropTypes.bool, }).isRequired, /** Personal details of all the users */ @@ -78,6 +83,7 @@ class IOUModal extends Component { this.createTransaction = this.createTransaction.bind(this); this.updateComment = this.updateComment.bind(this); this.addParticipants = this.addParticipants.bind(this); + this.getReady = this.getReady.bind(this); const participants = lodashGet(props, 'report.participants', []); const participantsWithDetails = getPersonalDetailsForLogins(participants, props.personalDetails) .map(personalDetails => ({ @@ -107,10 +113,6 @@ class IOUModal extends Component { } } - componentDidMount() { - getPreferredCurrency(); - } - componentDidUpdate(prevProps) { // Successfully close the modal if transaction creation has ended and there is no error if (prevProps.iou.creatingIOUTransaction && !this.props.iou.creatingIOUTransaction && !this.props.iou.error) { @@ -118,6 +120,12 @@ class IOUModal extends Component { } } + + getReady() { + getPreferredCurrency(); + } + + /** * Retrieve title for current step, based upon current step and type of IOU * @@ -216,67 +224,79 @@ class IOUModal extends Component { render() { const currentStep = this.steps[this.state.currentStepIndex]; return ( - <> - - - {this.state.currentStepIndex > 0 - && ( - + {({didScreenTransitionEnd}) => ( + <> + + - - - )} -
- - Navigation.dismissModal()} - style={[styles.touchableButtonImage]} - > - - + {this.state.currentStepIndex > 0 + && ( + + + + )} +
+ + Navigation.dismissModal()} + style={[styles.touchableButtonImage]} + > + + + + - - - {currentStep === Steps.IOUAmount && ( - { - this.setState({amount}); - this.navigateToNextStep(); - }} - currencySelected={this.currencySelected} - selectedCurrency={this.state.selectedCurrency} - /> - )} - {currentStep === Steps.IOUParticipants && ( - - )} - {currentStep === Steps.IOUConfirm && ( - + + + {didScreenTransitionEnd && ( + <> + {currentStep === Steps.IOUAmount && ( + { + this.setState({amount}); + this.navigateToNextStep(); + }} + currencySelected={this.currencySelected} + selectedCurrency={this.state.selectedCurrency} + /> + )} + {currentStep === Steps.IOUParticipants && ( + + )} + {currentStep === Steps.IOUConfirm && ( + + )} + + )} + + + )} - + ); } } diff --git a/src/pages/iou/IOURequestPage.js b/src/pages/iou/IOURequestPage.js index 67ee9d0f4dd4..6cacd162c105 100644 --- a/src/pages/iou/IOURequestPage.js +++ b/src/pages/iou/IOURequestPage.js @@ -1,12 +1,5 @@ import React from 'react'; import IOUModal from './IOUModal'; -import ScreenWrapper from '../../components/ScreenWrapper'; -export default props => ( - - {() => ( - // eslint-disable-next-line react/jsx-props-no-spreading - - )} - -); +// eslint-disable-next-line react/jsx-props-no-spreading +export default props => ; From c66c698241cce07b62ffe1e042d363d1050e8b12 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 17 May 2021 17:09:14 +0800 Subject: [PATCH 02/31] Fix issues with search page --- src/components/OptionsList.js | 6 ++++++ src/components/OptionsSelector.js | 6 ++++++ src/libs/OptionsListUtils.js | 5 ++++- src/pages/SearchPage.js | 1 + src/pages/home/sidebar/OptionRow.js | 8 +++++++- 5 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index 5629831e7f49..35b67ad50e57 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -74,6 +74,10 @@ const propTypes = { /** Toggle between compact and default view of the option */ optionMode: PropTypes.oneOf(['compact', 'default']), + + /* Whether we should display full display names for each option. + * If this is false, then we only display each participant's first name */ + shouldShowFullDisplayNames: PropTypes.bool, }; const defaultProps = { @@ -94,6 +98,7 @@ const defaultProps = { innerRef: null, showTitleTooltip: false, optionMode: undefined, + shouldShowFullDisplayNames: false, }; class OptionsList extends Component { @@ -170,6 +175,7 @@ class OptionsList extends Component { showSelectedState={this.props.canSelectMultipleOptions} hideAdditionalOptionStates={this.props.hideAdditionalOptionStates} forceTextUnreadStyle={this.props.forceTextUnreadStyle} + shouldShowFullDisplayNames={this.props.shouldShowFullDisplayNames} /> ); } diff --git a/src/components/OptionsSelector.js b/src/components/OptionsSelector.js index 341127cdb1e7..ed89e80d182e 100755 --- a/src/components/OptionsSelector.js +++ b/src/components/OptionsSelector.js @@ -64,6 +64,10 @@ const propTypes = { /** Whether to focus the textinput after an option is selected */ shouldFocusOnSelectRow: PropTypes.bool, + /* Whether we should display full display names for each option. + * If this is false, then we only display each participant's first name */ + shouldShowFullDisplayNames: PropTypes.bool, + ...withLocalizePropTypes, }; @@ -79,6 +83,7 @@ const defaultProps = { forceTextUnreadStyle: false, showTitleTooltip: false, shouldFocusOnSelectRow: false, + shouldShowFullDisplayNames: false, }; class OptionsSelector extends Component { @@ -217,6 +222,7 @@ class OptionsSelector extends Component { hideAdditionalOptionStates={this.props.hideAdditionalOptionStates} forceTextUnreadStyle={this.props.forceTextUnreadStyle} showTitleTooltip={this.props.showTitleTooltip} + shouldShowFullDisplayNames={this.props.shouldShowFullDisplayNames} /> ); diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 9bf836a563bc..f059ed0b2b8e 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -64,6 +64,7 @@ function getSearchText(report, personalDetailList) { }); if (report) { + searchTerms.push(report.reportName); searchTerms.push(...report.reportName.split(',').map(name => name.trim())); searchTerms.push(...report.participants); } @@ -97,7 +98,7 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - return { + const res = { text: report ? report.reportName : personalDetail.displayName, alternateText: (showChatPreviewLine && lastMessageText) ? lastMessageText @@ -118,6 +119,8 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie hasOutstandingIOU, iouReportID: lodashGet(report, 'iouReportID'), }; + + return res; } Onyx.connect({ diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index dff19ddea251..6f67f4c1d2f6 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -171,6 +171,7 @@ class SearchPage extends Component { hideSectionHeaders hideAdditionalOptionStates showTitleTooltip + shouldShowFullDisplayNames /> diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index c2d302d7d3e7..14da2511dfce 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -52,6 +52,10 @@ const propTypes = { /** Toggle between compact and default view */ mode: PropTypes.oneOf(['compact', 'default']), + + /* Whether we should display full display names for each option. + * If this is false, then we only display each participant's first name */ + shouldShowFullDisplayNames: PropTypes.bool, }; const defaultProps = { @@ -64,6 +68,7 @@ const defaultProps = { showTitleTooltip: false, mode: 'default', onSelectRow: null, + shouldShowFullDisplayNames: false, }; const OptionRow = ({ @@ -78,6 +83,7 @@ const OptionRow = ({ forceTextUnreadStyle, showTitleTooltip, mode, + shouldShowFullDisplayNames, }) => { const textStyle = optionIsFocused ? styles.sidebarLinkActiveText @@ -114,7 +120,7 @@ const OptionRow = ({ const displayNamesWithTooltips = _.map( option.participantsList, ({displayName, firstName, login}) => ( - {displayName: (isMultipleParticipant ? firstName : displayName) || login, tooltip: login} + {displayName: (isMultipleParticipant && !shouldShowFullDisplayNames ? firstName : displayName) || login, tooltip: login} ), ); const fullTitle = displayNamesWithTooltips.map(({displayName}) => displayName).join(', '); From 89a99f9a577d7bee8398cf96adf8af78140395bf Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 17 May 2021 17:20:51 +0800 Subject: [PATCH 03/31] Fix comment --- src/components/OptionsList.js | 2 +- src/components/OptionsSelector.js | 2 +- src/pages/home/sidebar/OptionRow.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index 35b67ad50e57..4834d299ab21 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -76,7 +76,7 @@ const propTypes = { optionMode: PropTypes.oneOf(['compact', 'default']), /* Whether we should display full display names for each option. - * If this is false, then we only display each participant's first name */ + * If this is false, then we only display each participant's first name for group chats */ shouldShowFullDisplayNames: PropTypes.bool, }; diff --git a/src/components/OptionsSelector.js b/src/components/OptionsSelector.js index ed89e80d182e..dbf64897341e 100755 --- a/src/components/OptionsSelector.js +++ b/src/components/OptionsSelector.js @@ -65,7 +65,7 @@ const propTypes = { shouldFocusOnSelectRow: PropTypes.bool, /* Whether we should display full display names for each option. - * If this is false, then we only display each participant's first name */ + * If this is false, then we only display each participant's first name for group chats */ shouldShowFullDisplayNames: PropTypes.bool, ...withLocalizePropTypes, diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 14da2511dfce..dcd983b9735e 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -54,7 +54,7 @@ const propTypes = { mode: PropTypes.oneOf(['compact', 'default']), /* Whether we should display full display names for each option. - * If this is false, then we only display each participant's first name */ + * If this is false, then we only display each participant's first name for group chats */ shouldShowFullDisplayNames: PropTypes.bool, }; From 1ed4111f339051e81653771e4a688eff5f318c8c Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 17 May 2021 17:21:44 +0800 Subject: [PATCH 04/31] Fix comment --- src/components/OptionsList.js | 4 ++-- src/components/OptionsSelector.js | 4 ++-- src/pages/home/sidebar/OptionRow.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index 4834d299ab21..f1b0e125fd32 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -75,8 +75,8 @@ const propTypes = { /** Toggle between compact and default view of the option */ optionMode: PropTypes.oneOf(['compact', 'default']), - /* Whether we should display full display names for each option. - * If this is false, then we only display each participant's first name for group chats */ + /* Whether we should display full display names of participants for group chat options. + * If this is false, then we only display each participant's first name. */ shouldShowFullDisplayNames: PropTypes.bool, }; diff --git a/src/components/OptionsSelector.js b/src/components/OptionsSelector.js index dbf64897341e..9d205386efd5 100755 --- a/src/components/OptionsSelector.js +++ b/src/components/OptionsSelector.js @@ -64,8 +64,8 @@ const propTypes = { /** Whether to focus the textinput after an option is selected */ shouldFocusOnSelectRow: PropTypes.bool, - /* Whether we should display full display names for each option. - * If this is false, then we only display each participant's first name for group chats */ + /* Whether we should display full display names of participants for group chat options. + * If this is false, then we only display each participant's first name. */ shouldShowFullDisplayNames: PropTypes.bool, ...withLocalizePropTypes, diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index dcd983b9735e..080f8b054072 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -53,8 +53,8 @@ const propTypes = { /** Toggle between compact and default view */ mode: PropTypes.oneOf(['compact', 'default']), - /* Whether we should display full display names for each option. - * If this is false, then we only display each participant's first name for group chats */ + /* Whether we should display full display names of participants for group chat options. + * If this is false, then we only display each participant's first name. */ shouldShowFullDisplayNames: PropTypes.bool, }; From 4afe449c324441a2b4767f9073b1aa1a4df7ffac Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 17 May 2021 17:22:13 +0800 Subject: [PATCH 05/31] Remove debug --- src/libs/OptionsListUtils.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index f059ed0b2b8e..464e676c7c46 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -98,7 +98,7 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - const res = { + return { text: report ? report.reportName : personalDetail.displayName, alternateText: (showChatPreviewLine && lastMessageText) ? lastMessageText @@ -119,8 +119,6 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie hasOutstandingIOU, iouReportID: lodashGet(report, 'iouReportID'), }; - - return res; } Onyx.connect({ From 0db6f8ce332a9cdb7fa2917431bee09d45989c6f Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 17 May 2021 17:23:36 +0800 Subject: [PATCH 06/31] Make prop required since it will always be included --- src/pages/home/sidebar/OptionRow.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 080f8b054072..9524ab1fb49a 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -55,7 +55,7 @@ const propTypes = { /* Whether we should display full display names of participants for group chat options. * If this is false, then we only display each participant's first name. */ - shouldShowFullDisplayNames: PropTypes.bool, + shouldShowFullDisplayNames: PropTypes.bool.isRequired, }; const defaultProps = { @@ -68,7 +68,6 @@ const defaultProps = { showTitleTooltip: false, mode: 'default', onSelectRow: null, - shouldShowFullDisplayNames: false, }; const OptionRow = ({ From 2d1f7c7c89da14c665b31c76c2c661aeeef5c657 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 09:26:21 +0800 Subject: [PATCH 07/31] style --- src/pages/home/sidebar/OptionRow.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 9524ab1fb49a..123127453e9e 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -118,9 +118,10 @@ const OptionRow = ({ const isMultipleParticipant = option.participantsList.length > 1; const displayNamesWithTooltips = _.map( option.participantsList, - ({displayName, firstName, login}) => ( - {displayName: (isMultipleParticipant && !shouldShowFullDisplayNames ? firstName : displayName) || login, tooltip: login} - ), + ({displayName, firstName, login}) => ({ + displayName: (isMultipleParticipant && !shouldShowFullDisplayNames ? firstName : displayName) || login, + tooltip: login, + }), ); const fullTitle = displayNamesWithTooltips.map(({displayName}) => displayName).join(', '); return ( From c2629b9d66f981b56f1526f34d7542080157f7a4 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 18:58:43 +0800 Subject: [PATCH 08/31] Use set --- src/libs/OptionsListUtils.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 464e676c7c46..01533f406e31 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -56,20 +56,23 @@ function getPersonalDetailsForLogins(logins, personalDetails) { * @return {String} */ function getSearchText(report, personalDetailList) { - const searchTerms = []; + // Use a set because it skips adding duplicates + // which could potentially significantly speed up our + // regex search if someone has thousands of chats + const searchTerms = new Set(); _.each(personalDetailList, (personalDetail) => { - searchTerms.push(personalDetail.displayName); - searchTerms.push(personalDetail.login); + searchTerms.add(personalDetail.displayName); + searchTerms.add(personalDetail.login); }); if (report) { - searchTerms.push(report.reportName); - searchTerms.push(...report.reportName.split(',').map(name => name.trim())); - searchTerms.push(...report.participants); + searchTerms.add(report.reportName); + searchTerms.add(...report.reportName.split(',').map(name => name.trim())); + searchTerms.add(...report.participants); } - return _.unique(searchTerms).join(' '); + return _.unique(Array.from(searchTerms)).join(' '); } /** From 905393832b31f4024f9824804061ebc9b6c60f1f Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 19:00:02 +0800 Subject: [PATCH 09/31] Update src/components/OptionsList.js Co-authored-by: Marc Glasser --- src/components/OptionsList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index f1b0e125fd32..96845eb999a6 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -75,7 +75,7 @@ const propTypes = { /** Toggle between compact and default view of the option */ optionMode: PropTypes.oneOf(['compact', 'default']), - /* Whether we should display full display names of participants for group chat options. + /** Whether we should display full display names of participants for group chat options. * If this is false, then we only display each participant's first name. */ shouldShowFullDisplayNames: PropTypes.bool, }; From 9e203cd51b86b9693fa8da1c06ed6315b44fb8cf Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 19:01:39 +0800 Subject: [PATCH 10/31] Remove full display names changes --- src/components/OptionsList.js | 6 ------ src/components/OptionsSelector.js | 6 ------ src/pages/SearchPage.js | 1 - src/pages/home/sidebar/OptionRow.js | 7 +------ 4 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index f1b0e125fd32..5629831e7f49 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -74,10 +74,6 @@ const propTypes = { /** Toggle between compact and default view of the option */ optionMode: PropTypes.oneOf(['compact', 'default']), - - /* Whether we should display full display names of participants for group chat options. - * If this is false, then we only display each participant's first name. */ - shouldShowFullDisplayNames: PropTypes.bool, }; const defaultProps = { @@ -98,7 +94,6 @@ const defaultProps = { innerRef: null, showTitleTooltip: false, optionMode: undefined, - shouldShowFullDisplayNames: false, }; class OptionsList extends Component { @@ -175,7 +170,6 @@ class OptionsList extends Component { showSelectedState={this.props.canSelectMultipleOptions} hideAdditionalOptionStates={this.props.hideAdditionalOptionStates} forceTextUnreadStyle={this.props.forceTextUnreadStyle} - shouldShowFullDisplayNames={this.props.shouldShowFullDisplayNames} /> ); } diff --git a/src/components/OptionsSelector.js b/src/components/OptionsSelector.js index 9d205386efd5..341127cdb1e7 100755 --- a/src/components/OptionsSelector.js +++ b/src/components/OptionsSelector.js @@ -64,10 +64,6 @@ const propTypes = { /** Whether to focus the textinput after an option is selected */ shouldFocusOnSelectRow: PropTypes.bool, - /* Whether we should display full display names of participants for group chat options. - * If this is false, then we only display each participant's first name. */ - shouldShowFullDisplayNames: PropTypes.bool, - ...withLocalizePropTypes, }; @@ -83,7 +79,6 @@ const defaultProps = { forceTextUnreadStyle: false, showTitleTooltip: false, shouldFocusOnSelectRow: false, - shouldShowFullDisplayNames: false, }; class OptionsSelector extends Component { @@ -222,7 +217,6 @@ class OptionsSelector extends Component { hideAdditionalOptionStates={this.props.hideAdditionalOptionStates} forceTextUnreadStyle={this.props.forceTextUnreadStyle} showTitleTooltip={this.props.showTitleTooltip} - shouldShowFullDisplayNames={this.props.shouldShowFullDisplayNames} /> ); diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index 6f67f4c1d2f6..dff19ddea251 100755 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -171,7 +171,6 @@ class SearchPage extends Component { hideSectionHeaders hideAdditionalOptionStates showTitleTooltip - shouldShowFullDisplayNames /> diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 123127453e9e..cff658c54d03 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -52,10 +52,6 @@ const propTypes = { /** Toggle between compact and default view */ mode: PropTypes.oneOf(['compact', 'default']), - - /* Whether we should display full display names of participants for group chat options. - * If this is false, then we only display each participant's first name. */ - shouldShowFullDisplayNames: PropTypes.bool.isRequired, }; const defaultProps = { @@ -82,7 +78,6 @@ const OptionRow = ({ forceTextUnreadStyle, showTitleTooltip, mode, - shouldShowFullDisplayNames, }) => { const textStyle = optionIsFocused ? styles.sidebarLinkActiveText @@ -119,7 +114,7 @@ const OptionRow = ({ const displayNamesWithTooltips = _.map( option.participantsList, ({displayName, firstName, login}) => ({ - displayName: (isMultipleParticipant && !shouldShowFullDisplayNames ? firstName : displayName) || login, + displayName: (isMultipleParticipant ? firstName : displayName) || login, tooltip: login, }), ); From 7908e9b09b5ec1ca3658a50bde5d4d3fcf2e2572 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 19:05:17 +0800 Subject: [PATCH 11/31] Remove leftover unused --- src/components/OptionsList.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index 00417499db48..5629831e7f49 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -94,7 +94,6 @@ const defaultProps = { innerRef: null, showTitleTooltip: false, optionMode: undefined, - shouldShowFullDisplayNames: false, }; class OptionsList extends Component { @@ -171,7 +170,6 @@ class OptionsList extends Component { showSelectedState={this.props.canSelectMultipleOptions} hideAdditionalOptionStates={this.props.hideAdditionalOptionStates} forceTextUnreadStyle={this.props.forceTextUnreadStyle} - shouldShowFullDisplayNames={this.props.shouldShowFullDisplayNames} /> ); } From 194653b757c2ec1ad39e0e94e3f0d66e4af5db96 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 19:06:02 +0800 Subject: [PATCH 12/31] Remove leftover unused --- src/pages/home/sidebar/OptionRow.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 62d9abc721fc..cff658c54d03 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -78,7 +78,6 @@ const OptionRow = ({ forceTextUnreadStyle, showTitleTooltip, mode, - shouldShowFullDisplayNames, }) => { const textStyle = optionIsFocused ? styles.sidebarLinkActiveText @@ -115,7 +114,7 @@ const OptionRow = ({ const displayNamesWithTooltips = _.map( option.participantsList, ({displayName, firstName, login}) => ({ - displayName: (isMultipleParticipant && !shouldShowFullDisplayNames ? firstName : displayName) || login, + displayName: (isMultipleParticipant ? firstName : displayName) || login, tooltip: login, }), ); From 328cca6027980914fb34362e61f2f22d6797acb0 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 19:07:19 +0800 Subject: [PATCH 13/31] Add back stuff messed up by merge --- src/pages/home/sidebar/OptionRow.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index cff658c54d03..e98084c09e73 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -52,6 +52,9 @@ const propTypes = { /** Toggle between compact and default view */ mode: PropTypes.oneOf(['compact', 'default']), + + // Whether this option should be disabled + isDisabled: PropTypes.bool, }; const defaultProps = { @@ -64,6 +67,7 @@ const defaultProps = { showTitleTooltip: false, mode: 'default', onSelectRow: null, + isDisabled: false, }; const OptionRow = ({ @@ -77,6 +81,7 @@ const OptionRow = ({ isSelected, forceTextUnreadStyle, showTitleTooltip, + isDisabled, mode, }) => { const textStyle = optionIsFocused @@ -134,6 +139,7 @@ const OptionRow = ({ getBackgroundColorStyle(backgroundColor), optionIsFocused ? styles.sidebarLinkActive : null, hovered && !optionIsFocused ? hoverStyle : null, + isDisabled && styles.cursorDisabled, ]} > From fd39d88e5ba1c720135b8c3557bd9d310145018d Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 18 May 2021 19:08:12 +0800 Subject: [PATCH 14/31] Remove unneeded whitespace change --- src/pages/home/sidebar/OptionRow.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index e98084c09e73..d15bd013e49d 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -118,10 +118,9 @@ const OptionRow = ({ const isMultipleParticipant = option.participantsList.length > 1; const displayNamesWithTooltips = _.map( option.participantsList, - ({displayName, firstName, login}) => ({ - displayName: (isMultipleParticipant ? firstName : displayName) || login, - tooltip: login, - }), + ({displayName, firstName, login}) => ( + {displayName: (isMultipleParticipant ? firstName : displayName) || login, tooltip: login} + ), ); const fullTitle = displayNamesWithTooltips.map(({displayName}) => displayName).join(', '); return ( From e3d642493a7dc8e8cc7807795c2d0f834faa4ae0 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 24 May 2021 15:45:16 +0800 Subject: [PATCH 15/31] Use participants set in search --- src/libs/OptionsListUtils.js | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 01533f406e31..b624c40fc505 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -100,7 +100,21 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie + _.unescape(report.lastMessageText) : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - + const participantsSet = new Set(); + personalDetailList.forEach((participant) => { + if (participant.login) { + participantsSet.add(participant.login.toLowerCase()); + } + if (participant.firstName) { + participantsSet.add(participant.firstName.toLowerCase()); + } + if (participant.lastName) { + participantsSet.add(participant.lastName.toLowerCase()); + } + if (participant.displayName) { + participantsSet.add(participant.displayName.toLowerCase()); + } + }); return { text: report ? report.reportName : personalDetail.displayName, alternateText: (showChatPreviewLine && lastMessageText) @@ -109,6 +123,7 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie icons: report ? report.icons : [personalDetail.avatar], tooltipText, participantsList: personalDetailList, + participantsSet, // It doesn't make sense to provide a login in the case of a report with multiple participants since // there isn't any one single login to refer to for a report. @@ -144,14 +159,16 @@ Onyx.connect({ * * @param {String} searchValue * @param {String} searchText + * @param {Set} participantsSet * @returns {Boolean} */ -function isSearchStringMatch(searchValue, searchText) { +function isSearchStringMatch(searchValue, searchText, participantsSet) { const searchWords = searchValue.split(' '); return _.every(searchWords, (word) => { const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); const valueToSearch = searchText && searchText.replace(new RegExp(/ /g), ''); - return matchRegex.test(valueToSearch); + return matchRegex.test(valueToSearch) + || participantsSet.has(word.trim().replace(new RegExp(/,/g), '')); }); } @@ -255,7 +272,7 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { } // Finally check to see if this options is a match for the provided search string if we have one - if (searchValue && !isSearchStringMatch(searchValue, reportOption.searchText)) { + if (searchValue && !isSearchStringMatch(searchValue, reportOption.searchText, reportOption.participantsSet)) { continue; } @@ -289,7 +306,7 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { return; } - if (searchValue && !isSearchStringMatch(searchValue, personalDetailOption.searchText)) { + if (searchValue && !isSearchStringMatch(searchValue, personalDetailOption.searchText, personalDetailOption.participantsSet)) { return; } From 7a451c80377b42d6bf7f8ddfb94846b4ba8b8fb3 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Mon, 24 May 2021 15:58:45 +0800 Subject: [PATCH 16/31] style --- src/libs/OptionsListUtils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index ffe19c324e1c..16c4a3226e18 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -272,7 +272,8 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { } // Finally check to see if this options is a match for the provided search string if we have one - if (searchValue && !isSearchStringMatch(searchValue, reportOption.searchText, reportOption.participantsSet)) { + const {searchText, participantsSet} = reportOption; + if (searchValue && !isSearchStringMatch(searchValue, searchText, participantsSet)) { continue; } @@ -305,11 +306,10 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { ))) { return; } - - if (searchValue && !isSearchStringMatch(searchValue, personalDetailOption.searchText, personalDetailOption.participantsSet)) { + const {searchText, participantsSet} = personalDetailOption; + if (searchValue && !isSearchStringMatch(searchValue, searchText, participantsSet)) { return; } - personalDetailsOptions.push(personalDetailOption); }); } From 06bf09c4ae72bd127a02cbe496958069825c5bab Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 25 May 2021 11:10:26 +0800 Subject: [PATCH 17/31] fix line breaks --- src/libs/OptionsListUtils.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 16c4a3226e18..91b522cd64d1 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -56,9 +56,8 @@ function getPersonalDetailsForLogins(logins, personalDetails) { * @return {String} */ function getSearchText(report, personalDetailList) { - // Use a set because it skips adding duplicates - // which could potentially significantly speed up our - // regex search if someone has thousands of chats + // Use a set because it skips adding duplicates, + // which could potentially significantly speed up our regex search if someone has thousands of chats const searchTerms = new Set(); _.each(personalDetailList, (personalDetail) => { From e18549bfc834ceefaee66d431f55f7600fb0c437 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 25 May 2021 11:11:14 +0800 Subject: [PATCH 18/31] use _.each --- src/libs/OptionsListUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 91b522cd64d1..4380bb21410d 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -100,7 +100,7 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); const participantsSet = new Set(); - personalDetailList.forEach((participant) => { + _.each(personalDetailList, (participant) => { if (participant.login) { participantsSet.add(participant.login.toLowerCase()); } From ec3f9d6c5646a96ca0368f717fcbaf037fe40905 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Tue, 25 May 2021 11:13:56 +0800 Subject: [PATCH 19/31] Rename participantsSet => participantNames --- src/libs/OptionsListUtils.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 4380bb21410d..f742215f4839 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -99,19 +99,19 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie + _.unescape(report.lastMessageText) : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - const participantsSet = new Set(); + const participantNames = new Set(); _.each(personalDetailList, (participant) => { if (participant.login) { - participantsSet.add(participant.login.toLowerCase()); + participantNames.add(participant.login.toLowerCase()); } if (participant.firstName) { - participantsSet.add(participant.firstName.toLowerCase()); + participantNames.add(participant.firstName.toLowerCase()); } if (participant.lastName) { - participantsSet.add(participant.lastName.toLowerCase()); + participantNames.add(participant.lastName.toLowerCase()); } if (participant.displayName) { - participantsSet.add(participant.displayName.toLowerCase()); + participantNames.add(participant.displayName.toLowerCase()); } }); return { @@ -122,7 +122,7 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie icons: report ? report.icons : [personalDetail.avatar], tooltipText, participantsList: personalDetailList, - participantsSet, + participantNames, // It doesn't make sense to provide a login in the case of a report with multiple participants since // there isn't any one single login to refer to for a report. @@ -158,16 +158,16 @@ Onyx.connect({ * * @param {String} searchValue * @param {String} searchText - * @param {Set} participantsSet + * @param {Set} participantNames * @returns {Boolean} */ -function isSearchStringMatch(searchValue, searchText, participantsSet) { +function isSearchStringMatch(searchValue, searchText, participantNames) { const searchWords = searchValue.split(' '); return _.every(searchWords, (word) => { const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); const valueToSearch = searchText && searchText.replace(new RegExp(/ /g), ''); return matchRegex.test(valueToSearch) - || participantsSet.has(word.trim().replace(new RegExp(/,/g), '')); + || participantNames.has(word.trim().replace(new RegExp(/,/g), '')); }); } @@ -271,8 +271,8 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { } // Finally check to see if this options is a match for the provided search string if we have one - const {searchText, participantsSet} = reportOption; - if (searchValue && !isSearchStringMatch(searchValue, searchText, participantsSet)) { + const {searchText, participantNames} = reportOption; + if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames)) { continue; } @@ -305,8 +305,8 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { ))) { return; } - const {searchText, participantsSet} = personalDetailOption; - if (searchValue && !isSearchStringMatch(searchValue, searchText, participantsSet)) { + const {searchText, participantNames} = personalDetailOption; + if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames)) { return; } personalDetailsOptions.push(personalDetailOption); From 77e5b53f1312152cb0ee2a0d01e6a0fa6583dfc1 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Wed, 26 May 2021 12:24:32 +0800 Subject: [PATCH 20/31] Change participantNames to an Object --- src/libs/OptionsListUtils.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index f742215f4839..c8a2c3071db4 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -99,19 +99,19 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie + _.unescape(report.lastMessageText) : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - const participantNames = new Set(); + const participantNames = {}; _.each(personalDetailList, (participant) => { if (participant.login) { - participantNames.add(participant.login.toLowerCase()); + participantNames[participant.login.toLowerCase()] = true; } if (participant.firstName) { - participantNames.add(participant.firstName.toLowerCase()); + participantNames[participant.firstName.toLowerCase()] = true; } if (participant.lastName) { - participantNames.add(participant.lastName.toLowerCase()); + participantNames[participant.lastName.toLowerCase()] = true; } if (participant.displayName) { - participantNames.add(participant.displayName.toLowerCase()); + participantNames[participant.displayName.toLowerCase()] = true; } }); return { @@ -158,7 +158,7 @@ Onyx.connect({ * * @param {String} searchValue * @param {String} searchText - * @param {Set} participantNames + * @param {Object} participantNames * @returns {Boolean} */ function isSearchStringMatch(searchValue, searchText, participantNames) { @@ -167,7 +167,7 @@ function isSearchStringMatch(searchValue, searchText, participantNames) { const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); const valueToSearch = searchText && searchText.replace(new RegExp(/ /g), ''); return matchRegex.test(valueToSearch) - || participantNames.has(word.trim().replace(new RegExp(/,/g), '')); + || participantNames[word.trim().replace(new RegExp(/,/g), '')]; }); } From 7699460ab102194f6b0df88e0bc5914f4862e185 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Wed, 26 May 2021 12:27:38 +0800 Subject: [PATCH 21/31] comment --- src/libs/OptionsListUtils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index c8a2c3071db4..d398a8484b99 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -99,6 +99,8 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie + _.unescape(report.lastMessageText) : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); + + // Using an object instead of an array since we can check if it has a key in constant time const participantNames = {}; _.each(personalDetailList, (participant) => { if (participant.login) { From 44647ff6b03d183da5d24e3b252ed0659101d032 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Thu, 27 May 2021 12:00:43 +0800 Subject: [PATCH 22/31] Update comment --- src/libs/OptionsListUtils.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index efbf6890ac16..3a3d178c1940 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -67,11 +67,11 @@ function getSearchText(report, personalDetailList) { if (report) { searchTerms.add(report.reportName); - searchTerms.add(...report.reportName.split(',').map(name => name.trim())); - searchTerms.add(...report.participants); + _.each(report.reportName.split(',').map(name => name.trim()), name => searchTerms.add(name)); + _.each(report.participants, participant => searchTerms.add(participant)); } - return _.unique(Array.from(searchTerms)).join(' '); + return Array.from(searchTerms).join(' '); } /** @@ -100,7 +100,10 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - // Using an object instead of an array since we can check if it has a key in constant time + // Using an object instead of an array, + // since worst-case an array would require iterating through all of its members, + // while an object would allow us to directly look up a key. + // Looking up a value in an object with n keys can be up to n times faster than in an array with n members. const participantNames = {}; _.each(personalDetailList, (participant) => { if (participant.login) { From 8024c7af922fd4829ff5e8e6a31b5e9aaae931ba Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Thu, 27 May 2021 12:04:12 +0800 Subject: [PATCH 23/31] simplify logic --- src/libs/OptionsListUtils.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 3a3d178c1940..e0a3903446e3 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -168,12 +168,14 @@ Onyx.connect({ * @returns {Boolean} */ function isSearchStringMatch(searchValue, searchText, participantNames) { - const searchWords = searchValue.split(' '); + const searchWords = searchValue + .replace(/,/g, ' ') + .split(' ') + .map(word => word.trim()); return _.every(searchWords, (word) => { const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); const valueToSearch = searchText && searchText.replace(new RegExp(/ /g), ''); - return matchRegex.test(valueToSearch) - || participantNames[word.trim().replace(new RegExp(/,/g), '')]; + return matchRegex.test(valueToSearch) || participantNames[word]; }); } From a6063b276f84ba88699bfe1b5f30b3f0b302c6ee Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Thu, 27 May 2021 12:06:13 +0800 Subject: [PATCH 24/31] remove set usage --- src/libs/OptionsListUtils.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index e0a3903446e3..bd484cd69c55 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -58,7 +58,7 @@ function getPersonalDetailsForLogins(logins, personalDetails) { function getSearchText(report, personalDetailList) { // Use a set because it skips adding duplicates, // which could potentially significantly speed up our regex search if someone has thousands of chats - const searchTerms = new Set(); + const searchTerms = []; _.each(personalDetailList, (personalDetail) => { searchTerms.add(personalDetail.displayName); @@ -66,12 +66,12 @@ function getSearchText(report, personalDetailList) { }); if (report) { - searchTerms.add(report.reportName); - _.each(report.reportName.split(',').map(name => name.trim()), name => searchTerms.add(name)); - _.each(report.participants, participant => searchTerms.add(participant)); + searchTerms.push(...report.reportName); + searchTerms.push(...report.reportName.split(',').map(name => name.trim())); + searchTerms.push(...report.participants); } - return Array.from(searchTerms).join(' '); + return _.unique(searchTerms).join(' '); } /** From b8a34a57709a6706d0a062bbea5d71394e191003 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Thu, 27 May 2021 12:39:17 +0800 Subject: [PATCH 25/31] Fix jest unit tests --- src/libs/OptionsListUtils.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index bd484cd69c55..d649855c713e 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -61,10 +61,9 @@ function getSearchText(report, personalDetailList) { const searchTerms = []; _.each(personalDetailList, (personalDetail) => { - searchTerms.add(personalDetail.displayName); - searchTerms.add(personalDetail.login); + searchTerms.push(personalDetail.displayName); + searchTerms.push(personalDetail.login); }); - if (report) { searchTerms.push(...report.reportName); searchTerms.push(...report.reportName.split(',').map(name => name.trim())); From cb31f89ffd0889d0d6ab90c9b5f8071e91827804 Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Thu, 27 May 2021 13:16:25 +0800 Subject: [PATCH 26/31] remove unused comment --- src/libs/OptionsListUtils.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index d649855c713e..edc836681d41 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -56,8 +56,6 @@ function getPersonalDetailsForLogins(logins, personalDetails) { * @return {String} */ function getSearchText(report, personalDetailList) { - // Use a set because it skips adding duplicates, - // which could potentially significantly speed up our regex search if someone has thousands of chats const searchTerms = []; _.each(personalDetailList, (personalDetail) => { From bc695a574096feed22444f38a9f947aeaa28de3f Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 28 May 2021 01:51:25 +0530 Subject: [PATCH 27/31] fix: IOS spinner --- src/components/ScreenWrapper.js | 3 ++- src/libs/onScreenTransitionEnd/index.ios.js | 19 +++++++++++++++++++ src/libs/onScreenTransitionEnd/index.js | 14 ++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 src/libs/onScreenTransitionEnd/index.ios.js create mode 100644 src/libs/onScreenTransitionEnd/index.js diff --git a/src/components/ScreenWrapper.js b/src/components/ScreenWrapper.js index 7ab7d5f8ff43..c9379be908eb 100644 --- a/src/components/ScreenWrapper.js +++ b/src/components/ScreenWrapper.js @@ -7,6 +7,7 @@ import {SafeAreaInsetsContext} from 'react-native-safe-area-context'; import styles, {getSafeAreaPadding} from '../styles/styles'; import HeaderGap from './HeaderGap'; import KeyboardShortcut from '../libs/KeyboardShortcut'; +import onScreenTransitionEnd from '../libs/onScreenTransitionEnd'; const propTypes = { /** Array of additional styles to add */ @@ -61,7 +62,7 @@ class ScreenWrapper extends React.Component { this.props.navigation.goBack(); }, [], true); - this.unsubscribeTransitionEnd = this.props.navigation.addListener('transitionEnd', () => { + this.unsubscribeTransitionEnd = onScreenTransitionEnd(this.props.navigation, () => { this.setState({didScreenTransitionEnd: true}); this.props.onTransitionEnd(); }); diff --git a/src/libs/onScreenTransitionEnd/index.ios.js b/src/libs/onScreenTransitionEnd/index.ios.js new file mode 100644 index 000000000000..c8acdf7d17b2 --- /dev/null +++ b/src/libs/onScreenTransitionEnd/index.ios.js @@ -0,0 +1,19 @@ +/** + * Navigation's transitionEnd is not reliable on IOS thus we use InteractonManager + * Some information https://github.com/software-mansion/react-native-screens/issues/713 +*/ +import {InteractionManager} from 'react-native'; + +/** + * Call the callback after screen transiton has ended + * + * @param {Object} navigation Screen navigation prop + * @param {Function} callback Method to call + * @returns {Function} + */ +function onScreenTransitionEnd(navigation, callback) { + const handle = InteractionManager.runAfterInteractions(callback); + return () => handle.cancel(); +} + +export default onScreenTransitionEnd; diff --git a/src/libs/onScreenTransitionEnd/index.js b/src/libs/onScreenTransitionEnd/index.js new file mode 100644 index 000000000000..f0d066ee9bde --- /dev/null +++ b/src/libs/onScreenTransitionEnd/index.js @@ -0,0 +1,14 @@ +import Str from 'expensify-common/lib/str'; + +/** + * Call the callback after screen transiton has ended + * + * @param {Object} navigation Screen navigation prop + * @param {Function} callback Method to call + * @returns {Function} + */ +function onScreenTransitionEnd(navigation, callback) { + return navigation.addListener('transitionEnd', evt => Str.result(callback, evt)); +} + +export default onScreenTransitionEnd; From d1177d895b822e4b75a3c000ad7f9c96c47ce9bf Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Fri, 28 May 2021 14:10:36 +0800 Subject: [PATCH 28/31] Move logic to a separate function, use Set for clarity --- src/libs/OptionsListUtils.js | 62 +++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index edc836681d41..7ac0ddabab90 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -48,6 +48,34 @@ function getPersonalDetailsForLogins(logins, personalDetails) { }); } +/** + * Constructs a Set with all possible names (displayName, firstName, lastName, email) for all participants in a report, + * to be used in isSearchStringMatch. + * + * @param {Array} personalDetailList + * @return {Set} + */ +function getParticipantNames(personalDetailList) { + // We use a Set because `Set.has(value)` on a Set of with n entries is up to n (or log(n)) times faster than + // `_.contains(Array, value)` for an Array with n members. + const participantNames = new Set(); + _.each(personalDetailList, (participant) => { + if (participant.login) { + participantNames.add(participant.login.toLowerCase()); + } + if (participant.firstName) { + participantNames.add(participant.firstName.toLowerCase()); + } + if (participant.lastName) { + participantNames.add(participant.lastName.toLowerCase()); + } + if (participant.displayName) { + participantNames.add(participant.displayName.toLowerCase()); + } + }); + return participantNames; +} + /** * Returns a string with all relevant search terms * @@ -96,26 +124,6 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie + _.unescape(report.lastMessageText) : ''; const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); - - // Using an object instead of an array, - // since worst-case an array would require iterating through all of its members, - // while an object would allow us to directly look up a key. - // Looking up a value in an object with n keys can be up to n times faster than in an array with n members. - const participantNames = {}; - _.each(personalDetailList, (participant) => { - if (participant.login) { - participantNames[participant.login.toLowerCase()] = true; - } - if (participant.firstName) { - participantNames[participant.firstName.toLowerCase()] = true; - } - if (participant.lastName) { - participantNames[participant.lastName.toLowerCase()] = true; - } - if (participant.displayName) { - participantNames[participant.displayName.toLowerCase()] = true; - } - }); const fullTitle = personalDetailList.map(({firstName, login}) => firstName || login).join(', '); return { text: hasMultipleParticipants ? fullTitle : report?.reportName || personalDetail.displayName, @@ -125,7 +133,6 @@ function createOption(personalDetailList, report, draftComments, {showChatPrevie icons: report ? report.icons : [personalDetail.avatar], tooltipText, participantsList: personalDetailList, - participantNames, // It doesn't make sense to provide a login in the case of a report with multiple participants since // there isn't any one single login to refer to for a report. @@ -161,7 +168,7 @@ Onyx.connect({ * * @param {String} searchValue * @param {String} searchText - * @param {Object} participantNames + * @param {Set} participantNames * @returns {Boolean} */ function isSearchStringMatch(searchValue, searchText, participantNames) { @@ -172,7 +179,7 @@ function isSearchStringMatch(searchValue, searchText, participantNames) { return _.every(searchWords, (word) => { const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); const valueToSearch = searchText && searchText.replace(new RegExp(/ /g), ''); - return matchRegex.test(valueToSearch) || participantNames[word]; + return matchRegex.test(valueToSearch) || participantNames.has(word); }); } @@ -275,8 +282,9 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { continue; } - // Finally check to see if this options is a match for the provided search string if we have one - const {searchText, participantNames} = reportOption; + // Finally check to see if this option is a match for the provided search string if we have one + const {searchText, participantsList} = reportOption; + const participantNames = getParticipantNames(participantsList); if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames)) { continue; } @@ -310,7 +318,8 @@ function getOptions(reports, personalDetails, draftComments, activeReportID, { ))) { return; } - const {searchText, participantNames} = personalDetailOption; + const {searchText, participantsList} = personalDetailOption; + const participantNames = getParticipantNames(participantsList); if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames)) { return; } @@ -537,4 +546,5 @@ export { getCurrencyListForSections, getIOUConfirmationOptionsFromMyPersonalDetail, getIOUConfirmationOptionsFromParticipants, + getParticipantNames, }; From 71c58ee8b17fce1503a8c9cabfcdc630b6fc5d2f Mon Sep 17 00:00:00 2001 From: Jasper Huang Date: Fri, 28 May 2021 14:13:53 +0800 Subject: [PATCH 29/31] Remove export --- src/libs/OptionsListUtils.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 7ac0ddabab90..cf97bf619cff 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -546,5 +546,4 @@ export { getCurrencyListForSections, getIOUConfirmationOptionsFromMyPersonalDetail, getIOUConfirmationOptionsFromParticipants, - getParticipantNames, }; From 18190bb95fd659643aad42fea0031af9f09d2c04 Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Fri, 28 May 2021 09:18:38 -0700 Subject: [PATCH 30/31] Temporarily replace green with cyan --- src/styles/colors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/styles/colors.js b/src/styles/colors.js index 89c10430fca0..f53f88052688 100644 --- a/src/styles/colors.js +++ b/src/styles/colors.js @@ -7,7 +7,7 @@ export default { dark: '#0b1b34', black: '#000000', blue: '#0185ff', - green: '#03d47c', + green: '#42f5f2', greenHover: '#03c775', red: '#fc3826', yellow: '#fed607', From 5177fe902be9f38bd8ad682759b2f07c96d04eee Mon Sep 17 00:00:00 2001 From: Rory Abraham Date: Fri, 28 May 2021 09:48:56 -0700 Subject: [PATCH 31/31] fix bad merge --- src/styles/colors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/styles/colors.js b/src/styles/colors.js index 19fa0dee8c08..80a6bb3284e7 100644 --- a/src/styles/colors.js +++ b/src/styles/colors.js @@ -6,7 +6,7 @@ export default { gray4: '#7D8B8F', dark: '#0b1b34', black: '#000000', - blue: '#03d47c', + blue: '#c542f5', green: '#42f5f2', greenHover: '#03c775', red: '#fc3826',