Skip to content

Commit

Permalink
Merge pull request #41898 from callstack-internal/hur/fix-memory-cons…
Browse files Browse the repository at this point in the history
…umption-on-web-and-desktop

perf: fix significant memory consumption on web and desktop
  • Loading branch information
danieldoglas authored May 10, 2024
2 parents 5730ba4 + 456fe95 commit 7fc1590
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 15 deletions.
18 changes: 11 additions & 7 deletions src/components/LHNOptionsList/OptionRowLHN.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,22 @@ function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, opti
}, []),
);

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) {
return null;
// rendering null as a render item causes the FlashList to render all
// its children and consume signficant memory. We can avoid this by
// rendering a placeholder view instead.
return <View style={sidebarInnerRowStyle} />;
}

const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.requiresAttentionFromCurrentUser(optionItem, optionItem.parentReportAction);
const isInFocusMode = viewMode === CONST.OPTION_MODE.COMPACT;
const textStyle = isFocused ? styles.sidebarLinkActiveText : styles.sidebarLinkText;
const textUnreadStyle = optionItem?.isUnread && optionItem.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE ? [textStyle, styles.sidebarLinkTextBold] : [textStyle];
const displayNameStyle = [styles.optionDisplayName, styles.optionDisplayNameCompact, styles.pre, textUnreadStyle, style];
Expand All @@ -66,11 +75,6 @@ function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, opti
: [textStyle, styles.optionAlternateText, styles.textLabelSupporting, style];

const contentContainerStyles = isInFocusMode ? [styles.flex1, styles.flexRow, styles.overflowHidden, StyleUtils.getCompactContentContainerStyles()] : [styles.flex1];
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],
);
const hoveredBackgroundColor = !!styles.sidebarLinkHover && 'backgroundColor' in styles.sidebarLinkHover ? styles.sidebarLinkHover.backgroundColor : theme.sidebar;
const focusedBackgroundColor = styles.sidebarLinkActive.backgroundColor;

Expand Down
11 changes: 3 additions & 8 deletions src/libs/Navigation/FreezeWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {useIsFocused, useNavigation, useRoute} from '@react-navigation/native';
import React, {useEffect, useRef, useState} from 'react';
import {Freeze} from 'react-freeze';
import type ChildrenProps from '@src/types/utils/ChildrenProps';
import shouldSetScreenBlurred from './shouldSetScreenBlurred';

type FreezeWrapperProps = ChildrenProps & {
/** Prop to disable freeze */
Expand All @@ -24,14 +25,8 @@ function FreezeWrapper({keepVisible = false, children}: FreezeWrapperProps) {

useEffect(() => {
const unsubscribe = navigation.addListener('state', () => {
// if the screen is more than 1 screen away from the current screen, freeze it,
// we don't want to freeze the screen if it's the previous screen because the freeze placeholder
// would be visible at the beginning of the back animation then
if ((navigation.getState()?.index ?? 0) - (screenIndexRef.current ?? 0) > 1) {
setIsScreenBlurred(true);
} else {
setIsScreenBlurred(false);
}
const navigationIndex = (navigation.getState()?.index ?? 0) - (screenIndexRef.current ?? 0);
setIsScreenBlurred(shouldSetScreenBlurred(navigationIndex));
});
return () => unsubscribe();
}, [isFocused, isScreenBlurred, navigation]);
Expand Down
12 changes: 12 additions & 0 deletions src/libs/Navigation/shouldSetScreenBlurred/index.native.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* @param navigationIndex
*
* Decides whether to set screen to blurred state.
*
* If the screen is more than 1 screen away from the current screen, freeze it,
* we don't want to freeze the screen if it's the previous screen because the freeze placeholder
* would be visible at the beginning of the back animation then
*/
const shouldSetScreenBlurred = (navigationIndex: number) => navigationIndex > 1;

export default shouldSetScreenBlurred;
13 changes: 13 additions & 0 deletions src/libs/Navigation/shouldSetScreenBlurred/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* @param navigationIndex
*
* Decides whether to set screen to blurred state.
*
* Allow freezing the first screen and more in the stack only on
* web and desktop platforms. The reason is that in the case of
* LHN, we have FlashList rendering in the back while we are on
* Settings screen.
*/
const shouldSetScreenBlurred = (navigationIndex: number) => navigationIndex >= 1;

export default shouldSetScreenBlurred;

0 comments on commit 7fc1590

Please sign in to comment.