Merge pull request #39327 from ZhenjaHorbach/user-can-access-the-back…

…ground-list-using-the-arrow-keys-while-the-RHP-is-open User can access the background list using the arrow keys while the RHP is open
Expensify · Apr 5, 2024 · 200de35 · 200de35
2 parents a01b338 + 2f1cede
commit 200de35
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 28 deletions.
diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js
@@ -1,5 +1,6 @@
+import {useIsFocused} from '@react-navigation/native';
 import PropTypes from 'prop-types';
-import {Component} from 'react';
+import React, {Component} from 'react';
 import KeyboardShortcut from '@libs/KeyboardShortcut';
 import CONST from '@src/CONST';
 
@@ -16,6 +17,9 @@ 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,
 
+    /** Whether navigation is focused */
+    isFocused: PropTypes.bool.isRequired,
+
     /** A callback executed when the focused input changes. */
     onFocusedIndexChanged: PropTypes.func.isRequired,
 
@@ -32,7 +36,7 @@ const defaultProps = {
     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 +81,7 @@ class ArrowKeyFocusManager extends Component {
     }
 
     onArrowUpKey() {
-        if (this.props.maxIndex < 0) {
+        if (this.props.maxIndex < 0 || !this.props.isFocused) {
             return;
         }
 
@@ -96,7 +100,7 @@ class ArrowKeyFocusManager extends Component {
     }
 
     onArrowDownKey() {
-        if (this.props.maxIndex < 0) {
+        if (this.props.maxIndex < 0 || !this.props.isFocused) {
             return;
         }
 
@@ -119,7 +123,20 @@ class ArrowKeyFocusManager extends Component {
     }
 }
 
-ArrowKeyFocusManager.propTypes = propTypes;
-ArrowKeyFocusManager.defaultProps = defaultProps;
+function ArrowKeyFocusManager(props) {
+    const isFocused = useIsFocused();
+
+    return (
+        <BaseArrowKeyFocusManager
+            // eslint-disable-next-line react/jsx-props-no-spreading
+            {...props}
+            isFocused={isFocused}
+        />
+    );
+}
+
+BaseArrowKeyFocusManager.propTypes = propTypes;
+BaseArrowKeyFocusManager.defaultProps = defaultProps;
+ArrowKeyFocusManager.displayName = 'ArrowKeyFocusManager';
 
 export default ArrowKeyFocusManager;
diff --git a/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx b/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx
@@ -24,6 +24,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';
@@ -294,6 +295,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
@@ -22,6 +22,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';
@@ -298,6 +299,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
@@ -24,6 +24,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';
@@ -305,6 +306,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
@@ -22,6 +22,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';
@@ -268,6 +269,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)}
                         />

diff --git 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/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
@@ -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(),

diff --git a/tests/perf-test/SearchPage.perf-test.tsx b/tests/perf-test/SearchPage.perf-test.tsx
@@ -45,15 +45,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
@@ -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
@@ -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<NavigationProp<ParamListBase>['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(),