Skip to content

perf: move screen focus to LHNOptionsList #63531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mountiny merged 1 commit into from
Jun 12, 2025
Merged

perf: move screen focus to LHNOptionsList #63531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -42,6 +42,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
const {isOffline} = useNetwork();
const flashListRef = useRef<FlashList<Report>>(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});
Expand Down Expand Up @@ -225,14 +226,15 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
lastReportActionTransaction={lastReportActionTransaction}
receiptTransactions={transactions}
viewMode={optionMode}
isFocused={!shouldDisableFocusOptions}
isOptionFocused={!shouldDisableFocusOptions}
lastMessageTextFromReport={lastMessageTextFromReport}
onSelectRow={onSelectRow}
preferredLocale={preferredLocale}
hasDraftComment={hasDraftComment}
transactionViolations={transactionViolations}
onLayout={onLayoutItem}
shouldShowRBRorGBRTooltip={shouldShowRBRorGBRTooltip}
isScreenFocused={isScreenFocused}
/>
);
},
Expand All @@ -253,6 +255,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
onLayoutItem,
isOffline,
firstReportIDWithGBRorRBR,
isScreenFocused,
],
);

Expand All @@ -271,6 +274,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
preferredLocale,
transactions,
isOffline,
isScreenFocused,
],
[
reportActions,
Expand All @@ -286,6 +290,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
preferredLocale,
transactions,
isOffline,
isScreenFocused,
],
);

Expand Down
32 changes: 11 additions & 21 deletions src/components/LHNOptionsList/OptionRowLHN.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -51,20 +50,20 @@ import type {OptionRowLHNProps} from './types';

function OptionRowLHN({
reportID,
isFocused = false,
isOptionFocused = false,
onSelectRow = () => {},
optionItem,
viewMode = 'default',
style,
onLayout = () => {},
hasDraftComment,
shouldShowRBRorGBRTooltip,
isScreenFocused = false,
}: OptionRowLHNProps) {
const theme = useTheme();
const styles = useThemeStyles();
const popoverAnchor = useRef<View>(null);
const StyleUtils = useStyleUtils();
const [isScreenFocused, setIsScreenFocused] = useState(false);
const {shouldUseNarrowLayout} = useResponsiveLayout();

const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${optionItem?.reportID}`, {canBeMissing: true});
Expand Down Expand Up @@ -105,23 +104,14 @@ 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<ViewStyle>(
isInFocusMode
? [styles.chatLinkRowPressable, styles.flexGrow1, styles.optionItemAvatarNameWrapper, styles.optionRowCompact, styles.justifyContentCenter]
: [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
Expand All @@ -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
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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')}.` : ''} ${
Expand All @@ -272,7 +262,7 @@ function OptionRowLHN({
firstIcon &&
(optionItem.shouldShowSubscript ? (
<SubscriptAvatar
backgroundColor={hovered && !isFocused ? hoveredBackgroundColor : subscriptAvatarBorderColor}
backgroundColor={hovered && !isOptionFocused ? hoveredBackgroundColor : subscriptAvatarBorderColor}
mainAvatar={firstIcon}
secondaryAvatar={optionItem.icons.at(1)}
size={isInFocusMode ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT}
Expand All @@ -284,8 +274,8 @@ function OptionRowLHN({
size={isInFocusMode ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT}
secondAvatarStyle={[
StyleUtils.getBackgroundAndBorderStyle(theme.sidebar),
isFocused ? StyleUtils.getBackgroundAndBorderStyle(focusedBackgroundColor) : undefined,
hovered && !isFocused ? StyleUtils.getBackgroundAndBorderStyle(hoveredBackgroundColor) : undefined,
isOptionFocused ? StyleUtils.getBackgroundAndBorderStyle(focusedBackgroundColor) : undefined,
hovered && !isOptionFocused ? StyleUtils.getBackgroundAndBorderStyle(hoveredBackgroundColor) : undefined,
]}
shouldShowTooltip={shouldOptionShowTooltip(optionItem)}
/>
Expand Down
6 changes: 3 additions & 3 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<OptionData | undefined>(undefined);
const optionItem = useMemo(() => {
Expand Down Expand Up @@ -88,7 +88,7 @@ function OptionRowLHNData({
<OptionRowLHN
// eslint-disable-next-line react/jsx-props-no-spreading
{...propsToForward}
isFocused={isReportFocused}
isOptionFocused={isReportFocused}
optionItem={optionItem}
/>
);
Expand Down
10 changes: 8 additions & 2 deletions src/components/LHNOptionsList/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -109,14 +109,17 @@ type OptionRowLHNDataProps = {

/** Whether to show the educational tooltip for the GBR or RBR */
shouldShowRBRorGBRTooltip: boolean;

/** Whether the screen is focused */
isScreenFocused?: boolean;
};

type OptionRowLHNProps = {
/** The ID of the report that the option is for */
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<View>) => void;
Expand All @@ -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};
Expand Down
174 changes: 174 additions & 0 deletions tests/ui/components/LHNOptionsListTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<typeof ReactNavigation>('@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<LHNOptionsListProps> = {}) => {
const mergedProps = {
data: props.data ?? defaultProps.data,
onSelectRow: props.onSelectRow ?? defaultProps.onSelectRow,
optionMode: props.optionMode ?? defaultProps.optionMode,
onFirstItemRendered: props.onFirstItemRendered ?? defaultProps.onFirstItemRendered,
};

return (
<NavigationContainer>
<ComposeProviders components={[OnyxProvider, LocaleContextProvider]}>
<LHNOptionsList
data={mergedProps.data}
onSelectRow={mergedProps.onSelectRow}
optionMode={mergedProps.optionMode}
onFirstItemRendered={mergedProps.onFirstItemRendered}
/>
</ComposeProviders>
</NavigationContainer>
);
};

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,
}),
}),
);
});
});
});