From b89c911cfdf6ca8f475ed6bb2ed4e922c41d2d02 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 19:55:57 +0100 Subject: [PATCH 01/13] Fix bug with User can access the background list using the arrow keys while the RHP is open --- .../BaseArrowKeyFocusManager.js} | 43 +++---------------- src/components/ArrowKeyFocusManager/index.js | 22 ++++++++++ .../ArrowKeyFocusManager/propTypes.js | 35 +++++++++++++++ 3 files changed, 64 insertions(+), 36 deletions(-) rename src/components/{ArrowKeyFocusManager.js => ArrowKeyFocusManager/BaseArrowKeyFocusManager.js} (68%) create mode 100644 src/components/ArrowKeyFocusManager/index.js create mode 100644 src/components/ArrowKeyFocusManager/propTypes.js diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager/BaseArrowKeyFocusManager.js similarity index 68% rename from src/components/ArrowKeyFocusManager.js rename to src/components/ArrowKeyFocusManager/BaseArrowKeyFocusManager.js index 19dc3a7ac614..6e37dbec04aa 100644 --- a/src/components/ArrowKeyFocusManager.js +++ b/src/components/ArrowKeyFocusManager/BaseArrowKeyFocusManager.js @@ -1,38 +1,9 @@ -import PropTypes from 'prop-types'; import {Component} from 'react'; import KeyboardShortcut from '@libs/KeyboardShortcut'; import CONST from '@src/CONST'; +import {arrowKeyFocusManagerDefaultProps, arrowKeyFocusManagerPropTypes} from './propTypes'; -const propTypes = { - /** Children to render. */ - children: PropTypes.oneOfType([PropTypes.func, PropTypes.node]).isRequired, - - /** Array of disabled indexes. */ - disabledIndexes: PropTypes.arrayOf(PropTypes.number), - - /** The current focused index. */ - focusedIndex: PropTypes.number.isRequired, - - /** The maximum index – provided so that the focus can be sent back to the beginning of the list when the end is reached. */ - maxIndex: PropTypes.number.isRequired, - - /** A callback executed when the focused input changes. */ - onFocusedIndexChanged: PropTypes.func.isRequired, - - /** If this value is true, then we exclude TextArea Node. */ - shouldExcludeTextAreaNodes: PropTypes.bool, - - /** If this value is true, then the arrow down callback would be triggered when the max index is exceeded */ - shouldResetIndexOnEndReached: PropTypes.bool, -}; - -const defaultProps = { - disabledIndexes: [], - shouldExcludeTextAreaNodes: true, - shouldResetIndexOnEndReached: true, -}; - -class ArrowKeyFocusManager extends Component { +class BaseArrowKeyFocusManager extends Component { componentDidMount() { const arrowUpConfig = CONST.KEYBOARD_SHORTCUTS.ARROW_UP; const arrowDownConfig = CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN; @@ -77,7 +48,7 @@ class ArrowKeyFocusManager extends Component { } onArrowUpKey() { - if (this.props.maxIndex < 0) { + if (this.props.maxIndex < 0 || !this.props.isFocused) { return; } @@ -96,7 +67,7 @@ class ArrowKeyFocusManager extends Component { } onArrowDownKey() { - if (this.props.maxIndex < 0) { + if (this.props.maxIndex < 0 || !this.props.isFocused) { return; } @@ -119,7 +90,7 @@ class ArrowKeyFocusManager extends Component { } } -ArrowKeyFocusManager.propTypes = propTypes; -ArrowKeyFocusManager.defaultProps = defaultProps; +BaseArrowKeyFocusManager.propTypes = arrowKeyFocusManagerPropTypes; +BaseArrowKeyFocusManager.defaultProps = arrowKeyFocusManagerDefaultProps; -export default ArrowKeyFocusManager; +export default BaseArrowKeyFocusManager; diff --git a/src/components/ArrowKeyFocusManager/index.js b/src/components/ArrowKeyFocusManager/index.js new file mode 100644 index 000000000000..0fde2d357c1a --- /dev/null +++ b/src/components/ArrowKeyFocusManager/index.js @@ -0,0 +1,22 @@ +import {useIsFocused} from '@react-navigation/native'; +import React from 'react'; +import BaseArrowKeyFocusManager from './BaseArrowKeyFocusManager'; +import {arrowKeyFocusManagerDefaultProps, arrowKeyFocusManagerPropTypes} from './propTypes'; + +function ArrowKeyFocusManager(props) { + const isFocused = useIsFocused(); + + return ( + + ); +} + +ArrowKeyFocusManager.propTypes = arrowKeyFocusManagerPropTypes; +ArrowKeyFocusManager.defaultProps = arrowKeyFocusManagerDefaultProps; +ArrowKeyFocusManager.displayName = 'ArrowKeyFocusManager'; + +export default ArrowKeyFocusManager; diff --git a/src/components/ArrowKeyFocusManager/propTypes.js b/src/components/ArrowKeyFocusManager/propTypes.js new file mode 100644 index 000000000000..3f3154f745c5 --- /dev/null +++ b/src/components/ArrowKeyFocusManager/propTypes.js @@ -0,0 +1,35 @@ +import PropTypes from 'prop-types'; + +const arrowKeyFocusManagerPropTypes = { + /** Children to render. */ + children: PropTypes.oneOfType([PropTypes.func, PropTypes.node]).isRequired, + + /** Array of disabled indexes. */ + disabledIndexes: PropTypes.arrayOf(PropTypes.number), + + /** The current focused index. */ + focusedIndex: PropTypes.number.isRequired, + + /** The maximum index – provided so that the focus can be sent back to the beginning of the list when the end is reached. */ + maxIndex: PropTypes.number.isRequired, + + /** A callback executed when the focused input changes. */ + onFocusedIndexChanged: PropTypes.func.isRequired, + + /** If this value is true, then we exclude TextArea Node. */ + shouldExcludeTextAreaNodes: PropTypes.bool, + + /** If this value is true, then the arrow down callback would be triggered when the max index is exceeded */ + shouldResetIndexOnEndReached: PropTypes.bool, + + /** Whether navigation is focused */ + isFocused: PropTypes.bool, +}; + +const arrowKeyFocusManagerDefaultProps = { + disabledIndexes: [], + shouldExcludeTextAreaNodes: true, + shouldResetIndexOnEndReached: true, +}; + +export {arrowKeyFocusManagerDefaultProps, arrowKeyFocusManagerPropTypes}; From b771515740111eb72e0de2a797663b68eba6c3c9 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 20:10:11 +0100 Subject: [PATCH 02/13] Refactor ArrowKeyFocusManager --- ...ocusManager.js => ArrowKeyFocusManager.js} | 56 +++++++++++++++++-- src/components/ArrowKeyFocusManager/index.js | 22 -------- .../ArrowKeyFocusManager/propTypes.js | 35 ------------ 3 files changed, 51 insertions(+), 62 deletions(-) rename src/components/{ArrowKeyFocusManager/BaseArrowKeyFocusManager.js => ArrowKeyFocusManager.js} (63%) delete mode 100644 src/components/ArrowKeyFocusManager/index.js delete mode 100644 src/components/ArrowKeyFocusManager/propTypes.js diff --git a/src/components/ArrowKeyFocusManager/BaseArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js similarity index 63% rename from src/components/ArrowKeyFocusManager/BaseArrowKeyFocusManager.js rename to src/components/ArrowKeyFocusManager.js index 6e37dbec04aa..c8bdf1845f2b 100644 --- a/src/components/ArrowKeyFocusManager/BaseArrowKeyFocusManager.js +++ b/src/components/ArrowKeyFocusManager.js @@ -1,7 +1,40 @@ -import {Component} from 'react'; +import {useIsFocused} from '@react-navigation/native'; +import PropTypes from 'prop-types'; +import React, {Component} from 'react'; import KeyboardShortcut from '@libs/KeyboardShortcut'; import CONST from '@src/CONST'; -import {arrowKeyFocusManagerDefaultProps, arrowKeyFocusManagerPropTypes} from './propTypes'; + +const propTypes = { + /** Children to render. */ + children: PropTypes.oneOfType([PropTypes.func, PropTypes.node]).isRequired, + + /** Array of disabled indexes. */ + disabledIndexes: PropTypes.arrayOf(PropTypes.number), + + /** The current focused index. */ + focusedIndex: PropTypes.number.isRequired, + + /** The maximum index – provided so that the focus can be sent back to the beginning of the list when the end is reached. */ + maxIndex: PropTypes.number.isRequired, + + /** A callback executed when the focused input changes. */ + onFocusedIndexChanged: PropTypes.func.isRequired, + + /** Whether navigation is focused */ + isFocused: PropTypes.bool.isRequired, + + /** If this value is true, then we exclude TextArea Node. */ + shouldExcludeTextAreaNodes: PropTypes.bool, + + /** If this value is true, then the arrow down callback would be triggered when the max index is exceeded */ + shouldResetIndexOnEndReached: PropTypes.bool, +}; + +const defaultProps = { + disabledIndexes: [], + shouldExcludeTextAreaNodes: true, + shouldResetIndexOnEndReached: true, +}; class BaseArrowKeyFocusManager extends Component { componentDidMount() { @@ -90,7 +123,20 @@ class BaseArrowKeyFocusManager extends Component { } } -BaseArrowKeyFocusManager.propTypes = arrowKeyFocusManagerPropTypes; -BaseArrowKeyFocusManager.defaultProps = arrowKeyFocusManagerDefaultProps; +function ArrowKeyFocusManager(props) { + const isFocused = useIsFocused(); + + return ( + + ); +} + +BaseArrowKeyFocusManager.propTypes = propTypes; +BaseArrowKeyFocusManager.defaultProps = defaultProps; +ArrowKeyFocusManager.displayName = 'ArrowKeyFocusManager'; -export default BaseArrowKeyFocusManager; +export default ArrowKeyFocusManager; diff --git a/src/components/ArrowKeyFocusManager/index.js b/src/components/ArrowKeyFocusManager/index.js deleted file mode 100644 index 0fde2d357c1a..000000000000 --- a/src/components/ArrowKeyFocusManager/index.js +++ /dev/null @@ -1,22 +0,0 @@ -import {useIsFocused} from '@react-navigation/native'; -import React from 'react'; -import BaseArrowKeyFocusManager from './BaseArrowKeyFocusManager'; -import {arrowKeyFocusManagerDefaultProps, arrowKeyFocusManagerPropTypes} from './propTypes'; - -function ArrowKeyFocusManager(props) { - const isFocused = useIsFocused(); - - return ( - - ); -} - -ArrowKeyFocusManager.propTypes = arrowKeyFocusManagerPropTypes; -ArrowKeyFocusManager.defaultProps = arrowKeyFocusManagerDefaultProps; -ArrowKeyFocusManager.displayName = 'ArrowKeyFocusManager'; - -export default ArrowKeyFocusManager; diff --git a/src/components/ArrowKeyFocusManager/propTypes.js b/src/components/ArrowKeyFocusManager/propTypes.js deleted file mode 100644 index 3f3154f745c5..000000000000 --- a/src/components/ArrowKeyFocusManager/propTypes.js +++ /dev/null @@ -1,35 +0,0 @@ -import PropTypes from 'prop-types'; - -const arrowKeyFocusManagerPropTypes = { - /** Children to render. */ - children: PropTypes.oneOfType([PropTypes.func, PropTypes.node]).isRequired, - - /** Array of disabled indexes. */ - disabledIndexes: PropTypes.arrayOf(PropTypes.number), - - /** The current focused index. */ - focusedIndex: PropTypes.number.isRequired, - - /** The maximum index – provided so that the focus can be sent back to the beginning of the list when the end is reached. */ - maxIndex: PropTypes.number.isRequired, - - /** A callback executed when the focused input changes. */ - onFocusedIndexChanged: PropTypes.func.isRequired, - - /** If this value is true, then we exclude TextArea Node. */ - shouldExcludeTextAreaNodes: PropTypes.bool, - - /** If this value is true, then the arrow down callback would be triggered when the max index is exceeded */ - shouldResetIndexOnEndReached: PropTypes.bool, - - /** Whether navigation is focused */ - isFocused: PropTypes.bool, -}; - -const arrowKeyFocusManagerDefaultProps = { - disabledIndexes: [], - shouldExcludeTextAreaNodes: true, - shouldResetIndexOnEndReached: true, -}; - -export {arrowKeyFocusManagerDefaultProps, arrowKeyFocusManagerPropTypes}; From 108f04f9effc6811b53cd52b01f0d0a6db4e2bc9 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 21:39:35 +0100 Subject: [PATCH 03/13] Update propTypes for ArrowKeyFocusManager --- src/components/ArrowKeyFocusManager.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js index c8bdf1845f2b..2532e52156df 100644 --- a/src/components/ArrowKeyFocusManager.js +++ b/src/components/ArrowKeyFocusManager.js @@ -17,12 +17,12 @@ const propTypes = { /** The maximum index – provided so that the focus can be sent back to the beginning of the list when the end is reached. */ maxIndex: PropTypes.number.isRequired, - /** A callback executed when the focused input changes. */ - onFocusedIndexChanged: PropTypes.func.isRequired, - /** Whether navigation is focused */ isFocused: PropTypes.bool.isRequired, + /** A callback executed when the focused input changes. */ + onFocusedIndexChanged: PropTypes.func.isRequired, + /** If this value is true, then we exclude TextArea Node. */ shouldExcludeTextAreaNodes: PropTypes.bool, From e3f803b5998dad4bc24e009cae014f329514b838 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 21:54:50 +0100 Subject: [PATCH 04/13] Update propTypes for ArrowKeyFocusManager x2 --- src/components/ArrowKeyFocusManager.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js index 2532e52156df..3dccdb92bd0d 100644 --- a/src/components/ArrowKeyFocusManager.js +++ b/src/components/ArrowKeyFocusManager.js @@ -18,7 +18,7 @@ const propTypes = { maxIndex: PropTypes.number.isRequired, /** Whether navigation is focused */ - isFocused: PropTypes.bool.isRequired, + isFocused: PropTypes.bool, /** A callback executed when the focused input changes. */ onFocusedIndexChanged: PropTypes.func.isRequired, @@ -34,6 +34,7 @@ const defaultProps = { disabledIndexes: [], shouldExcludeTextAreaNodes: true, shouldResetIndexOnEndReached: true, + isFocused: false, }; class BaseArrowKeyFocusManager extends Component { From 9efe7126b4f47606b3c587ab63a56d6bc16b2257 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 22:17:20 +0100 Subject: [PATCH 05/13] Update perf-tests --- src/components/ArrowKeyFocusManager.js | 1 - tests/perf-test/SearchPage.perf-test.tsx | 5 +---- tests/perf-test/SignInPage.perf-test.tsx | 5 +---- tests/utils/LHNTestUtils.tsx | 8 +------- 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js index 3dccdb92bd0d..c78d97ad68fe 100644 --- a/src/components/ArrowKeyFocusManager.js +++ b/src/components/ArrowKeyFocusManager.js @@ -34,7 +34,6 @@ const defaultProps = { disabledIndexes: [], shouldExcludeTextAreaNodes: true, shouldResetIndexOnEndReached: true, - isFocused: false, }; class BaseArrowKeyFocusManager extends Component { diff --git a/tests/perf-test/SearchPage.perf-test.tsx b/tests/perf-test/SearchPage.perf-test.tsx index 5f2dd3800e0f..489c282898cd 100644 --- a/tests/perf-test/SearchPage.perf-test.tsx +++ b/tests/perf-test/SearchPage.perf-test.tsx @@ -43,15 +43,12 @@ jest.mock('@src/libs/API', () => ({ jest.mock('@src/libs/Navigation/Navigation'); -const mockedNavigate = jest.fn(); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); return { ...actualNav, useFocusEffect: jest.fn(), - useIsFocused: () => ({ - navigate: mockedNavigate, - }), + useIsFocused: () => true, useRoute: () => jest.fn(), useNavigation: () => ({ navigate: jest.fn(), diff --git a/tests/perf-test/SignInPage.perf-test.tsx b/tests/perf-test/SignInPage.perf-test.tsx index e3e2c20ae72a..651d85a70e58 100644 --- a/tests/perf-test/SignInPage.perf-test.tsx +++ b/tests/perf-test/SignInPage.perf-test.tsx @@ -26,15 +26,12 @@ jest.mock('../../src/libs/API', () => ({ read: jest.fn(), })); -const mockedNavigate = jest.fn(); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); return { ...actualNav, useFocusEffect: jest.fn(), - useIsFocused: () => ({ - navigate: mockedNavigate, - }), + useIsFocused: () => true, useRoute: () => jest.fn(), useNavigation: () => ({ navigate: jest.fn(), diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index f6eee590313b..58c765b5c98e 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -1,7 +1,5 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import type {NavigationProp} from '@react-navigation/core/src/types'; import type * as Navigation from '@react-navigation/native'; -import type {ParamListBase} from '@react-navigation/routers'; import {render} from '@testing-library/react-native'; import type {ReactElement} from 'react'; import React from 'react'; @@ -33,17 +31,13 @@ type MockedSidebarLinksProps = { currentReportID?: string; }; -// we have to mock `useIsFocused` because it's used in the SidebarLinks component -const mockedNavigate: jest.MockedFn['navigate']> = jest.fn(); jest.mock('@react-navigation/native', (): typeof Navigation => { const actualNav = jest.requireActual('@react-navigation/native'); return { ...actualNav, useRoute: jest.fn(), useFocusEffect: jest.fn(), - useIsFocused: () => ({ - navigate: mockedNavigate, - }), + useIsFocused: () => true, useNavigation: () => ({ navigate: jest.fn(), addListener: jest.fn(), From 9eba416539f7960f8eabbc0f196af620499aff5c Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 22:18:03 +0100 Subject: [PATCH 06/13] Update perf-tests x2 --- src/components/ArrowKeyFocusManager.js | 2 +- tests/perf-test/ReportActionCompose.perf-test.tsx | 4 +--- tests/perf-test/ReportScreen.perf-test.tsx | 5 +---- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js index c78d97ad68fe..2532e52156df 100644 --- a/src/components/ArrowKeyFocusManager.js +++ b/src/components/ArrowKeyFocusManager.js @@ -18,7 +18,7 @@ const propTypes = { maxIndex: PropTypes.number.isRequired, /** Whether navigation is focused */ - isFocused: PropTypes.bool, + isFocused: PropTypes.bool.isRequired, /** A callback executed when the focused input changes. */ onFocusedIndexChanged: PropTypes.func.isRequired, diff --git a/tests/perf-test/ReportActionCompose.perf-test.tsx b/tests/perf-test/ReportActionCompose.perf-test.tsx index 8d6cb1ac7e57..7d3846e4e29a 100644 --- a/tests/perf-test/ReportActionCompose.perf-test.tsx +++ b/tests/perf-test/ReportActionCompose.perf-test.tsx @@ -38,9 +38,7 @@ jest.mock('@react-navigation/native', () => { navigate: jest.fn(), addListener: () => jest.fn(), }), - useIsFocused: () => ({ - navigate: jest.fn(), - }), + useIsFocused: () => true, } as typeof Navigation; }); diff --git a/tests/perf-test/ReportScreen.perf-test.tsx b/tests/perf-test/ReportScreen.perf-test.tsx index ff3d1473c662..b55f4b9ccb93 100644 --- a/tests/perf-test/ReportScreen.perf-test.tsx +++ b/tests/perf-test/ReportScreen.perf-test.tsx @@ -81,15 +81,12 @@ jest.mock('@src/hooks/usePermissions.ts'); jest.mock('@src/libs/Navigation/Navigation'); -const mockedNavigate = jest.fn(); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); return { ...actualNav, useFocusEffect: jest.fn(), - useIsFocused: () => ({ - navigate: mockedNavigate, - }), + useIsFocused: () => true, useRoute: () => jest.fn(), useNavigation: () => ({ navigate: jest.fn(), From defd7311fdb2bc37ce93981dc97ac00eef344db7 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 22:24:05 +0100 Subject: [PATCH 07/13] Update perf-tests x3 --- src/components/SelectionList/BaseListItem.tsx | 138 ++++++++---------- .../SelectionList/TableListItem.tsx | 12 -- src/pages/workspace/WorkspaceMembersPage.tsx | 1 + .../ReportActionCompose.perf-test.tsx | 2 +- tests/perf-test/SignInPage.perf-test.tsx | 2 +- 5 files changed, 67 insertions(+), 88 deletions(-) diff --git a/src/components/SelectionList/BaseListItem.tsx b/src/components/SelectionList/BaseListItem.tsx index 42fdc7dc575e..67de9153d0d6 100644 --- a/src/components/SelectionList/BaseListItem.tsx +++ b/src/components/SelectionList/BaseListItem.tsx @@ -58,85 +58,75 @@ function BaseListItem({ }; return ( - onDismissError(item)} - pendingAction={pendingAction} - errors={errors} - errorRowStyles={styles.ph5} - style={containerStyle} + onSelectRow(item)} + disabled={isDisabled} + accessibilityLabel={item.text ?? ''} + hoverDimmingValue={1} + hoverStyle={!item.isDisabled && !item.isSelected && styles.hoveredComponentBG} + onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined} + nativeID={keyForList ?? ''} + style={pressableStyle} > - onSelectRow(item)} - disabled={isDisabled} - accessibilityLabel={item.text ?? ''} - role={CONST.ROLE.BUTTON} - hoverDimmingValue={1} - hoverStyle={!item.isDisabled && !item.isSelected && styles.hoveredComponentBG} - dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}} - onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined} - nativeID={keyForList ?? ''} - style={pressableStyle} - > - - {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.LEFT && ( - - - {item.isSelected && ( - - )} - - - )} - - {typeof children === 'function' ? children(hovered) : children} - - {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.RIGHT && ( - - - - )} - - {!canSelectMultiple && item.isSelected && !rightHandSideComponent && ( - - + + {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.LEFT && ( + + + {item.isSelected && ( - + )} + + + )} + + {typeof children === 'function' ? children(hovered) : children} + + {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.RIGHT && ( + + + + )} + + {!canSelectMultiple && item.isSelected && !rightHandSideComponent && ( + + + - )} - {rightHandSideComponentRender()} - - {FooterComponent} - - + + )} + {rightHandSideComponentRender()} + + {FooterComponent} + ); } diff --git a/src/components/SelectionList/TableListItem.tsx b/src/components/SelectionList/TableListItem.tsx index b7c3ed549d82..1fb996bc13b9 100644 --- a/src/components/SelectionList/TableListItem.tsx +++ b/src/components/SelectionList/TableListItem.tsx @@ -31,10 +31,6 @@ function TableListItem({ return ( {!!item.alternateText && ( + // { navigate: jest.fn(), addListener: () => jest.fn(), }), - useIsFocused: () => true, + useIsFocused: () => true, } as typeof Navigation; }); diff --git a/tests/perf-test/SignInPage.perf-test.tsx b/tests/perf-test/SignInPage.perf-test.tsx index 651d85a70e58..dc93b0d81059 100644 --- a/tests/perf-test/SignInPage.perf-test.tsx +++ b/tests/perf-test/SignInPage.perf-test.tsx @@ -31,7 +31,7 @@ jest.mock('@react-navigation/native', () => { return { ...actualNav, useFocusEffect: jest.fn(), - useIsFocused: () => true, + useIsFocused: () => true, useRoute: () => jest.fn(), useNavigation: () => ({ navigate: jest.fn(), From aeb387f1c930698143fc5ade272f349422df68bf Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 22:27:43 +0100 Subject: [PATCH 08/13] Revert some changes --- src/components/SelectionList/BaseListItem.tsx | 138 ++++++++++-------- .../SelectionList/TableListItem.tsx | 12 ++ src/pages/workspace/WorkspaceMembersPage.tsx | 1 - 3 files changed, 86 insertions(+), 65 deletions(-) diff --git a/src/components/SelectionList/BaseListItem.tsx b/src/components/SelectionList/BaseListItem.tsx index 67de9153d0d6..6acb9284db6b 100644 --- a/src/components/SelectionList/BaseListItem.tsx +++ b/src/components/SelectionList/BaseListItem.tsx @@ -58,75 +58,85 @@ function BaseListItem({ }; return ( - onSelectRow(item)} - disabled={isDisabled} - accessibilityLabel={item.text ?? ''} - hoverDimmingValue={1} - hoverStyle={!item.isDisabled && !item.isSelected && styles.hoveredComponentBG} - onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined} - nativeID={keyForList ?? ''} - style={pressableStyle} + onDismissError(item)} + pendingAction={pendingAction} + errors={errors} + errorRowStyles={styles.ph5} + style={containerStyle} > - - {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.LEFT && ( - - - {item.isSelected && ( - - )} - - - )} - - {typeof children === 'function' ? children(hovered) : children} + onSelectRow(item)} + disabled={isDisabled} + accessibilityLabel={item.text ?? ''} + role={CONST.ROLE.BUTTON} + hoverDimmingValue={1} + hoverStyle={!item.isSelected && styles.hoveredComponentBG} + dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}} + onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined} + nativeID={keyForList ?? ''} + style={pressableStyle} + > + + {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.LEFT && ( + + + {item.isSelected && ( + + )} + + + )} - {canSelectMultiple && checkmarkPosition === CONST.DIRECTION.RIGHT && ( - - - - )} + {typeof children === 'function' ? children(hovered) : children} - {!canSelectMultiple && item.isSelected && !rightHandSideComponent && ( - - - + + + )} + + {!canSelectMultiple && item.isSelected && !rightHandSideComponent && ( + + + + - - )} - {rightHandSideComponentRender()} - - {FooterComponent} - + )} + {rightHandSideComponentRender()} + + {FooterComponent} + + ); } diff --git a/src/components/SelectionList/TableListItem.tsx b/src/components/SelectionList/TableListItem.tsx index 1fb996bc13b9..b7c3ed549d82 100644 --- a/src/components/SelectionList/TableListItem.tsx +++ b/src/components/SelectionList/TableListItem.tsx @@ -31,6 +31,10 @@ function TableListItem({ return ( {!!item.alternateText && ( - // Date: Sat, 30 Mar 2024 22:28:52 +0100 Subject: [PATCH 09/13] Revert some changes x2 --- src/components/SelectionList/BaseListItem.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/SelectionList/BaseListItem.tsx b/src/components/SelectionList/BaseListItem.tsx index 6acb9284db6b..42fdc7dc575e 100644 --- a/src/components/SelectionList/BaseListItem.tsx +++ b/src/components/SelectionList/BaseListItem.tsx @@ -73,7 +73,7 @@ function BaseListItem({ accessibilityLabel={item.text ?? ''} role={CONST.ROLE.BUTTON} hoverDimmingValue={1} - hoverStyle={!item.isSelected && styles.hoveredComponentBG} + hoverStyle={!item.isDisabled && !item.isSelected && styles.hoveredComponentBG} dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}} onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined} nativeID={keyForList ?? ''} @@ -87,7 +87,7 @@ function BaseListItem({ // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing disabled={isDisabled || item.isDisabledCheckbox} onPress={handleCheckboxPress} - style={[styles.cursorUnset, StyleUtils.getCheckboxPressableStyle(), item.isDisabledCheckbox && styles.cursorDisabled]} + style={[styles.cursorUnset, StyleUtils.getCheckboxPressableStyle(), item.isDisabledCheckbox && styles.cursorDisabled, styles.mr3]} > {item.isSelected && ( From 13affe7836a81c1b3eef58f1e677b7f4a545eee1 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 22:51:00 +0100 Subject: [PATCH 10/13] Update perf-tests --- tests/perf-test/SidebarUtils.perf-test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 8566abb97c7f..df99f9ce3758 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -2,6 +2,7 @@ import {rand} from '@ngneat/falso'; import type {OnyxCollection} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import {measureFunction} from 'reassure'; +import type Navigation from '@libs/Navigation/Navigation'; import SidebarUtils from '@libs/SidebarUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -73,6 +74,18 @@ const allReportActions = Object.fromEntries( const currentReportId = '1'; const transactionViolations = {} as OnyxCollection; +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + navigate: jest.fn(), + addListener: () => jest.fn(), + }), + useIsFocused: () => true, + } as typeof Navigation; +}); + describe('SidebarUtils', () => { beforeAll(() => { Onyx.init({ From aae8ff4b45a59e811e25a384fe472d7080931c26 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 23:43:37 +0100 Subject: [PATCH 11/13] Update perf-tests x2 --- tests/perf-test/OptionsSelector.perf-test.tsx | 13 +++++++++++++ tests/perf-test/SidebarUtils.perf-test.ts | 12 ------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/tests/perf-test/OptionsSelector.perf-test.tsx b/tests/perf-test/OptionsSelector.perf-test.tsx index 44dc4ac6c317..fe234dda1e19 100644 --- a/tests/perf-test/OptionsSelector.perf-test.tsx +++ b/tests/perf-test/OptionsSelector.perf-test.tsx @@ -5,6 +5,7 @@ import type {ComponentType} from 'react'; import {measurePerformance} from 'reassure'; import type {WithLocalizeProps} from '@components/withLocalize'; import type {WithNavigationFocusProps} from '@components/withNavigationFocus'; +import type Navigation from '@libs/Navigation/Navigation'; import OptionsSelector from '@src/components/OptionsSelector'; import variables from '@src/styles/variables'; @@ -38,6 +39,18 @@ jest.mock('@src/components/withNavigationFocus', () => (Component: ComponentType return WithNavigationFocus; }); +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + navigate: jest.fn(), + addListener: () => jest.fn(), + }), + useIsFocused: () => true, + } as typeof Navigation; +}); + type GenerateSectionsProps = Array<{numberOfItems: number; shouldShow?: boolean}>; const generateSections = (sections: GenerateSectionsProps) => diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index df99f9ce3758..e9ab30495c87 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -74,18 +74,6 @@ const allReportActions = Object.fromEntries( const currentReportId = '1'; const transactionViolations = {} as OnyxCollection; -jest.mock('@react-navigation/native', () => { - const actualNav = jest.requireActual('@react-navigation/native'); - return { - ...actualNav, - useNavigation: () => ({ - navigate: jest.fn(), - addListener: () => jest.fn(), - }), - useIsFocused: () => true, - } as typeof Navigation; -}); - describe('SidebarUtils', () => { beforeAll(() => { Onyx.init({ From 917cf001c037c2f4ea5ccf9432d94ac5c6c68f65 Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sat, 30 Mar 2024 23:44:12 +0100 Subject: [PATCH 12/13] Update perf-tests x3 --- tests/perf-test/SidebarUtils.perf-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index e9ab30495c87..8566abb97c7f 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -2,7 +2,6 @@ import {rand} from '@ngneat/falso'; import type {OnyxCollection} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import {measureFunction} from 'reassure'; -import type Navigation from '@libs/Navigation/Navigation'; import SidebarUtils from '@libs/SidebarUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; From f4f7b19e4bd8f45dc472056cee99f309928fa79f Mon Sep 17 00:00:00 2001 From: Yauheni Date: Sun, 31 Mar 2024 01:14:03 +0100 Subject: [PATCH 13/13] Fix bug with blue border --- src/pages/workspace/categories/WorkspaceCategoriesPage.tsx | 2 ++ src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx | 2 ++ src/pages/workspace/tags/WorkspaceTagsPage.tsx | 2 ++ src/pages/workspace/taxes/WorkspaceTaxesPage.tsx | 2 ++ 4 files changed, 8 insertions(+) diff --git a/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx b/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx index c5e79effe276..c4975ab340be 100644 --- a/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx +++ b/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx @@ -23,6 +23,7 @@ import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import {deleteWorkspaceCategories, setWorkspaceCategoryEnabled} from '@libs/actions/Policy'; +import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import localeCompare from '@libs/LocaleCompare'; import Navigation from '@libs/Navigation/Navigation'; import * as PolicyUtils from '@libs/PolicyUtils'; @@ -316,6 +317,7 @@ function WorkspaceCategoriesPage({policy, policyCategories, route}: WorkspaceCat sections={[{data: categoryList, isDisabled: false}]} onCheckboxPress={toggleCategory} onSelectRow={navigateToCategorySettings} + shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()} onSelectAll={toggleAllCategories} showScrollIndicator ListItem={TableListItem} diff --git a/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx b/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx index a5356a8fd05a..ac84455b0e10 100644 --- a/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx +++ b/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx @@ -21,6 +21,7 @@ import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import * as CurrencyUtils from '@libs/CurrencyUtils'; +import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import Navigation from '@libs/Navigation/Navigation'; import type {WorkspacesCentralPaneNavigatorParamList} from '@navigation/types'; import AdminPolicyAccessOrNotFoundWrapper from '@pages/workspace/AdminPolicyAccessOrNotFoundWrapper'; @@ -308,6 +309,7 @@ function PolicyDistanceRatesPage({policy, route}: PolicyDistanceRatesPageProps) onDismissError={dismissError} showScrollIndicator ListItem={TableListItem} + shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()} customListHeader={getCustomListHeader()} listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]} /> diff --git a/src/pages/workspace/tags/WorkspaceTagsPage.tsx b/src/pages/workspace/tags/WorkspaceTagsPage.tsx index 53376c05878f..118f32549386 100644 --- a/src/pages/workspace/tags/WorkspaceTagsPage.tsx +++ b/src/pages/workspace/tags/WorkspaceTagsPage.tsx @@ -23,6 +23,7 @@ import useNetwork from '@hooks/useNetwork'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; +import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import localeCompare from '@libs/LocaleCompare'; import Navigation from '@libs/Navigation/Navigation'; import * as PolicyUtils from '@libs/PolicyUtils'; @@ -315,6 +316,7 @@ function WorkspaceTagsPage({policyTags, route}: WorkspaceTagsPageProps) { showScrollIndicator ListItem={TableListItem} customListHeader={getCustomListHeader()} + shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()} listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]} onDismissError={(item) => Policy.clearPolicyTagErrors(route.params.policyID, item.value)} /> diff --git a/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx b/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx index 4f8782dcdf3f..ce9bbf1e9ae4 100644 --- a/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx +++ b/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx @@ -21,6 +21,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import {openPolicyTaxesPage} from '@libs/actions/Policy'; import {clearTaxRateError, deletePolicyTaxes, setPolicyTaxesEnabled} from '@libs/actions/TaxRate'; +import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import * as ErrorUtils from '@libs/ErrorUtils'; import Navigation from '@libs/Navigation/Navigation'; import * as PolicyUtils from '@libs/PolicyUtils'; @@ -276,6 +277,7 @@ function WorkspaceTaxesPage({ showScrollIndicator ListItem={TableListItem} customListHeader={getCustomListHeader()} + shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()} listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]} onDismissError={(item) => (item.keyForList ? clearTaxRateError(policyID, item.keyForList, item.pendingAction) : undefined)} />