From f9f1088cb5577a4dbacdf38f0c0326bea7db7e89 Mon Sep 17 00:00:00 2001 From: Huu Le <20178761+huult@users.noreply.github.com> Date: Thu, 14 Aug 2025 10:30:33 +0700 Subject: [PATCH 1/4] Fix back button on report detail page resetting scroll position on reports list --- .../ScrollOffsetContextProvider.tsx | 30 +++++++++++--- src/components/Search/SearchList.tsx | 41 +++++++++++++++++-- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/components/ScrollOffsetContextProvider.tsx b/src/components/ScrollOffsetContextProvider.tsx index 8298e5930613..e04b59ac82c7 100644 --- a/src/components/ScrollOffsetContextProvider.tsx +++ b/src/components/ScrollOffsetContextProvider.tsx @@ -40,11 +40,26 @@ const defaultValue: ScrollOffsetContextValue = { const ScrollOffsetContext = createContext(defaultValue); -/** This function is prepared to work with HOME screens. May need modification if we want to handle other types of screens. */ +/** This function is prepared to work with HOME and SEARCH screens. */ function getKey(route: PlatformStackRouteProp | NavigationPartialRoute): string { + // Handle routes with direct policyID parameter (HOME screens) if (route.params && 'policyID' in route.params && typeof route.params.policyID === 'string') { return `${route.name}-${route.params.policyID}`; } + + // Handle SEARCH.ROOT screens where policyID might be embedded in the search query + if (route.name === SCREENS.SEARCH.ROOT && route.params && 'q' in route.params && typeof route.params.q === 'string') { + const policyIDMatch = route.params.q.match(/policyID:([^\\s]+)/); + if (policyIDMatch) { + return `${route.name}-${policyIDMatch[1]}`; + } + } + + // Handle other SEARCH screens with reportID parameter + if (route.params && 'reportID' in route.params && typeof route.params.reportID === 'string') { + return `${route.name}-${route.params.reportID}`; + } + return `${route.name}-global`; } @@ -58,9 +73,9 @@ function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProp return; } - // If the priority mode changes, we need to clear the scroll offsets for the home screens because it affects the size of the elements and scroll positions wouldn't be correct. + // If the priority mode changes, we need to clear the scroll offsets for the home and search screens because it affects the size of the elements and scroll positions wouldn't be correct. for (const key of Object.keys(scrollOffsetsRef.current)) { - if (key.includes(SCREENS.HOME)) { + if (key.includes(SCREENS.HOME) || key.includes(SCREENS.SEARCH.ROOT)) { delete scrollOffsetsRef.current[key]; } } @@ -79,9 +94,14 @@ function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProp const cleanStaleScrollOffsets: ScrollOffsetContextValue['cleanStaleScrollOffsets'] = useCallback((state) => { const sidebarRoutes = state.routes.filter((route) => isSidebarScreenName(route.name)); - const scrollOffsetKeysOfExistingScreens = sidebarRoutes.map((route) => getKey(route)); + + const searchRoutes = state.routes.filter((route) => route.name === SCREENS.SEARCH.ROOT); + const allRelevantRoutes = [...sidebarRoutes, ...searchRoutes]; + const scrollOffsetKeysOfExistingScreens = allRelevantRoutes.map((route) => getKey(route)); + for (const key of Object.keys(scrollOffsetsRef.current)) { - if (!scrollOffsetKeysOfExistingScreens.includes(key)) { + const isSearchRoute = key.startsWith('Search_Root-'); + if (!isSearchRoute && !scrollOffsetKeysOfExistingScreens.includes(key)) { delete scrollOffsetsRef.current[key]; } } diff --git a/src/components/Search/SearchList.tsx b/src/components/Search/SearchList.tsx index 314924d100cc..7c89bb3dc4be 100644 --- a/src/components/Search/SearchList.tsx +++ b/src/components/Search/SearchList.tsx @@ -1,7 +1,7 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useIsFocused, useRoute} from '@react-navigation/native'; import {FlashList} from '@shopify/flash-list'; import type {FlashListProps, ViewToken} from '@shopify/flash-list'; -import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react'; +import React, {forwardRef, useCallback, useContext, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react'; import type {ForwardedRef} from 'react'; import {View} from 'react-native'; import type {NativeSyntheticEvent, StyleProp, ViewStyle} from 'react-native'; @@ -12,6 +12,7 @@ import MenuItem from '@components/MenuItem'; import Modal from '@components/Modal'; import {usePersonalDetails} from '@components/OnyxListItemProvider'; import {PressableWithFeedback} from '@components/Pressable'; +import {ScrollOffsetContext} from '@components/ScrollOffsetContextProvider'; import type ChatListItem from '@components/SelectionList/ChatListItem'; import type TaskListItem from '@components/SelectionList/Search/TaskListItem'; import type TransactionGroupListItem from '@components/SelectionList/Search/TransactionGroupListItem'; @@ -182,6 +183,9 @@ function SearchList( const [isUserValidated] = useOnyx(ONYXKEYS.ACCOUNT, {selector: (account) => account?.validated, canBeMissing: true}); const [userBillingFundID] = useOnyx(ONYXKEYS.NVP_BILLING_FUND_ID, {canBeMissing: true}); + const route = useRoute(); + const {saveScrollOffset, getScrollOffset} = useContext(ScrollOffsetContext); + const handleLongPressRow = useCallback( (item: SearchListItem) => { // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing @@ -420,6 +424,35 @@ function SearchList( }; }, [calculatedListHeight, calculatedListWidth]); + const handleScroll = useCallback['onScroll']>>( + (e) => { + onScroll(e); + + if (e.nativeEvent.layoutMeasurement.height > 0) { + saveScrollOffset(route, e.nativeEvent.contentOffset.y); + } + }, + [onScroll, route, saveScrollOffset], + ); + + const handleLayout = useCallback(() => { + onLayout?.(); + + const offset = getScrollOffset(route); + if (!offset || !listRef.current) { + return; + } + + // Use requestAnimationFrame to ensure proper scrolling on iOS + requestAnimationFrame(() => { + if (!offset || !listRef.current) { + return; + } + + listRef.current.scrollToOffset({offset}); + }); + }, [onLayout, getScrollOffset, route]); + return ( {tableHeaderVisible && ( @@ -457,7 +490,7 @@ function SearchList( data={data} renderItem={renderItem} keyExtractor={keyExtractor} - onScroll={onScroll} + onScroll={handleScroll} showsVerticalScrollIndicator={false} ref={listRef} extraData={[focusedIndex, isFocused]} @@ -465,7 +498,7 @@ function SearchList( onEndReachedThreshold={onEndReachedThreshold} ListFooterComponent={ListFooterComponent} onViewableItemsChanged={onViewableItemsChanged} - onLayout={onLayout} + onLayout={handleLayout} removeClippedSubviews drawDistance={1000} estimatedItemSize={estimatedItemSize} From 82ee08e29d7b3cd61d89aae03d213b7b032a3d13 Mon Sep 17 00:00:00 2001 From: Huu Le <20178761+huult@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:43:23 +0700 Subject: [PATCH 2/4] Update scroll offset in context provider function --- .../ScrollOffsetContextProvider.tsx | 64 +++++++++++++------ 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/src/components/ScrollOffsetContextProvider.tsx b/src/components/ScrollOffsetContextProvider.tsx index e04b59ac82c7..30c804967797 100644 --- a/src/components/ScrollOffsetContextProvider.tsx +++ b/src/components/ScrollOffsetContextProvider.tsx @@ -1,4 +1,5 @@ import type {ParamListBase} from '@react-navigation/native'; +import {findFocusedRoute} from '@react-navigation/native'; import React, {createContext, useCallback, useEffect, useMemo, useRef} from 'react'; import useOnyx from '@hooks/useOnyx'; import usePrevious from '@hooks/usePrevious'; @@ -47,19 +48,14 @@ function getKey(route: PlatformStackRouteProp | NavigationPartial return `${route.name}-${route.params.policyID}`; } - // Handle SEARCH.ROOT screens where policyID might be embedded in the search query + // Handle SEARCH screens with query parameters if (route.name === SCREENS.SEARCH.ROOT && route.params && 'q' in route.params && typeof route.params.q === 'string') { - const policyIDMatch = route.params.q.match(/policyID:([^\\s]+)/); - if (policyIDMatch) { - return `${route.name}-${policyIDMatch[1]}`; - } - } - - // Handle other SEARCH screens with reportID parameter - if (route.params && 'reportID' in route.params && typeof route.params.reportID === 'string') { - return `${route.name}-${route.params.reportID}`; + // Encode the query to handle spaces and special characters + const encodedQuery = encodeURIComponent(route.params.q); + return `${route.name}-${encodedQuery}`; } + // For other routes, just use route name return `${route.name}-global`; } @@ -92,21 +88,50 @@ function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProp return scrollOffsetsRef.current[getKey(route)]; }, []); - const cleanStaleScrollOffsets: ScrollOffsetContextValue['cleanStaleScrollOffsets'] = useCallback((state) => { - const sidebarRoutes = state.routes.filter((route) => isSidebarScreenName(route.name)); + const cleanSearchScreenOffsets = useCallback((keys: string[], currentKey: string | null, isSearchMoneyRequestReport: boolean) => { + keys.forEach((key) => { + const shouldDeleteKey = key.startsWith(SCREENS.SEARCH.ROOT) && key !== currentKey && !isSearchMoneyRequestReport; - const searchRoutes = state.routes.filter((route) => route.name === SCREENS.SEARCH.ROOT); - const allRelevantRoutes = [...sidebarRoutes, ...searchRoutes]; - const scrollOffsetKeysOfExistingScreens = allRelevantRoutes.map((route) => getKey(route)); + if (shouldDeleteKey) { + delete scrollOffsetsRef.current[key]; + } + }); + }, []); - for (const key of Object.keys(scrollOffsetsRef.current)) { - const isSearchRoute = key.startsWith('Search_Root-'); - if (!isSearchRoute && !scrollOffsetKeysOfExistingScreens.includes(key)) { + const cleanNonSearchScreenOffsets = useCallback((keys: string[], existingScreenKeys: string[]) => { + keys.forEach((key) => { + const shouldDeleteKey = !key.startsWith(SCREENS.SEARCH.ROOT) && !existingScreenKeys.includes(key); + + if (shouldDeleteKey) { delete scrollOffsetsRef.current[key]; } - } + }); }, []); + const cleanStaleScrollOffsets: ScrollOffsetContextValue['cleanStaleScrollOffsets'] = useCallback( + (state) => { + const sidebarRoutes = state.routes.filter((route) => isSidebarScreenName(route.name)); + const existingScreenKeys = sidebarRoutes.map((route) => getKey(route)); + + const focusedRoute = findFocusedRoute(state); + const routeName = focusedRoute?.name; + + const isSearchScreen = routeName === SCREENS.SEARCH.ROOT; + const isSearchMoneyRequestReport = routeName === SCREENS.SEARCH.MONEY_REQUEST_REPORT || routeName === SCREENS.SEARCH.REPORT_RHP; + + const scrollOffsetKeys = Object.keys(scrollOffsetsRef.current); + + if (isSearchScreen) { + const currentKey = focusedRoute ? getKey(focusedRoute) : null; + cleanSearchScreenOffsets(scrollOffsetKeys, currentKey, isSearchMoneyRequestReport); + return; + } + + cleanNonSearchScreenOffsets(scrollOffsetKeys, existingScreenKeys); + }, + [cleanSearchScreenOffsets, cleanNonSearchScreenOffsets], + ); + const saveScrollIndex: ScrollOffsetContextValue['saveScrollIndex'] = useCallback((route, scrollIndex) => { scrollOffsetsRef.current[getKey(route)] = scrollIndex; }, []); @@ -115,6 +140,7 @@ function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProp if (!scrollOffsetsRef.current) { return; } + return scrollOffsetsRef.current[getKey(route)]; }, []); From 0920622f0dbcb312ffc826b7ee16e4c9035fb8f0 Mon Sep 17 00:00:00 2001 From: Huu Le <20178761+huult@users.noreply.github.com> Date: Thu, 14 Aug 2025 12:51:37 +0700 Subject: [PATCH 3/4] Refactor cleanStaleScrollOffsets function for clarity --- .../ScrollOffsetContextProvider.tsx | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/components/ScrollOffsetContextProvider.tsx b/src/components/ScrollOffsetContextProvider.tsx index 30c804967797..fb37d2f57c3e 100644 --- a/src/components/ScrollOffsetContextProvider.tsx +++ b/src/components/ScrollOffsetContextProvider.tsx @@ -88,30 +88,20 @@ function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProp return scrollOffsetsRef.current[getKey(route)]; }, []); - const cleanSearchScreenOffsets = useCallback((keys: string[], currentKey: string | null, isSearchMoneyRequestReport: boolean) => { + const cleanScrollOffsets = useCallback((keys: string[], shouldDelete: (key: string) => boolean) => { keys.forEach((key) => { - const shouldDeleteKey = key.startsWith(SCREENS.SEARCH.ROOT) && key !== currentKey && !isSearchMoneyRequestReport; - - if (shouldDeleteKey) { - delete scrollOffsetsRef.current[key]; + if (!shouldDelete(key)) { + return; } - }); - }, []); - const cleanNonSearchScreenOffsets = useCallback((keys: string[], existingScreenKeys: string[]) => { - keys.forEach((key) => { - const shouldDeleteKey = !key.startsWith(SCREENS.SEARCH.ROOT) && !existingScreenKeys.includes(key); - - if (shouldDeleteKey) { - delete scrollOffsetsRef.current[key]; - } + delete scrollOffsetsRef.current[key]; }); }, []); const cleanStaleScrollOffsets: ScrollOffsetContextValue['cleanStaleScrollOffsets'] = useCallback( (state) => { const sidebarRoutes = state.routes.filter((route) => isSidebarScreenName(route.name)); - const existingScreenKeys = sidebarRoutes.map((route) => getKey(route)); + const existingScreenKeys = sidebarRoutes.map(getKey); const focusedRoute = findFocusedRoute(state); const routeName = focusedRoute?.name; @@ -121,15 +111,14 @@ function ScrollOffsetContextProvider({children}: ScrollOffsetContextProviderProp const scrollOffsetKeys = Object.keys(scrollOffsetsRef.current); - if (isSearchScreen) { + if (isSearchScreen || isSearchMoneyRequestReport) { const currentKey = focusedRoute ? getKey(focusedRoute) : null; - cleanSearchScreenOffsets(scrollOffsetKeys, currentKey, isSearchMoneyRequestReport); + cleanScrollOffsets(scrollOffsetKeys, (key) => key.startsWith(SCREENS.SEARCH.ROOT) && key !== currentKey && !isSearchMoneyRequestReport); return; } - - cleanNonSearchScreenOffsets(scrollOffsetKeys, existingScreenKeys); + cleanScrollOffsets(scrollOffsetKeys, (key) => !existingScreenKeys.includes(key)); }, - [cleanSearchScreenOffsets, cleanNonSearchScreenOffsets], + [cleanScrollOffsets], ); const saveScrollIndex: ScrollOffsetContextValue['saveScrollIndex'] = useCallback((route, scrollIndex) => { From c4ced9587931478b8e12051d551a8ee99e1062ff Mon Sep 17 00:00:00 2001 From: Huu Le <20178761+huult@users.noreply.github.com> Date: Thu, 14 Aug 2025 13:09:47 +0700 Subject: [PATCH 4/4] Make prop handling safer --- src/components/Search/SearchList.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/Search/SearchList.tsx b/src/components/Search/SearchList.tsx index 7c89bb3dc4be..1ecd72cdb6d0 100644 --- a/src/components/Search/SearchList.tsx +++ b/src/components/Search/SearchList.tsx @@ -426,7 +426,9 @@ function SearchList( const handleScroll = useCallback['onScroll']>>( (e) => { - onScroll(e); + if (onScroll && typeof onScroll === 'function') { + onScroll(e); + } if (e.nativeEvent.layoutMeasurement.height > 0) { saveScrollOffset(route, e.nativeEvent.contentOffset.y); @@ -436,7 +438,9 @@ function SearchList( ); const handleLayout = useCallback(() => { - onLayout?.(); + if (onLayout && typeof onLayout === 'function') { + onLayout(); + } const offset = getScrollOffset(route); if (!offset || !listRef.current) {