Restore workspace list scroll position after navigation #86158
+135
−43
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import {useIsFocused, useRoute} from '@react-navigation/native'; | ||
| import {Str} from 'expensify-common'; | ||
| import React, {useCallback, useContext, useEffect, useLayoutEffect, useRef, useState} from 'react'; | ||
| import {FlatList, InteractionManager, View} from 'react-native'; | ||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
| import type {ValueOf} from 'type-fest'; | ||
|
|
@@ -19,6 +19,8 @@ import {usePersonalDetails} from '@components/OnyxListItemProvider'; | |
| import type {PopoverMenuItem} from '@components/PopoverMenu'; | ||
| import {PressableWithoutFeedback} from '@components/Pressable'; | ||
| import ScreenWrapper from '@components/ScreenWrapper'; | ||
| import {ScrollOffsetContext} from '@components/ScrollOffsetContextProvider'; | ||
| import type {ScrollViewProps} from '@components/ScrollView'; | ||
| import SearchBar from '@components/SearchBar'; | ||
| import type {ListItem} from '@components/SelectionList/types'; | ||
| import Text from '@components/Text'; | ||
|
|
@@ -40,6 +42,7 @@ import useSearchResults from '@hooks/useSearchResults'; | |
| import useTheme from '@hooks/useTheme'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import useTransactionViolationOfWorkspace from '@hooks/useTransactionViolationOfWorkspace'; | ||
| import useWindowDimensions from '@hooks/useWindowDimensions'; | ||
| import {isConnectionInProgress} from '@libs/actions/connections'; | ||
| import {close} from '@libs/actions/Modal'; | ||
| import {clearWorkspaceOwnerChangeFlow, isApprover as isApproverUserAction, requestWorkspaceOwnerChange} from '@libs/actions/Policy/Member'; | ||
|
|
@@ -68,6 +71,7 @@ import shouldRenderTransferOwnerButton from '@libs/shouldRenderTransferOwnerButt | |
| import {isSubscriptionTypeOfInvoicing, shouldCalculateBillNewDot as shouldCalculateBillNewDotFn} from '@libs/SubscriptionUtils'; | ||
| import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan'; | ||
| import type {AvatarSource} from '@libs/UserAvatarUtils'; | ||
| import variables from '@styles/variables'; | ||
| import {setNameValuePair} from '@userActions/User'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
|
|
@@ -132,6 +136,13 @@ function WorkspacesListPage() { | |
| const icons = useMemoizedLazyExpensifyIcons(['Building', 'Exit', 'Copy', 'Star', 'Trashcan', 'Transfer', 'FallbackWorkspaceAvatar']); | ||
| const theme = useTheme(); | ||
| const styles = useThemeStyles(); | ||
|
|
||
| // Fallback estimated height (in px) for a workspace row: avatar + vertical padding (styles.p5 top + bottom) + bottom margin (styles.mb2). | ||
| // Used to calculate initialNumToRender when no measured height is available yet. | ||
| const rowMarginBottom = (styles.mb2 as Partial<Record<'marginBottom', number>>)?.marginBottom ?? 0; | ||
| const rowPaddingVertical = (styles.p5 as Partial<Record<'padding', number>>)?.padding ?? 0; | ||
| const estimatedItemHeight = variables.avatarSizeNormal + rowPaddingVertical * 2 + rowMarginBottom; | ||
|
|
||
| const expensifyIcons = useMemoizedLazyExpensifyIcons(['Building', 'Exit', 'Copy', 'Star', 'Trashcan', 'Transfer', 'Plus', 'FallbackWorkspaceAvatar']); | ||
| const {translate, localeCompare} = useLocalize(); | ||
| useDocumentTitle(translate('common.workspaces')); | ||
|
|
@@ -187,13 +198,40 @@ function WorkspacesListPage() { | |
|
|
||
| const policyToDelete = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyIDToDelete}`]; | ||
|
|
||
| const {saveScrollOffset, getScrollOffset, saveAverageItemLength, getAverageItemLength} = useContext(ScrollOffsetContext); | ||
| const {windowHeight} = useWindowDimensions(); | ||
| const onScroll = useCallback<NonNullable<ScrollViewProps['onScroll']>>( | ||
| (e) => { | ||
| // If the layout measurement is 0, it means the list is not displayed but the onScroll may be triggered with offset value 0. | ||
| // We should ignore this case. | ||
| if (e.nativeEvent.layoutMeasurement.height === 0) { | ||
| return; | ||
| } | ||
| saveScrollOffset(route, e.nativeEvent.contentOffset.y); | ||
| }, | ||
| [route, saveScrollOffset], | ||
| ); | ||
|
|
||
| const flatlistRef = useRef<FlatList | null>(null); | ||
| const measuredItemHeight = useRef<number>(getAverageItemLength(route) ?? 0); | ||
|
|
||
| useLayoutEffect(() => { | ||
| const scrollOffset = getScrollOffset(route); | ||
| if (!scrollOffset || !flatlistRef.current) { | ||
| return; | ||
| } | ||
| flatlistRef.current.scrollToOffset({ | ||
| offset: scrollOffset, | ||
| animated: false, | ||
| }); | ||
| }, [getScrollOffset, route]); | ||
|
|
||
| // We need this to update translation for deleting a workspace when it has third party card feeds or expensify card assigned. | ||
| const workspaceAccountID = policyToDelete?.workspaceAccountID ?? CONST.DEFAULT_NUMBER_ID; | ||
| const [cardFeeds, , defaultCardFeeds] = useCardFeeds(policyIDToDelete); | ||
| const [cardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`, { | ||
| selector: filterInactiveCards, | ||
| }); | ||
| const [lastAccessedWorkspacePolicyID] = useOnyx(ONYXKEYS.LAST_ACCESSED_WORKSPACE_POLICY_ID); | ||
|
|
||
| const hasCardFeedOrExpensifyCard = | ||
|
|
@@ -436,48 +474,61 @@ function WorkspacesListPage() { | |
| } | ||
|
|
||
| return ( | ||
| <View | ||
| key={`${item.title}_${index}`} | ||
| onLayout={(e) => { | ||
| const height = e.nativeEvent.layout.height; | ||
| if (height <= 0) { | ||
| return; | ||
| } | ||
| if (height !== getAverageItemLength(route)) { | ||
| measuredItemHeight.current = height; | ||
| saveAverageItemLength(route, height); | ||
| } | ||
| }} | ||
| > | ||
| <OfflineWithFeedback | ||
| pendingAction={item.pendingAction} | ||
| errorRowStyles={[styles.ph5, styles.mt3]} | ||
| onClose={item.dismissError} | ||
| errors={item.errors} | ||
| style={styles.mb2} | ||
| shouldShowErrorMessages={item.policyID !== policyIDToDelete} | ||
| shouldHideOnDelete={false} | ||
| > | ||
| <PressableWithoutFeedback | ||
| accessible={false} | ||
| style={[styles.mh5]} | ||
| disabled={item.disabled} | ||
| onPress={item.action} | ||
| sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.WORKSPACE_MENU_ITEM} | ||
| > | ||
| {({hovered}) => ( | ||
| <WorkspacesListRow | ||
| title={item.title} | ||
| policyID={item.policyID} | ||
| menuItems={threeDotsMenuItems} | ||
| workspaceIcon={item.icon} | ||
| ownerAccountID={item.ownerAccountID} | ||
| workspaceType={item.type} | ||
| shouldAnimateInHighlight={shouldAnimateInHighlight} | ||
| isJoinRequestPending={item?.isJoinRequestPending} | ||
| rowStyles={hovered && styles.hoveredComponentBG} | ||
| layoutWidth={isLessThanMediumScreen ? CONST.LAYOUT_WIDTH.NARROW : CONST.LAYOUT_WIDTH.WIDE} | ||
| brickRoadIndicator={item.brickRoadIndicator} | ||
| shouldDisableThreeDotsMenu={item.disabled} | ||
| style={[item.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? styles.offlineFeedbackDeleted : {}]} | ||
| isDefault={isDefault} | ||
| isLoadingBill={isLoadingBill} | ||
| resetLoadingSpinnerIconIndex={resetLoadingSpinnerIconIndex} | ||
| isHovered={hovered} | ||
| disabled={item.disabled} | ||
| onPress={item.action} | ||
| /> | ||
| )} | ||
| </PressableWithoutFeedback> | ||
| </OfflineWithFeedback> | ||
| </View> | ||
| ); | ||
| }; | ||
|
|
||
|
|
@@ -683,6 +734,21 @@ function WorkspacesListPage() { | |
| shouldShowDomainsSection && !domains.length ? [{listItemType: 'domains-empty-state' as const}] : [], | ||
| ].flat(); | ||
|
|
||
| // Compute initialNumToRender: render enough items to cover the saved scroll offset so | ||
| // useLayoutEffect can restore position before first paint. Uses measured row height when | ||
| // available (persisted across navigations via ScrollOffsetContext), otherwise falls back to an estimate. | ||
| const savedScrollOffset = getScrollOffset(route) ?? 0; | ||
| const computedInitialNumToRender = (() => { | ||
| if (savedScrollOffset <= 0 || data.length === 0) { | ||
| return undefined; | ||
| } | ||
| // eslint-disable-next-line react-hooks/refs -- Reading the local measured height during render is intentional; the value is an optimization hint for initialNumToRender. | ||
| const localMeasured = measuredItemHeight.current > 0 ? measuredItemHeight.current : undefined; | ||
| const itemHeight = localMeasured ?? getAverageItemLength(route) ?? estimatedItemHeight; | ||
| const viewportItems = Math.ceil(windowHeight / itemHeight); | ||
| return Math.min(Math.ceil(savedScrollOffset / itemHeight) + viewportItems, data.length); | ||
|
Comment on lines
+748
to
+749
There was a problem hiding this comment.
The Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. Agree. Let's fix this.
Contributor
Author
There was a problem hiding this comment. Yes, The problem: The current How The tradeoff:
However, workspace rows dominate the list (the vast majority of items for high-traffic accounts), and the non-uniform items are at boundaries. Small inaccuracies from height mismatches are acceptable for scroll restoration — the user just needs to land near where they were, not pixel-perfect. Implementation sketch: const savedScrollIndex = savedScrollOffset > 0
? Math.round(savedScrollOffset / itemHeight)
: undefined;
// On the FlatList:
getItemLayout={(_, index) => ({
length: itemHeight,
offset: itemHeight * index,
index,
})}
initialScrollIndex={savedScrollIndex}This eliminates One caveat: Want me to implement this approach? |
||
| })(); | ||
|
|
||
| // eslint-disable-next-line react/no-unused-prop-types | ||
| const renderItem = ({item, index}: {item: WorkspaceOrDomainListItem; index: number}) => { | ||
| switch (item.listItemType) { | ||
|
|
@@ -765,6 +831,8 @@ function WorkspacesListPage() { | |
| ListHeaderComponent={listHeaderComponent} | ||
| keyboardShouldPersistTaps="handled" | ||
| contentContainerStyle={styles.pb20} | ||
| onScroll={onScroll} | ||
| initialNumToRender={computedInitialNumToRender} | ||
| /> | ||
| )} | ||
| </View> | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us keep the measuredItemHeight and averageItemLength within ScrollOffsetContextProvider in sync by assigning
heighttomeasuredItemHeight.currenthere.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done — moved the ref assignment inside the conditional block so both the local ref and context value are updated together, keeping them in sync.