Skip to content

Commit

Permalink
Merge pull request Expensify#45874 from software-mansion-labs/brtqkr/…
Browse files Browse the repository at this point in the history
…44443-fix-assign-delay

Fix assign person delay
  • Loading branch information
mountiny authored Aug 7, 2024
2 parents ba39514 + 24c8a84 commit f9306d5
Show file tree
Hide file tree
Showing 51 changed files with 66 additions and 92 deletions.
2 changes: 1 addition & 1 deletion src/components/CurrencySelectionList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function CurrencySelectionList({searchInputLabel, initiallySelectedCurrencyCode,
textInputValue={searchValue}
onChangeText={setSearchValue}
onSelectRow={onSelect}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
headerMessage={headerMessage}
initiallyFocusedOptionKey={initiallySelectedCurrencyCode}
showScrollIndicator
Expand Down
2 changes: 1 addition & 1 deletion src/components/MoneyRequestConfirmationList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ function MoneyRequestConfirmationList({
sections={sections}
ListItem={UserListItem}
onSelectRow={navigateToReportOrUserDetail}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
canSelectMultiple={false}
shouldPreventDefaultFocusOnSelectRow
footerContent={footerContent}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Search/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ function Search({queryJSON, policyIDs, isCustomQuery}: SearchProps) {
ListItem={ListItem}
onSelectRow={openReport}
getItemHeight={getItemHeightMemoized}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
listHeaderWrapperStyle={[styles.ph8, styles.pv3, styles.pb5]}
containerStyle={[styles.pv0]}
Expand Down
51 changes: 13 additions & 38 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {useFocusEffect, useIsFocused} from '@react-navigation/native';
import lodashDebounce from 'lodash/debounce';
import isEmpty from 'lodash/isEmpty';
import type {ForwardedRef} from 'react';
import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
Expand All @@ -21,6 +20,7 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useKeyboardState from '@hooks/useKeyboardState';
import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import useSingleExecution from '@hooks/useSingleExecution';
import useThemeStyles from '@hooks/useThemeStyles';
import getSectionsWithIndexOffset from '@libs/getSectionsWithIndexOffset';
import Log from '@libs/Log';
Expand All @@ -40,7 +40,7 @@ function BaseSelectionList<TItem extends ListItem>(
shouldUseUserSkeletonView,
canSelectMultiple = false,
onSelectRow,
shouldDebounceRowSelect = false,
shouldSingleExecuteRowSelect = false,
onCheckboxPress,
onSelectAll,
onDismissError,
Expand Down Expand Up @@ -117,6 +117,7 @@ function BaseSelectionList<TItem extends ListItem>(
const [currentPage, setCurrentPage] = useState(1);
const isTextInputFocusedRef = useRef<boolean>(false);
const isEmptyList = sections.length === 0;
const {singleExecution} = useSingleExecution();

const incrementPage = () => setCurrentPage((prev) => prev + 1);

Expand Down Expand Up @@ -282,9 +283,6 @@ function BaseSelectionList<TItem extends ListItem>(
onChangeText?.('');
}, [onChangeText]);

// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
const debouncedOnSelectRow = useCallback(lodashDebounce(onSelectRow, 200), [onSelectRow]);

/**
* Logic to run when a row is selected, either with click/press or keyboard hotkeys.
*
Expand Down Expand Up @@ -315,11 +313,7 @@ function BaseSelectionList<TItem extends ListItem>(
setFocusedIndex(indexToFocus);
}

if (shouldDebounceRowSelect) {
debouncedOnSelectRow(item);
} else {
onSelectRow(item);
}
onSelectRow(item);

if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
innerTextInputRef.current.focus();
Expand All @@ -344,11 +338,6 @@ function BaseSelectionList<TItem extends ListItem>(
selectRow(focusedOption);
};

// This debounce happens on the trailing edge because on repeated enter presses, rapid component state update cancels the existing debounce and the redundant
// enter presses runs the debounced function again.
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
const debouncedSelectFocusedOption = useCallback(lodashDebounce(selectFocusedOption, 100), [selectFocusedOption]);

/**
* This function is used to compute the layout of any given item in our list.
* We need to implement it so that we can programmatically scroll to items outside the virtual render window of the SectionList.
Expand Down Expand Up @@ -457,7 +446,13 @@ function BaseSelectionList<TItem extends ListItem>(
showTooltip={showTooltip}
canSelectMultiple={canSelectMultiple}
onLongPressRow={onLongPressRow}
onSelectRow={() => selectRow(item, index)}
onSelectRow={() => {
if (shouldSingleExecuteRowSelect) {
singleExecution(() => selectRow(item, index))();
} else {
selectRow(item);
}
}}
onCheckboxPress={handleOnCheckboxPress()}
onDismissError={() => onDismissError?.(item)}
shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow}
Expand Down Expand Up @@ -513,26 +508,6 @@ function BaseSelectionList<TItem extends ListItem>(
[scrollToIndex, setFocusedIndex],
);

useEffect(() => {
if (!(shouldDebounceRowSelect && debouncedOnSelectRow.cancel)) {
return;
}

return () => {
debouncedOnSelectRow.cancel();
};
}, [debouncedOnSelectRow, shouldDebounceRowSelect]);

useEffect(() => {
if (!(shouldDebounceRowSelect && debouncedSelectFocusedOption.cancel)) {
return;
}

return () => {
debouncedSelectFocusedOption.cancel();
};
}, [debouncedSelectFocusedOption, shouldDebounceRowSelect]);

/** Function to focus text input */
const focusTextInput = useCallback(() => {
if (!innerTextInputRef.current) {
Expand Down Expand Up @@ -627,7 +602,7 @@ function BaseSelectionList<TItem extends ListItem>(
useImperativeHandle(ref, () => ({scrollAndHighlightItem, clearInputAfterSelect}), [scrollAndHighlightItem, clearInputAfterSelect]);

/** Selects row when pressing Enter */
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, shouldDebounceRowSelect ? debouncedSelectFocusedOption : selectFocusedOption, {
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, {
captureOnInputs: true,
shouldBubble: !flattenedSections.allOptions[focusedIndex],
shouldStopPropagation,
Expand Down Expand Up @@ -687,7 +662,7 @@ function BaseSelectionList<TItem extends ListItem>(
selectTextOnFocus
spellCheck={false}
iconLeft={textInputIconLeft}
onSubmitEditing={shouldDebounceRowSelect ? debouncedSelectFocusedOption : selectFocusedOption}
onSubmitEditing={selectFocusedOption}
blurOnSubmit={!!flattenedSections.allOptions.length}
isLoading={isLoadingNewOptions}
testID="selection-list-text-input"
Expand Down
4 changes: 2 additions & 2 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ type BaseSelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {
/** Callback to fire when a row is pressed */
onSelectRow: (item: TItem) => void;

/** Whether to debounce `onRowSelect` */
shouldDebounceRowSelect?: boolean;
/** Whether to single execution `onRowSelect` - workaround for unintentional multiple navigation calls https://github.com/Expensify/App/issues/44443 */
shouldSingleExecuteRowSelect?: boolean;

/** Whether to update the focused index on a row select */
shouldUpdateFocusedIndex?: boolean;
Expand Down
8 changes: 4 additions & 4 deletions src/components/SelectionScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ type SelectionScreenProps<T = string> = {
/** A function to run when the X button next to the error is clicked */
onClose?: () => void;

/** Whether to debounce `onRowSelect` */
shouldDebounceRowSelect?: boolean;
/** Whether to single execute `onRowSelect` - this prevents bugs related to double interactions */
shouldSingleExecuteRowSelect?: boolean;

/** Used for dynamic header title translation with parameters */
headerTitleAlreadyTranslated?: string;
Expand All @@ -112,7 +112,7 @@ function SelectionScreen<T = string>({
errors,
errorRowStyles,
onClose,
shouldDebounceRowSelect,
shouldSingleExecuteRowSelect,
headerTitleAlreadyTranslated,
}: SelectionScreenProps<T>) {
const {translate} = useLocalize();
Expand Down Expand Up @@ -152,7 +152,7 @@ function SelectionScreen<T = string>({
listEmptyContent={listEmptyContent}
listFooterContent={listFooterContent}
sectionListStyle={[styles.flexGrow0]}
shouldDebounceRowSelect={shouldDebounceRowSelect}
shouldSingleExecuteRowSelect={shouldSingleExecuteRowSelect}
>
<ErrorMessageRow
errors={errors}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ChatFinderPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ function ChatFinderPage({betas, isSearchingForReports, navigation}: ChatFinderPa
headerMessage={headerMessage}
onLayout={setPerformanceTimersEnd}
onSelectRow={selectReport}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
showLoadingPlaceholder={!areOptionsInitialized || !isScreenTransitionEnd}
footerContent={!isDismissed && ChatFinderPageFooterInstance}
isLoadingNewOptions={!!isSearchingForReports}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/NewChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ function NewChatPage({isGroupChat}: NewChatPageProps) {
textInputLabel={translate('selectionList.nameEmailOrPhoneNumber')}
headerMessage={headerMessage}
onSelectRow={createChat}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
onConfirm={(e, option) => (selectedOptions.length > 0 ? createGroup() : createChat(option))}
rightHandSideComponent={itemRightSideComponent}
footerContent={footerContent}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function BusinessTypeSelectorModal({isVisible, currentBusinessType, onBusinessTy
sections={[{data: incorporationTypes}]}
initiallyFocusedOptionKey={currentBusinessType}
onSelectRow={onBusinessTypeSelected}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
shouldStopPropagation
shouldUseDynamicMaxToRenderPerBatch
ListItem={RadioListItem}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ReportParticipantRoleSelectionPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function ReportParticipantRoleSelectionPage({report, route}: ReportParticipantRo
sections={[{data: items}]}
ListItem={RadioListItem}
onSelectRow={changeRole}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={items.find((item) => item.isSelected)?.keyForList}
/>
</View>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ReportParticipantsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ function ReportParticipantsPage({report}: WithReportOrNotFoundProps) {
ListItem={TableListItem}
headerContent={headerContent}
onSelectRow={openMemberDetails}
shouldDebounceRowSelect={!(isGroupChat && isCurrentUserAdmin)}
shouldSingleExecuteRowSelect={!(isGroupChat && isCurrentUserAdmin)}
onCheckboxPress={(item) => toggleUser(item)}
onSelectAll={() => toggleAllUsers(participants)}
showScrollIndicator
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
onChangeText={setSearchTerm}
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
onSelectRow={onSelectRow}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
footerContent={footerContent}
headerMessage={header}
showLoadingPlaceholder={!areOptionsInitialized || !didScreenTransitionEnd}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/step/IOURequestStepDistanceRate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function IOURequestStepDistanceRate({
sections={[{data: sections}]}
ListItem={RadioListItem}
onSelectRow={({value}) => selectDistanceRate(value ?? '')}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={initiallyFocusedOption}
/>
</StepScreenWrapper>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/step/IOURequestStepSendFrom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function IOURequestStepSendFrom({route, transaction, allPolicies}: IOURequestSte
<SelectionList
sections={[{data: workspaceOptions, title: translate('common.workspaces')}]}
onSelectRow={selectWorkspace}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
ListItem={UserListItem}
initiallyFocusedOptionKey={selectedWorkspace?.policyID}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/step/IOURequestStepSplitPayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function IOURequestStepSplitPayer({
sections={sections}
ListItem={UserListItem}
onSelectRow={setSplitPayer}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
showLoadingPlaceholder={!didScreenTransitionEnd}
/>
</StepScreenWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ function BaseShareLogList({onAttachLogToReport}: BaseShareLogListProps) {
ListItem={UserListItem}
sections={didScreenTransitionEnd ? sections : CONST.EMPTY_ARRAY}
onSelectRow={attachLogToReport}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
onChangeText={setSearchValue}
textInputValue={searchValue}
headerMessage={searchOptions.headerMessage}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Preferences/LanguagePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function LanguagePage() {
sections={[{data: localesToLanguages}]}
ListItem={RadioListItem}
onSelectRow={(language) => App.setLocaleAndNavigate(language.value)}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={localesToLanguages.find((locale) => locale.isSelected)?.keyForList}
/>
</ScreenWrapper>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Preferences/PriorityModePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function PriorityModePage() {
sections={[{data: priorityModes}]}
ListItem={RadioListItem}
onSelectRow={updateMode}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={priorityModes.find((mode) => mode.isSelected)?.keyForList}
/>
</ScreenWrapper>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Preferences/ThemePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function ThemePage({preferredTheme}: ThemePageProps) {
sections={[{data: localesToThemes}]}
ListItem={RadioListItem}
onSelectRow={(theme) => User.updateTheme(theme.value)}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={localesToThemes.find((theme) => theme.isSelected)?.keyForList}
/>
</ScreenWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ function CountrySelectionPage({route, navigation}: CountrySelectionPageProps) {
sections={[{data: searchResults}]}
ListItem={RadioListItem}
onSelectRow={selectCountry}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
onChangeText={setSearchValue}
initiallyFocusedOptionKey={currentCountry}
shouldUseDynamicMaxToRenderPerBatch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ function StateSelectionPage() {

<SelectionList
onSelectRow={selectCountryState}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
headerMessage={headerMessage}
// Label can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Profile/PronounsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function PronounsPage({currentUserPersonalDetails, isLoadingApp = true}: Pronoun
sections={[{data: filteredPronounsList}]}
ListItem={RadioListItem}
onSelectRow={updatePronouns}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
onChangeText={setSearchValue}
initiallyFocusedOptionKey={currentPronounsKey}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Profile/TimezoneSelectPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function TimezoneSelectPage({currentUserPersonalDetails}: TimezoneSelectPageProp
textInputValue={timezoneInputText}
onChangeText={filterShownTimezones}
onSelectRow={saveSelectedTimezone}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
sections={[{data: timezoneOptions, isDisabled: timezone.automatic}]}
initiallyFocusedOptionKey={timezoneOptions.find((tz) => tz.text === timezone.selected)?.keyForList}
showScrollIndicator
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Report/NotificationPreferencePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function NotificationPreferencePage({report}: NotificationPreferencePageProps) {
onSelectRow={(option) =>
report && ReportActions.updateNotificationPreference(report.reportID, report.notificationPreference, option.value, true, undefined, undefined, report)
}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={notificationPreferenceOptions.find((locale) => locale.isSelected)?.keyForList}
/>
</FullPageNotFoundView>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Report/VisibilityPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function VisibilityPage({report}: VisibilityProps) {
}
changeVisibility(option.value);
}}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={visibilityOptions.find((visibility) => visibility.isSelected)?.keyForList}
ListItem={RadioListItem}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Report/WriteCapabilityPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function WriteCapabilityPage({report, policy}: WriteCapabilityPageProps) {
sections={[{data: writeCapabilityOptions}]}
ListItem={RadioListItem}
onSelectRow={(option) => report && ReportActions.updateWriteCapabilityAndNavigate(report, option.value)}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
initiallyFocusedOptionKey={writeCapabilityOptions.find((locale) => locale.isSelected)?.keyForList}
/>
</FullPageNotFoundView>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/tasks/TaskAssigneeSelectorModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ function TaskAssigneeSelectorModal({reports, task}: TaskAssigneeSelectorModalPro
sections={areOptionsInitialized ? sections : []}
ListItem={UserListItem}
onSelectRow={selectReport}
shouldDebounceRowSelect
shouldSingleExecuteRowSelect
onChangeText={setSearchValue}
textInputValue={searchValue}
headerMessage={headerMessage}
Expand Down
Loading

0 comments on commit f9306d5

Please sign in to comment.