From 8905d81e9d83a9354890d993dda45ace1d5067dd Mon Sep 17 00:00:00 2001 From: Github Date: Wed, 4 Jun 2025 16:47:45 +0200 Subject: [PATCH] perf: move screen focus to LHNOptionsList --- .../LHNOptionsList/LHNOptionsList.tsx | 9 +- .../LHNOptionsList/OptionRowLHN.tsx | 32 ++-- .../LHNOptionsList/OptionRowLHNData.tsx | 6 +- src/components/LHNOptionsList/types.ts | 10 +- tests/ui/components/LHNOptionsListTest.tsx | 174 ++++++++++++++++++ 5 files changed, 203 insertions(+), 28 deletions(-) create mode 100644 tests/ui/components/LHNOptionsListTest.tsx diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 35f1f4bde7e9..b6abe464bedb 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -1,4 +1,4 @@ -import {useRoute} from '@react-navigation/native'; +import {useIsFocused, useRoute} from '@react-navigation/native'; import type {FlashListProps} from '@shopify/flash-list'; import {FlashList} from '@shopify/flash-list'; import type {ReactElement} from 'react'; @@ -42,6 +42,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio const {isOffline} = useNetwork(); const flashListRef = useRef>(null); const route = useRoute(); + const isScreenFocused = useIsFocused(); const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {canBeMissing: false}); const [reportAttributes] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {selector: (attributes) => attributes?.reports, canBeMissing: false}); @@ -225,7 +226,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio lastReportActionTransaction={lastReportActionTransaction} receiptTransactions={transactions} viewMode={optionMode} - isFocused={!shouldDisableFocusOptions} + isOptionFocused={!shouldDisableFocusOptions} lastMessageTextFromReport={lastMessageTextFromReport} onSelectRow={onSelectRow} preferredLocale={preferredLocale} @@ -233,6 +234,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio transactionViolations={transactionViolations} onLayout={onLayoutItem} shouldShowRBRorGBRTooltip={shouldShowRBRorGBRTooltip} + isScreenFocused={isScreenFocused} /> ); }, @@ -253,6 +255,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio onLayoutItem, isOffline, firstReportIDWithGBRorRBR, + isScreenFocused, ], ); @@ -271,6 +274,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio preferredLocale, transactions, isOffline, + isScreenFocused, ], [ reportActions, @@ -286,6 +290,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio preferredLocale, transactions, isOffline, + isScreenFocused, ], ); diff --git a/src/components/LHNOptionsList/OptionRowLHN.tsx b/src/components/LHNOptionsList/OptionRowLHN.tsx index da12c0ce5eb2..f7038fb4475c 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.tsx +++ b/src/components/LHNOptionsList/OptionRowLHN.tsx @@ -1,5 +1,4 @@ -import {useFocusEffect} from '@react-navigation/native'; -import React, {useCallback, useMemo, useRef, useState} from 'react'; +import React, {useMemo, useRef, useState} from 'react'; import type {GestureResponderEvent, ViewStyle} from 'react-native'; import {StyleSheet, View} from 'react-native'; import {useOnyx} from 'react-native-onyx'; @@ -51,7 +50,7 @@ import type {OptionRowLHNProps} from './types'; function OptionRowLHN({ reportID, - isFocused = false, + isOptionFocused = false, onSelectRow = () => {}, optionItem, viewMode = 'default', @@ -59,12 +58,12 @@ function OptionRowLHN({ onLayout = () => {}, hasDraftComment, shouldShowRBRorGBRTooltip, + isScreenFocused = false, }: OptionRowLHNProps) { const theme = useTheme(); const styles = useThemeStyles(); const popoverAnchor = useRef(null); const StyleUtils = useStyleUtils(); - const [isScreenFocused, setIsScreenFocused] = useState(false); const {shouldUseNarrowLayout} = useResponsiveLayout(); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${optionItem?.reportID}`, {canBeMissing: true}); @@ -105,15 +104,6 @@ function OptionRowLHN({ const {translate} = useLocalize(); const [isContextMenuActive, setIsContextMenuActive] = useState(false); - useFocusEffect( - useCallback(() => { - setIsScreenFocused(true); - return () => { - setIsScreenFocused(false); - }; - }, []), - ); - const isInFocusMode = viewMode === CONST.OPTION_MODE.COMPACT; const sidebarInnerRowStyle = StyleSheet.flatten( isInFocusMode @@ -121,7 +111,7 @@ function OptionRowLHN({ : [styles.chatLinkRowPressable, styles.flexGrow1, styles.optionItemAvatarNameWrapper, styles.optionRow, styles.justifyContentCenter], ); - if (!optionItem && !isFocused) { + if (!optionItem && !isOptionFocused) { // rendering null as a render item causes the FlashList to render all // its children and consume significant memory on the first render. We can avoid this by // rendering a placeholder view instead. This behaviour is only observed when we @@ -140,7 +130,7 @@ function OptionRowLHN({ } const brickRoadIndicator = optionItem.brickRoadIndicator; - const textStyle = isFocused ? styles.sidebarLinkActiveText : styles.sidebarLinkText; + const textStyle = isOptionFocused ? styles.sidebarLinkActiveText : styles.sidebarLinkText; const textUnreadStyle = shouldUseBoldText(optionItem) ? [textStyle, styles.sidebarLinkTextBold] : [textStyle]; const displayNameStyle = [styles.optionDisplayName, styles.optionDisplayNameCompact, styles.pre, textUnreadStyle, style]; const alternateTextStyle = isInFocusMode @@ -188,7 +178,7 @@ function OptionRowLHN({ const statusContent = formattedDate ? `${statusText ? `${statusText} ` : ''}(${formattedDate})` : statusText; const isStatusVisible = !!emojiCode && isOneOnOneChat(!isEmptyObject(report) ? report : undefined); - const subscriptAvatarBorderColor = isFocused ? focusedBackgroundColor : theme.sidebar; + const subscriptAvatarBorderColor = isOptionFocused ? focusedBackgroundColor : theme.sidebar; const firstIcon = optionItem.icons?.at(0); const onOptionPress = (event: GestureResponderEvent | KeyboardEvent | undefined) => { @@ -256,8 +246,8 @@ function OptionRowLHN({ styles.sidebarLink, styles.sidebarLinkInnerLHN, StyleUtils.getBackgroundColorStyle(theme.sidebar), - isFocused ? styles.sidebarLinkActive : null, - (hovered || isContextMenuActive) && !isFocused ? styles.sidebarLinkHover : null, + isOptionFocused ? styles.sidebarLinkActive : null, + (hovered || isContextMenuActive) && !isOptionFocused ? styles.sidebarLinkHover : null, ]} role={CONST.ROLE.BUTTON} accessibilityLabel={`${translate('accessibilityHints.navigatesToChat')} ${optionItem.text}. ${optionItem.isUnread ? `${translate('common.unread')}.` : ''} ${ @@ -272,7 +262,7 @@ function OptionRowLHN({ firstIcon && (optionItem.shouldShowSubscript ? ( diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index 3b8d8f349a4b..641c071112fc 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -14,7 +14,7 @@ import type {OptionRowLHNDataProps} from './types'; * re-render if the data really changed. */ function OptionRowLHNData({ - isFocused = false, + isOptionFocused = false, fullReport, reportAttributes, oneTransactionThreadReport, @@ -35,7 +35,7 @@ function OptionRowLHNData({ }: OptionRowLHNDataProps) { const reportID = propsToForward.reportID; const currentReportIDValue = useCurrentReportID(); - const isReportFocused = isFocused && currentReportIDValue?.currentReportID === reportID; + const isReportFocused = isOptionFocused && currentReportIDValue?.currentReportID === reportID; const optionItemRef = useRef(undefined); const optionItem = useMemo(() => { @@ -88,7 +88,7 @@ function OptionRowLHNData({ ); diff --git a/src/components/LHNOptionsList/types.ts b/src/components/LHNOptionsList/types.ts index ef5a8d7e9d02..b290c0b55656 100644 --- a/src/components/LHNOptionsList/types.ts +++ b/src/components/LHNOptionsList/types.ts @@ -37,7 +37,7 @@ type LHNOptionsListProps = CustomLHNOptionsListProps; type OptionRowLHNDataProps = { /** Whether row should be focused */ - isFocused?: boolean; + isOptionFocused?: boolean; /** List of users' personal details */ personalDetails?: PersonalDetailsList; @@ -109,6 +109,9 @@ type OptionRowLHNDataProps = { /** Whether to show the educational tooltip for the GBR or RBR */ shouldShowRBRorGBRTooltip: boolean; + + /** Whether the screen is focused */ + isScreenFocused?: boolean; }; type OptionRowLHNProps = { @@ -116,7 +119,7 @@ type OptionRowLHNProps = { reportID: string; /** Whether this option is currently in focus so we can modify its style */ - isFocused?: boolean; + isOptionFocused?: boolean; /** A function that is called when an option is selected. Selected option is passed as a param */ onSelectRow?: (optionItem: OptionData, popoverAnchor: RefObject) => void; @@ -137,6 +140,9 @@ type OptionRowLHNProps = { /** Whether to show the educational tooltip on the GBR or RBR */ shouldShowRBRorGBRTooltip: boolean; + + /** Whether the screen is focused */ + isScreenFocused?: boolean; }; type RenderItemProps = {item: Report}; diff --git a/tests/ui/components/LHNOptionsListTest.tsx b/tests/ui/components/LHNOptionsListTest.tsx new file mode 100644 index 000000000000..f4e9da0c3aa2 --- /dev/null +++ b/tests/ui/components/LHNOptionsListTest.tsx @@ -0,0 +1,174 @@ +import {NavigationContainer} from '@react-navigation/native'; +import type * as ReactNavigation from '@react-navigation/native'; +import {render, screen, userEvent, waitFor} from '@testing-library/react-native'; +import React from 'react'; +import Onyx from 'react-native-onyx'; +import ComposeProviders from '@components/ComposeProviders'; +import LHNOptionsList from '@components/LHNOptionsList/LHNOptionsList'; +import type {LHNOptionsListProps} from '@components/LHNOptionsList/types'; +import {LocaleContextProvider} from '@components/LocaleContextProvider'; +import OnyxProvider from '@components/OnyxProvider'; +import {showContextMenu} from '@pages/home/report/ContextMenu/ReportActionContextMenu'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import {getFakeReport} from '../../utils/LHNTestUtils'; + +// Mock the context menu +jest.mock('@pages/home/report/ContextMenu/ReportActionContextMenu', () => ({ + showContextMenu: jest.fn(), +})); + +// Mock the useRootNavigationState hook +jest.mock('@src/hooks/useRootNavigationState'); + +// Mock navigation hooks +const mockUseIsFocused = jest.fn().mockReturnValue(false); +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + useIsFocused: () => mockUseIsFocused(), + useRoute: jest.fn(), + useNavigationState: jest.fn(), + createNavigationContainerRef: () => ({ + getState: () => jest.fn(), + }), + }; +}); + +const getReportItem = (reportID: string) => { + return screen.findByTestId(reportID); +}; + +const getReportItemButton = () => { + return userEvent.setup(); +}; + +describe('LHNOptionsList', () => { + const mockReport = getFakeReport([1, 2], 0, false); + + const defaultProps: LHNOptionsListProps = { + data: [mockReport], + onSelectRow: jest.fn(), + optionMode: CONST.OPTION_MODE.DEFAULT, + onFirstItemRendered: jest.fn(), + }; + + const getLHNOptionsListElement = (props: Partial = {}) => { + const mergedProps = { + data: props.data ?? defaultProps.data, + onSelectRow: props.onSelectRow ?? defaultProps.onSelectRow, + optionMode: props.optionMode ?? defaultProps.optionMode, + onFirstItemRendered: props.onFirstItemRendered ?? defaultProps.onFirstItemRendered, + }; + + return ( + + + + + + ); + }; + + beforeEach(() => { + Onyx.init({ + keys: ONYXKEYS, + }); + + jest.clearAllMocks(); + }); + + afterEach(() => { + return Onyx.clear(); + }); + + it('shows context menu on long press', async () => { + // Given the screen is focused. + mockUseIsFocused.mockReturnValue(true); + + // Given the LHNOptionsList is rendered with a report. + render(getLHNOptionsListElement()); + + // Then wait for the report to be displayed in the LHNOptionsList. + const reportItem = await waitFor(() => getReportItem(mockReport.reportID)); + expect(reportItem).toBeTruthy(); + + // When the user long presses the report item. + const user = getReportItemButton(); + await user.longPress(reportItem); + + // Then wait for all state updates to complete and verify the context menu is shown + await waitFor(() => { + expect(showContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + type: CONST.CONTEXT_MENU_TYPES.REPORT, + report: expect.objectContaining({ + reportID: mockReport.reportID, + }), + }), + ); + }); + }); + + it('does not show context menu when screen is not focused', async () => { + // Given the screen is not focused. + mockUseIsFocused.mockReturnValue(false); + + // When the LHNOptionsList is rendered. + render(getLHNOptionsListElement()); + + // Then wait for the report to be displayed in the LHNOptionsList. + const reportItem = await waitFor(() => getReportItem(mockReport.reportID)); + expect(reportItem).toBeTruthy(); + + // When the user long presses the report item. + const user = getReportItemButton(); + await user.longPress(reportItem); + + // Then wait for all state updates to complete and verify the context menu is not shown + await waitFor(() => { + expect(showContextMenu).not.toHaveBeenCalled(); + }); + }); + + it('shows context menu after returning from chat', async () => { + // Given the screen is focused. + mockUseIsFocused.mockReturnValue(true); + + // When the LHNOptionsList is rendered. + const {rerender} = render(getLHNOptionsListElement()); + + // Then wait for the report to be displayed in the LHNOptionsList. + const reportItem = await waitFor(() => getReportItem(mockReport.reportID)); + expect(reportItem).toBeTruthy(); + + // When the user navigates to chat and back by re-rendering with different focus state + rerender(getLHNOptionsListElement()); + + // When the user re-renders again to simulate returning to the screen + rerender(getLHNOptionsListElement()); + + // When the user long presses the report item. + const user = getReportItemButton(); + await user.longPress(reportItem); + + // Then wait for all state updates to complete and verify the context menu is shown + await waitFor(() => { + expect(showContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + type: CONST.CONTEXT_MENU_TYPES.REPORT, + report: expect.objectContaining({ + reportID: mockReport.reportID, + }), + }), + ); + }); + }); +});