Skip to content
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.

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

Fix LHN flickers on web #56472

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 13 additions & 5 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import usePrevious from '@hooks/usePrevious';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import {isValidDraftComment} from '@libs/DraftCommentUtils';
import getPlatform from '@libs/getPlatform';
import {getIOUReportIDOfLastAction, getLastMessageTextForReport} from '@libs/OptionsListUtils';
import {getOriginalMessage, getSortedReportActionsForDisplay, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {canUserPerformWriteAction} from '@libs/ReportUtils';
Expand All @@ -31,7 +32,7 @@ import type {LHNOptionsListProps, RenderItemProps} from './types';
const keyExtractor = (item: string) => `report_${item}`;

function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optionMode, shouldDisableFocusOptions = false, onFirstItemRendered = () => {}}: LHNOptionsListProps) {
const {saveScrollOffset, getScrollOffset} = useContext(ScrollOffsetContext);
const {saveScrollOffset, getScrollOffset, saveScrollIndex, getScrollIndex} = useContext(ScrollOffsetContext);
const flashListRef = useRef<FlashList<string>>(null);
const route = useRoute();

Expand All @@ -49,6 +50,9 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
const {translate, preferredLocale} = useLocalize();
const estimatedListSize = useLHNEstimatedListSize();
const shouldShowEmptyLHN = data.length === 0;
const estimatedItemSize = optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight;
const platform = getPlatform();
const isWebOrDesktop = platform === CONST.PLATFORM.WEB || platform === CONST.PLATFORM.DESKTOP;

// When the first item renders we want to call the onFirstItemRendered callback.
// At this point in time we know that the list is actually displaying items.
Expand Down Expand Up @@ -237,14 +241,17 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
return;
}
saveScrollOffset(route, e.nativeEvent.contentOffset.y);
if (isWebOrDesktop) {
saveScrollIndex(route, Math.floor(e.nativeEvent.contentOffset.y / estimatedItemSize));
}
},
[route, saveScrollOffset],
[estimatedItemSize, isWebOrDesktop, route, saveScrollIndex, saveScrollOffset],
);

const onLayout = useCallback(() => {
const offset = getScrollOffset(route);

if (!(offset && flashListRef.current)) {
if (!(offset && flashListRef.current) || isWebOrDesktop) {
return;
}

Expand All @@ -255,7 +262,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
}
flashListRef.current.scrollToOffset({offset});
});
}, [route, flashListRef, getScrollOffset]);
}, [getScrollOffset, route, isWebOrDesktop]);

return (
<View style={[style ?? styles.flex1, shouldShowEmptyLHN ? styles.emptyLHNWrapper : undefined]}>
Expand All @@ -279,12 +286,13 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
testID="lhn-options-list"
keyExtractor={keyExtractor}
renderItem={renderItem}
estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
estimatedItemSize={estimatedItemSize}
extraData={extraData}
showsVerticalScrollIndicator={false}
onLayout={onLayout}
onScroll={onScroll}
estimatedListSize={estimatedListSize}
initialScrollIndex={isWebOrDesktop ? getScrollIndex(route) : undefined}
/>
)}
</View>
Expand Down
42 changes: 27 additions & 15 deletions src/components/ScrollOffsetContextProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import type {ParamListBase} from '@react-navigation/native';
import React, {createContext, useCallback, useEffect, useMemo, useRef} from 'react';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import usePrevious from '@hooks/usePrevious';
import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types';
import type {NavigationPartialRoute, State} from '@libs/Navigation/types';
import NAVIGATORS from '@src/NAVIGATORS';
import ONYXKEYS from '@src/ONYXKEYS';
import SCREENS from '@src/SCREENS';
import type {PriorityMode} from '@src/types/onyx';

type ScrollOffsetContextValue = {
/** Save scroll offset of flashlist on given screen */
Expand All @@ -16,23 +15,26 @@ type ScrollOffsetContextValue = {
/** Get scroll offset value for given screen */
getScrollOffset: (route: PlatformStackRouteProp<ParamListBase>) => number | undefined;

/** Save scroll index of flashlist on given screen */
saveScrollIndex: (route: PlatformStackRouteProp<ParamListBase>, scrollIndex: number) => void;

/** Get scroll index value for given screen */
getScrollIndex: (route: PlatformStackRouteProp<ParamListBase>) => number | undefined;

/** Clean scroll offsets of screen that aren't anymore in the state */
cleanStaleScrollOffsets: (state: State) => void;
};

type ScrollOffsetContextProviderOnyxProps = {
/** The chat priority mode */
priorityMode: PriorityMode;
};

type ScrollOffsetContextProviderProps = ScrollOffsetContextProviderOnyxProps & {
type ScrollOffsetContextProviderProps = {
/** Actual content wrapped by this component */
children: React.ReactNode;
};

const defaultValue: ScrollOffsetContextValue = {
saveScrollOffset: () => {},
getScrollOffset: () => undefined,
saveScrollIndex: () => {},
getScrollIndex: () => undefined,
cleanStaleScrollOffsets: () => {},
};

Expand All @@ -46,7 +48,8 @@ function getKey(route: PlatformStackRouteProp<ParamListBase> | NavigationPartial
return `${route.name}-global`;
}

function ScrollOffsetContextProvider({children, priorityMode}: ScrollOffsetContextProviderProps) {
function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProps) {
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE);
const scrollOffsetsRef = useRef<Record<string, number>>({});
const previousPriorityMode = usePrevious(priorityMode);

Expand Down Expand Up @@ -87,23 +90,32 @@ function ScrollOffsetContextProvider({children, priorityMode}: ScrollOffsetConte
}
}, []);

const saveScrollIndex: ScrollOffsetContextValue['saveScrollIndex'] = useCallback((route, scrollIndex) => {
scrollOffsetsRef.current[getKey(route)] = scrollIndex;
}, []);

const getScrollIndex: ScrollOffsetContextValue['getScrollIndex'] = useCallback((route) => {
if (!scrollOffsetsRef.current) {
return;
}
return scrollOffsetsRef.current[getKey(route)];
}, []);

const contextValue = useMemo(
(): ScrollOffsetContextValue => ({
saveScrollOffset,
getScrollOffset,
cleanStaleScrollOffsets,
saveScrollIndex,
getScrollIndex,
}),
[saveScrollOffset, getScrollOffset, cleanStaleScrollOffsets],
[saveScrollOffset, getScrollOffset, cleanStaleScrollOffsets, saveScrollIndex, getScrollIndex],
);

return <ScrollOffsetContext.Provider value={contextValue}>{children}</ScrollOffsetContext.Provider>;
}

export default withOnyx<ScrollOffsetContextProviderProps, ScrollOffsetContextProviderOnyxProps>({
priorityMode: {
key: ONYXKEYS.NVP_PRIORITY_MODE,
},
})(ScrollOffsetContextProvider);
export default ScrollOffsetContextProvider;

export {ScrollOffsetContext};

Expand Down