Skip to content

Commit

Permalink
Merge pull request Expensify#26415 from thiagobrez/thiagobrez/selecti…
Browse files Browse the repository at this point in the history
…on-list-fix-autofocus

fix(selection-list): focus input on screen focus
  • Loading branch information
roryabraham authored Sep 12, 2023
2 parents 662af46 + 13ed92c commit a365583
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 26 deletions.
1 change: 0 additions & 1 deletion src/components/CountryPicker/CountrySelectorModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ function CountrySelectorModal({currentCountry, isVisible, onClose, onCountrySele
sections={[{data: searchResults, indexOffset: 0}]}
onSelectRow={onCountrySelected}
onChangeText={setSearchValue}
shouldDelayFocus
initiallyFocusedOptionKey={currentCountry}
/>
</ScreenWrapper>
Expand Down
31 changes: 14 additions & 17 deletions src/components/SelectionList/BaseSelectionList.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React, {useEffect, useMemo, useRef, useState} from 'react';
import React, {useCallback, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
import _ from 'underscore';
import lodashGet from 'lodash/get';
import {useFocusEffect} from '@react-navigation/native';
import SectionList from '../SectionList';
import Text from '../Text';
import styles from '../../styles/styles';
Expand Down Expand Up @@ -42,7 +43,6 @@ function BaseSelectionList({
keyboardType = CONST.KEYBOARD_TYPE.DEFAULT,
onChangeText,
initiallyFocusedOptionKey = '',
shouldDelayFocus = false,
onScroll,
onScrollBeginDrag,
headerMessage = '',
Expand Down Expand Up @@ -266,23 +266,20 @@ function BaseSelectionList({
);
};

/** Focuses the text input when the component mounts. If `props.shouldDelayFocus` is true, we wait for the animation to finish */
useEffect(() => {
if (shouldShowTextInput) {
if (shouldDelayFocus) {
/** Focuses the text input when the component comes into focus and after any navigation animations finish. */
useFocusEffect(
useCallback(() => {
if (shouldShowTextInput) {
focusTimeoutRef.current = setTimeout(() => textInputRef.current.focus(), CONST.ANIMATED_TRANSITION);
} else {
textInputRef.current.focus();
}
}

return () => {
if (!focusTimeoutRef.current) {
return;
}
clearTimeout(focusTimeoutRef.current);
};
}, [shouldDelayFocus, shouldShowTextInput]);
return () => {
if (!focusTimeoutRef.current) {
return;
}
clearTimeout(focusTimeoutRef.current);
};
}, [shouldShowTextInput]),
);

/** Selects row when pressing Enter */
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, {
Expand Down
3 changes: 0 additions & 3 deletions src/components/SelectionList/selectionListPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ const propTypes = {
/** Item `keyForList` to focus initially */
initiallyFocusedOptionKey: PropTypes.string,

/** Whether to delay focus on the text input when mounting. Used for a smoother animation on Android */
shouldDelayFocus: PropTypes.bool,

/** Callback to fire when the list is scrolled */
onScroll: PropTypes.func,

Expand Down
1 change: 0 additions & 1 deletion src/components/StatePicker/StateSelectorModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ function StateSelectorModal({currentState, isVisible, onClose, onStateSelected,
sections={[{data: searchResults, indexOffset: 0}]}
onSelectRow={onStateSelected}
onChangeText={setSearchValue}
shouldDelayFocus
initiallyFocusedOptionKey={currentState}
/>
</ScreenWrapper>
Expand Down
1 change: 0 additions & 1 deletion src/pages/settings/Profile/PronounsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ function PronounsPage({currentUserPersonalDetails}) {
onSelectRow={updatePronouns}
onChangeText={setSearchValue}
initiallyFocusedOptionKey={currentPronounsKey}
shouldDelayFocus
/>
</ScreenWrapper>
);
Expand Down
1 change: 0 additions & 1 deletion src/pages/settings/Profile/TimezoneSelectPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ function TimezoneSelectPage(props) {
onSelectRow={saveSelectedTimezone}
sections={[{data: timezoneOptions, indexOffset: 0, isDisabled: timezone.automatic}]}
initiallyFocusedOptionKey={_.get(_.filter(timezoneOptions, (tz) => tz.text === timezone.selected)[0], 'keyForList')}
shouldDelayFocus
showScrollIndicator
/>
</ScreenWrapper>
Expand Down
1 change: 0 additions & 1 deletion src/pages/workspace/WorkspaceInvitePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ function WorkspaceInvitePage(props) {
onSelectRow={toggleOption}
onConfirm={inviteUser}
showScrollIndicator
shouldDelayFocus
showLoadingPlaceholder={!didScreenTransitionEnd || !OptionsListUtils.isPersonalDetailsReady(props.personalDetails)}
/>
<View style={[styles.flexShrink0]}>
Expand Down
1 change: 0 additions & 1 deletion src/pages/workspace/WorkspaceMembersPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,6 @@ function WorkspaceMembersPage(props) {
onSelectAll={() => toggleAllUsers(data)}
onDismissError={dismissError}
showLoadingPlaceholder={!OptionsListUtils.isPersonalDetailsReady(props.personalDetails) || _.isEmpty(props.policyMembers)}
shouldDelayFocus
showScrollIndicator
/>
</View>
Expand Down
5 changes: 5 additions & 0 deletions tests/perf-test/SelectionList.perf-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ jest.mock('../../src/components/withKeyboardState', () => (Component) => (props)
/>
));

jest.mock('@react-navigation/native', () => ({
useFocusEffect: () => {},
createNavigationContainerRef: jest.fn(),
}));

function SelectionListWrapper(args) {
const [selectedIds, setSelectedIds] = useState([]);

Expand Down

0 comments on commit a365583

Please sign in to comment.