From a3c16e54f26a0db4e8e89a9401786e9d6075ca04 Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Thu, 31 Aug 2023 17:34:22 +0100 Subject: [PATCH 1/3] fix(selection-list): focus input on screen focus --- .../CountryPicker/CountrySelectorModal.js | 1 - .../SelectionList/BaseSelectionList.js | 31 +++++++++---------- .../SelectionList/selectionListPropTypes.js | 3 -- .../StatePicker/StateSelectorModal.js | 1 - src/pages/settings/Profile/PronounsPage.js | 1 - .../settings/Profile/TimezoneSelectPage.js | 1 - src/pages/workspace/WorkspaceInvitePage.js | 1 - src/pages/workspace/WorkspaceMembersPage.js | 1 - 8 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/components/CountryPicker/CountrySelectorModal.js b/src/components/CountryPicker/CountrySelectorModal.js index 146b023bbf0c..d8fdfa157a18 100644 --- a/src/components/CountryPicker/CountrySelectorModal.js +++ b/src/components/CountryPicker/CountrySelectorModal.js @@ -81,7 +81,6 @@ function CountrySelectorModal({currentCountry, isVisible, onClose, onCountrySele sections={[{data: searchResults, indexOffset: 0}]} onSelectRow={onCountrySelected} onChangeText={setSearchValue} - shouldDelayFocus initiallyFocusedOptionKey={currentCountry} /> diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 046b64e9e5c0..63daae22ef5d 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -1,4 +1,4 @@ -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'; @@ -23,6 +23,7 @@ import useLocalize from '../../hooks/useLocalize'; import Log from '../../libs/Log'; import OptionsListSkeletonView from '../OptionsListSkeletonView'; import useActiveElement from '../../hooks/useActiveElement'; +import {useFocusEffect} from '@react-navigation/native'; const propTypes = { ...keyboardStatePropTypes, @@ -42,7 +43,6 @@ function BaseSelectionList({ keyboardType = CONST.KEYBOARD_TYPE.DEFAULT, onChangeText, initiallyFocusedOptionKey = '', - shouldDelayFocus = false, onScroll, onScrollBeginDrag, headerMessage = '', @@ -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, { diff --git a/src/components/SelectionList/selectionListPropTypes.js b/src/components/SelectionList/selectionListPropTypes.js index 9adf42833ebc..1a4fd6c5ecab 100644 --- a/src/components/SelectionList/selectionListPropTypes.js +++ b/src/components/SelectionList/selectionListPropTypes.js @@ -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, diff --git a/src/components/StatePicker/StateSelectorModal.js b/src/components/StatePicker/StateSelectorModal.js index 91ee1b225a1f..abd12c7f5e4e 100644 --- a/src/components/StatePicker/StateSelectorModal.js +++ b/src/components/StatePicker/StateSelectorModal.js @@ -86,7 +86,6 @@ function StateSelectorModal({currentState, isVisible, onClose, onStateSelected, sections={[{data: searchResults, indexOffset: 0}]} onSelectRow={onStateSelected} onChangeText={setSearchValue} - shouldDelayFocus initiallyFocusedOptionKey={currentState} /> diff --git a/src/pages/settings/Profile/PronounsPage.js b/src/pages/settings/Profile/PronounsPage.js index ce460bc30ff4..1706af1b9cac 100644 --- a/src/pages/settings/Profile/PronounsPage.js +++ b/src/pages/settings/Profile/PronounsPage.js @@ -109,7 +109,6 @@ function PronounsPage(props) { onSelectRow={updatePronouns} onChangeText={onChangeText} initiallyFocusedOptionKey={initiallyFocusedOption.keyForList} - shouldDelayFocus /> )} diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index 718b2bd91290..8c9311eb3517 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -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 /> diff --git a/src/pages/workspace/WorkspaceInvitePage.js b/src/pages/workspace/WorkspaceInvitePage.js index 6db3a20a3e4a..4626a3f171eb 100644 --- a/src/pages/workspace/WorkspaceInvitePage.js +++ b/src/pages/workspace/WorkspaceInvitePage.js @@ -227,7 +227,6 @@ function WorkspaceInvitePage(props) { onSelectRow={toggleOption} onConfirm={inviteUser} showScrollIndicator - shouldDelayFocus showLoadingPlaceholder={!didScreenTransitionEnd || !OptionsListUtils.isPersonalDetailsReady(props.personalDetails)} /> diff --git a/src/pages/workspace/WorkspaceMembersPage.js b/src/pages/workspace/WorkspaceMembersPage.js index d598f90e4326..53ddd0fb5d96 100644 --- a/src/pages/workspace/WorkspaceMembersPage.js +++ b/src/pages/workspace/WorkspaceMembersPage.js @@ -393,7 +393,6 @@ function WorkspaceMembersPage(props) { onSelectAll={() => toggleAllUsers(data)} onDismissError={dismissError} showLoadingPlaceholder={!OptionsListUtils.isPersonalDetailsReady(props.personalDetails) || _.isEmpty(props.policyMembers)} - shouldDelayFocus showScrollIndicator /> From 5e9fcf8aea250cfc770cf9b59e65cfd144683b4f Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Thu, 31 Aug 2023 18:53:00 +0100 Subject: [PATCH 2/3] chore: fix lint and tests --- src/components/SelectionList/BaseSelectionList.js | 2 +- tests/perf-test/SelectionList.perf-test.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 63daae22ef5d..b76ded8a542f 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -2,6 +2,7 @@ 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'; @@ -23,7 +24,6 @@ import useLocalize from '../../hooks/useLocalize'; import Log from '../../libs/Log'; import OptionsListSkeletonView from '../OptionsListSkeletonView'; import useActiveElement from '../../hooks/useActiveElement'; -import {useFocusEffect} from '@react-navigation/native'; const propTypes = { ...keyboardStatePropTypes, diff --git a/tests/perf-test/SelectionList.perf-test.js b/tests/perf-test/SelectionList.perf-test.js index 82cec956713f..d16875e31357 100644 --- a/tests/perf-test/SelectionList.perf-test.js +++ b/tests/perf-test/SelectionList.perf-test.js @@ -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([]); From 13ed92c91b0a05b2f50a4cd0eec5a18ac6184a4e Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Tue, 12 Sep 2023 08:54:29 +0100 Subject: [PATCH 3/3] chore: fix lint --- src/pages/settings/Profile/PronounsPage.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/settings/Profile/PronounsPage.js b/src/pages/settings/Profile/PronounsPage.js index 83f52ff89247..5ce9957f568f 100644 --- a/src/pages/settings/Profile/PronounsPage.js +++ b/src/pages/settings/Profile/PronounsPage.js @@ -80,8 +80,7 @@ function PronounsPage({currentUserPersonalDetails}) { onSelectRow={updatePronouns} onChangeText={setSearchValue} initiallyFocusedOptionKey={currentPronounsKey} - /> - + /> ); }