From 00a7bedf2c987059770e45be00a49af529c1ad30 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Thu, 1 Aug 2024 07:08:49 +0700 Subject: [PATCH 01/13] Fix the new message button doesn't work for some comment linkings Signed-off-by: Tsaqif --- src/pages/home/report/ReportActionsView.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index a3ae69be722e..4cf980189e17 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -103,6 +103,7 @@ function ReportActionsView({ const reactionListRef = useContext(ReactionListContext); const route = useRoute>(); const reportActionID = route?.params?.reportActionID; + const prevReportActionID = usePrevious(reportActionID); const didLayout = useRef(false); const didLoadOlderChats = useRef(false); const didLoadNewerChats = useRef(false); @@ -146,7 +147,7 @@ function ReportActionsView({ // Change the list ID only for comment linking to get the positioning right const listID = useMemo(() => { - if (!reportActionID) { + if (!reportActionID && !prevReportActionID) { // Keep the old list ID since we're not in the Comment Linking flow return listOldID; } From 498791719b6100a8a11ffc6cbb2c3110dde45c75 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Thu, 1 Aug 2024 20:20:18 +0700 Subject: [PATCH 02/13] Fix pagination util prev page Signed-off-by: Tsaqif --- src/libs/PaginationUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/PaginationUtils.ts b/src/libs/PaginationUtils.ts index fe75b6fb9927..84a39933342e 100644 --- a/src/libs/PaginationUtils.ts +++ b/src/libs/PaginationUtils.ts @@ -124,7 +124,7 @@ function mergeAndSortContinuousPages(sortedItems: TResource[], pages: const result = [sortedPages[0]]; for (let i = 1; i < sortedPages.length; i++) { const page = sortedPages[i]; - const prevPage = sortedPages[i - 1]; + const prevPage = result.at(-1); // Current page is inside the previous page, skip if (page.lastIndex <= prevPage.lastIndex && page.lastID !== CONST.PAGINATION_END_ID) { From 76f948986eccf3e3561e33c730cd2f951cee2a72 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Fri, 2 Aug 2024 10:10:59 +0700 Subject: [PATCH 03/13] Using one source of route for report screen, view and list Signed-off-by: Tsaqif --- src/libs/PaginationUtils.ts | 2 +- src/pages/home/report/ReportActionsView.tsx | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/libs/PaginationUtils.ts b/src/libs/PaginationUtils.ts index 84a39933342e..d1c096e99a8f 100644 --- a/src/libs/PaginationUtils.ts +++ b/src/libs/PaginationUtils.ts @@ -124,7 +124,7 @@ function mergeAndSortContinuousPages(sortedItems: TResource[], pages: const result = [sortedPages[0]]; for (let i = 1; i < sortedPages.length; i++) { const page = sortedPages[i]; - const prevPage = result.at(-1); + const prevPage = result?.at(-1) ?? sortedPages[i - 1]; // Current page is inside the previous page, skip if (page.lastIndex <= prevPage.lastIndex && page.lastID !== CONST.PAGINATION_END_ID) { diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index 4cf980189e17..b76c6bd15a42 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -85,6 +85,8 @@ const SPACER = 16; let listOldID = Math.round(Math.random() * 100); function ReportActionsView({ + reportIDFromRoute: reportID, + reportActionIDFromRoute: reportActionID, report, transactionThreadReport, session, @@ -102,8 +104,8 @@ function ReportActionsView({ useCopySelectionHelper(); const reactionListRef = useContext(ReactionListContext); const route = useRoute>(); - const reportActionID = route?.params?.reportActionID; const prevReportActionID = usePrevious(reportActionID); + const prevReportID = usePrevious(reportID); const didLayout = useRef(false); const didLoadOlderChats = useRef(false); const didLoadNewerChats = useRef(false); @@ -122,7 +124,6 @@ function ReportActionsView({ const prevAuthTokenType = usePrevious(session?.authTokenType); const [isNavigatingToLinkedMessage, setNavigatingToLinkedMessage] = useState(!!reportActionID); const prevShouldUseNarrowLayoutRef = useRef(shouldUseNarrowLayout); - const reportID = report.reportID; const isLoading = (!!reportActionID && isLoadingInitialReportActions) || !isReadyForCommentLinking; const isReportFullyVisible = useMemo((): boolean => getIsReportFullyVisible(isFocused), [isFocused]); const openReportIfNecessary = () => { @@ -147,7 +148,7 @@ function ReportActionsView({ // Change the list ID only for comment linking to get the positioning right const listID = useMemo(() => { - if (!reportActionID && !prevReportActionID) { + if (!reportActionID && !prevReportActionID && (prevReportID === reportID)) { // Keep the old list ID since we're not in the Comment Linking flow return listOldID; } @@ -157,7 +158,7 @@ function ReportActionsView({ listOldID = newID; return newID; // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [route, isLoadingInitialReportActions, reportActionID]); + }, [reportID, reportActionID]); // When we are offline before opening an IOU/Expense report, // the total of the report and sometimes the expense aren't displayed because these actions aren't returned until `OpenReport` API is complete. @@ -239,7 +240,7 @@ function ReportActionsView({ if (!reportActionID) { return -1; } - return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(isFirstLinkedActionRender.current ? reportActionID : currentReportActionID)); + return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionID)); }, [combinedReportActions, currentReportActionID, reportActionID]); const reportActions = useMemo(() => { @@ -494,6 +495,7 @@ function ReportActionsView({ Date: Fri, 2 Aug 2024 21:01:30 +0700 Subject: [PATCH 04/13] using one source of route in report screen, view and list and fixees types and lints Signed-off-by: Tsaqif --- src/pages/home/ReportScreen.tsx | 9 ++++--- src/pages/home/report/ReportActionsList.tsx | 7 ++++-- src/pages/home/report/ReportActionsView.tsx | 28 ++++++++++----------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 94089f880c92..f12672dd64d7 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -1,5 +1,6 @@ import {PortalHost} from '@gorhom/portal'; -import {useIsFocused} from '@react-navigation/native'; +import type {RouteProp} from '@react-navigation/native'; +import {useIsFocused, useRoute} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; import lodashIsEqual from 'lodash/isEqual'; import React, {memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; @@ -32,6 +33,7 @@ import useViewportOffsetTop from '@hooks/useViewportOffsetTop'; import Timing from '@libs/actions/Timing'; import Log from '@libs/Log'; import Navigation from '@libs/Navigation/Navigation'; +import type {AuthScreensParamList} from '@libs/Navigation/types'; import clearReportNotifications from '@libs/Notification/clearReportNotifications'; import Performance from '@libs/Performance'; import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; @@ -39,7 +41,6 @@ import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import shouldFetchReport from '@libs/shouldFetchReport'; import * as ValidationUtils from '@libs/ValidationUtils'; -import type {AuthScreensParamList} from '@navigation/types'; import * as ComposerActions from '@userActions/Composer'; import * as Report from '@userActions/Report'; import CONST from '@src/CONST'; @@ -95,7 +96,8 @@ function getParentReportAction(parentReportActions: OnyxEntry>(); const styles = useThemeStyles(); const {translate} = useLocalize(); const reportIDFromRoute = getReportID(route); @@ -782,6 +784,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro ; + /** The transaction thread report associated with the current report, if any */ transactionThreadReport: OnyxEntry; @@ -142,6 +145,7 @@ const onScrollToIndexFailed = () => {}; function ReportActionsList({ report, + route, transactionThreadReport, reportActions = [], parentReportAction, @@ -171,7 +175,6 @@ function ReportActionsList({ const {isOffline} = useNetwork(); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`); - const route = useRoute>(); const opacity = useSharedValue(0); const reportScrollManager = useReportScrollManager(); const userActiveSince = useRef(null); diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index b76c6bd15a42..3bd5903a63b9 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -1,5 +1,5 @@ +import {useIsFocused} from '@react-navigation/native'; import type {RouteProp} from '@react-navigation/native'; -import {useIsFocused, useRoute} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import {InteractionManager} from 'react-native'; @@ -13,7 +13,6 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useWindowDimensions from '@hooks/useWindowDimensions'; import DateUtils from '@libs/DateUtils'; import getIsReportFullyVisible from '@libs/getIsReportFullyVisible'; -import type {AuthScreensParamList} from '@libs/Navigation/types'; import * as NumberUtils from '@libs/NumberUtils'; import {generateNewRandomInt} from '@libs/NumberUtils'; import Performance from '@libs/Performance'; @@ -22,6 +21,7 @@ import * as ReportUtils from '@libs/ReportUtils'; import {isUserCreatedPolicyRoom} from '@libs/ReportUtils'; import {didUserLogInDuringSession} from '@libs/SessionUtils'; import shouldFetchReport from '@libs/shouldFetchReport'; +import type {AuthScreensParamList} from '@navigation/types'; import {ReactionListContext} from '@pages/home/ReportScreenContext'; import * as Report from '@userActions/Report'; import Timing from '@userActions/Timing'; @@ -50,6 +50,9 @@ type ReportActionsViewProps = ReportActionsViewOnyxProps & { /** The report currently being looked at */ report: OnyxTypes.Report; + /** The current route of report screen */ + route: RouteProp; + /** Array of report actions for this report */ reportActions?: OnyxTypes.ReportAction[]; @@ -85,8 +88,7 @@ const SPACER = 16; let listOldID = Math.round(Math.random() * 100); function ReportActionsView({ - reportIDFromRoute: reportID, - reportActionIDFromRoute: reportActionID, + route, report, transactionThreadReport, session, @@ -103,9 +105,8 @@ function ReportActionsView({ }: ReportActionsViewProps) { useCopySelectionHelper(); const reactionListRef = useContext(ReactionListContext); - const route = useRoute>(); + const reportActionID = route?.params?.reportActionID; const prevReportActionID = usePrevious(reportActionID); - const prevReportID = usePrevious(reportID); const didLayout = useRef(false); const didLoadOlderChats = useRef(false); const didLoadNewerChats = useRef(false); @@ -124,6 +125,7 @@ function ReportActionsView({ const prevAuthTokenType = usePrevious(session?.authTokenType); const [isNavigatingToLinkedMessage, setNavigatingToLinkedMessage] = useState(!!reportActionID); const prevShouldUseNarrowLayoutRef = useRef(shouldUseNarrowLayout); + const reportID = report.reportID; const isLoading = (!!reportActionID && isLoadingInitialReportActions) || !isReadyForCommentLinking; const isReportFullyVisible = useMemo((): boolean => getIsReportFullyVisible(isFocused), [isFocused]); const openReportIfNecessary = () => { @@ -148,7 +150,7 @@ function ReportActionsView({ // Change the list ID only for comment linking to get the positioning right const listID = useMemo(() => { - if (!reportActionID && !prevReportActionID && (prevReportID === reportID)) { + if (!reportActionID && !prevReportActionID) { // Keep the old list ID since we're not in the Comment Linking flow return listOldID; } @@ -158,7 +160,7 @@ function ReportActionsView({ listOldID = newID; return newID; // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [reportID, reportActionID]); + }, [route, isLoadingInitialReportActions, reportActionID]); // When we are offline before opening an IOU/Expense report, // the total of the report and sometimes the expense aren't displayed because these actions aren't returned until `OpenReport` API is complete. @@ -241,6 +243,7 @@ function ReportActionsView({ return -1; } return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionID)); + // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [combinedReportActions, currentReportActionID, reportActionID]); const reportActions = useMemo(() => { @@ -287,6 +290,7 @@ function ReportActionsView({ // Get newer actions based on the newest reportAction for the transaction thread report const newestActionTransactionThreadReport = reportActionIDMap.find((item) => item.reportID === transactionThreadReport.reportID); + Report.getNewerActions(newestActionTransactionThreadReport?.reportID ?? '-1', newestActionTransactionThreadReport?.reportActionID ?? '-1'); } else { Report.getNewerActions(reportID, newestReportAction.reportActionID); @@ -494,8 +498,8 @@ function ReportActionsView({ <> Date: Fri, 2 Aug 2024 21:16:52 +0700 Subject: [PATCH 05/13] revert index of linked action search Signed-off-by: Tsaqif --- src/pages/home/report/ReportActionsView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index 3bd5903a63b9..048e35339ed2 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -242,7 +242,7 @@ function ReportActionsView({ if (!reportActionID) { return -1; } - return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionID)); + return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(isFirstLinkedActionRender.current ? reportActionID : currentReportActionID)); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [combinedReportActions, currentReportActionID, reportActionID]); From ceae85fbe2c64f30309a9ca0b2d5b76c3c1971e6 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Fri, 2 Aug 2024 21:29:24 +0700 Subject: [PATCH 06/13] Fix typechek error Signed-off-by: Tsaqif --- tests/perf-test/ReportActionsList.perf-test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/perf-test/ReportActionsList.perf-test.tsx b/tests/perf-test/ReportActionsList.perf-test.tsx index f0fef9a8e574..1447f5f298f2 100644 --- a/tests/perf-test/ReportActionsList.perf-test.tsx +++ b/tests/perf-test/ReportActionsList.perf-test.tsx @@ -72,6 +72,7 @@ beforeAll(() => const mockOnLayout = jest.fn(); const mockOnScroll = jest.fn(); const mockLoadChats = jest.fn(); +const mockRoute = {params: {reportID: '1', reportActionID: ''}, key: 'Report', name: 'Report' as const}; const mockRef = {current: null, flatListRef: null, scrollPosition: null, setScrollPosition: () => {}}; beforeEach(() => { @@ -91,6 +92,7 @@ function ReportActionsListWrapper() { parentReportActionForTransactionThread={undefined} sortedReportActions={ReportTestUtils.getMockedSortedReportActions(500)} report={LHNTestUtilsModule.getFakeReport()} + route={mockRoute} onLayout={mockOnLayout} onScroll={mockOnScroll} onContentSizeChange={() => {}} From a9d1374e2a8999b3fe6922ab4ada64cb72f9f266 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Sat, 3 Aug 2024 06:29:35 +0700 Subject: [PATCH 07/13] Fix perf-test error Signed-off-by: Tsaqif --- src/pages/home/report/ReportActionsView.tsx | 2 -- tests/perf-test/ReportScreen.perf-test.tsx | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index 048e35339ed2..85a1638442e4 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -243,7 +243,6 @@ function ReportActionsView({ return -1; } return combinedReportActions.findIndex((obj) => String(obj.reportActionID) === String(isFirstLinkedActionRender.current ? reportActionID : currentReportActionID)); - // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [combinedReportActions, currentReportActionID, reportActionID]); const reportActions = useMemo(() => { @@ -290,7 +289,6 @@ function ReportActionsView({ // Get newer actions based on the newest reportAction for the transaction thread report const newestActionTransactionThreadReport = reportActionIDMap.find((item) => item.reportID === transactionThreadReport.reportID); - Report.getNewerActions(newestActionTransactionThreadReport?.reportID ?? '-1', newestActionTransactionThreadReport?.reportActionID ?? '-1'); } else { Report.getNewerActions(reportID, newestReportAction.reportActionID); diff --git a/tests/perf-test/ReportScreen.perf-test.tsx b/tests/perf-test/ReportScreen.perf-test.tsx index 3621566fe56c..dc71c09424cd 100644 --- a/tests/perf-test/ReportScreen.perf-test.tsx +++ b/tests/perf-test/ReportScreen.perf-test.tsx @@ -112,13 +112,14 @@ jest.mock('@src/libs/Navigation/Navigation', () => ({ isDisplayedInModal: jest.fn(() => false), })); +const mockRoute = {params: {reportID: '1', reportActionID: ''}, key: 'Report', name: 'Report' as const}; jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); return { ...actualNav, useFocusEffect: jest.fn(), useIsFocused: () => true, - useRoute: () => jest.fn(), + useRoute: () => mockRoute, useNavigation: () => ({ navigate: jest.fn(), addListener: () => jest.fn(), @@ -212,7 +213,6 @@ function ReportScreenWrapper(props: ReportScreenWrapperProps) { const report = {...createRandomReport(1), policyID: '1'}; const reportActions = ReportTestUtils.getMockedReportActionsMap(1000); -const mockRoute = {params: {reportID: '1', reportActionID: ''}, key: 'Report', name: 'Report' as const}; test('[ReportScreen] should render ReportScreen', async () => { const {addListener} = createAddListenerMock(); From 02fca25488c656e8e9545594518bb85ac2c0b776 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Mon, 5 Aug 2024 07:20:49 +0700 Subject: [PATCH 08/13] Change method to get last item of result Signed-off-by: Tsaqif --- src/libs/PaginationUtils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/PaginationUtils.ts b/src/libs/PaginationUtils.ts index d1c096e99a8f..c86b20434b78 100644 --- a/src/libs/PaginationUtils.ts +++ b/src/libs/PaginationUtils.ts @@ -124,7 +124,8 @@ function mergeAndSortContinuousPages(sortedItems: TResource[], pages: const result = [sortedPages[0]]; for (let i = 1; i < sortedPages.length; i++) { const page = sortedPages[i]; - const prevPage = result?.at(-1) ?? sortedPages[i - 1]; + const prevPage = result[result.length - 1]; + //const prevPage = result?.at(-1) ?? sortedPages[i - 1]; // Current page is inside the previous page, skip if (page.lastIndex <= prevPage.lastIndex && page.lastID !== CONST.PAGINATION_END_ID) { From da00fa5a06c0d6d37e66c90ae44e99edbe29646e Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Mon, 5 Aug 2024 07:53:24 +0700 Subject: [PATCH 09/13] Fix lint Signed-off-by: Tsaqif --- src/libs/PaginationUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/PaginationUtils.ts b/src/libs/PaginationUtils.ts index c86b20434b78..a08868f5dcc1 100644 --- a/src/libs/PaginationUtils.ts +++ b/src/libs/PaginationUtils.ts @@ -125,7 +125,6 @@ function mergeAndSortContinuousPages(sortedItems: TResource[], pages: for (let i = 1; i < sortedPages.length; i++) { const page = sortedPages[i]; const prevPage = result[result.length - 1]; - //const prevPage = result?.at(-1) ?? sortedPages[i - 1]; // Current page is inside the previous page, skip if (page.lastIndex <= prevPage.lastIndex && page.lastID !== CONST.PAGINATION_END_ID) { From 7553ed69079fb92558c5ec3b59ad7619f606a681 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Tue, 6 Aug 2024 12:48:12 +0700 Subject: [PATCH 10/13] Revert route scheme Signed-off-by: Tsaqif --- src/pages/home/ReportScreen.tsx | 9 +++------ src/pages/home/report/ReportActionsList.tsx | 7 ++----- src/pages/home/report/ReportActionsView.tsx | 15 +++------------ 3 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index f12672dd64d7..94089f880c92 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -1,6 +1,5 @@ import {PortalHost} from '@gorhom/portal'; -import type {RouteProp} from '@react-navigation/native'; -import {useIsFocused, useRoute} from '@react-navigation/native'; +import {useIsFocused} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; import lodashIsEqual from 'lodash/isEqual'; import React, {memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; @@ -33,7 +32,6 @@ import useViewportOffsetTop from '@hooks/useViewportOffsetTop'; import Timing from '@libs/actions/Timing'; import Log from '@libs/Log'; import Navigation from '@libs/Navigation/Navigation'; -import type {AuthScreensParamList} from '@libs/Navigation/types'; import clearReportNotifications from '@libs/Notification/clearReportNotifications'; import Performance from '@libs/Performance'; import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; @@ -41,6 +39,7 @@ import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import shouldFetchReport from '@libs/shouldFetchReport'; import * as ValidationUtils from '@libs/ValidationUtils'; +import type {AuthScreensParamList} from '@navigation/types'; import * as ComposerActions from '@userActions/Composer'; import * as Report from '@userActions/Report'; import CONST from '@src/CONST'; @@ -96,8 +95,7 @@ function getParentReportAction(parentReportActions: OnyxEntry>(); +function ReportScreen({route, currentReportID = '', navigation}: ReportScreenProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); const reportIDFromRoute = getReportID(route); @@ -784,7 +782,6 @@ function ReportScreen({currentReportID = '', navigation}: ReportScreenProps) { ; - /** The transaction thread report associated with the current report, if any */ transactionThreadReport: OnyxEntry; @@ -145,7 +142,6 @@ const onScrollToIndexFailed = () => {}; function ReportActionsList({ report, - route, transactionThreadReport, reportActions = [], parentReportAction, @@ -175,6 +171,7 @@ function ReportActionsList({ const {isOffline} = useNetwork(); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`); + const route = useRoute>(); const opacity = useSharedValue(0); const reportScrollManager = useReportScrollManager(); const userActiveSince = useRef(null); diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index f1c04f1bf53c..a8eedcd70661 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -1,5 +1,5 @@ -import {useIsFocused} from '@react-navigation/native'; import type {RouteProp} from '@react-navigation/native'; +import {useIsFocused, useRoute} from '@react-navigation/native'; import lodashIsEqual from 'lodash/isEqual'; import React, {useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; import {InteractionManager} from 'react-native'; @@ -13,6 +13,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useWindowDimensions from '@hooks/useWindowDimensions'; import DateUtils from '@libs/DateUtils'; import getIsReportFullyVisible from '@libs/getIsReportFullyVisible'; +import type {AuthScreensParamList} from '@libs/Navigation/types'; import * as NumberUtils from '@libs/NumberUtils'; import {generateNewRandomInt} from '@libs/NumberUtils'; import Performance from '@libs/Performance'; @@ -21,7 +22,6 @@ import * as ReportUtils from '@libs/ReportUtils'; import {isUserCreatedPolicyRoom} from '@libs/ReportUtils'; import {didUserLogInDuringSession} from '@libs/SessionUtils'; import shouldFetchReport from '@libs/shouldFetchReport'; -import type {AuthScreensParamList} from '@navigation/types'; import {ReactionListContext} from '@pages/home/ReportScreenContext'; import * as Report from '@userActions/Report'; import Timing from '@userActions/Timing'; @@ -50,9 +50,6 @@ type ReportActionsViewProps = ReportActionsViewOnyxProps & { /** The report currently being looked at */ report: OnyxTypes.Report; - /** The current route of report screen */ - route: RouteProp; - /** Array of report actions for this report */ reportActions?: OnyxTypes.ReportAction[]; @@ -88,7 +85,6 @@ const SPACER = 16; let listOldID = Math.round(Math.random() * 100); function ReportActionsView({ - route, report, transactionThreadReport, session, @@ -105,6 +101,7 @@ function ReportActionsView({ }: ReportActionsViewProps) { useCopySelectionHelper(); const reactionListRef = useContext(ReactionListContext); + const route = useRoute>(); const reportActionID = route?.params?.reportActionID; const prevReportActionID = usePrevious(reportActionID); const didLayout = useRef(false); @@ -495,7 +492,6 @@ function ReportActionsView({ <> Date: Tue, 6 Aug 2024 12:53:26 +0700 Subject: [PATCH 11/13] Revert perf-test Signed-off-by: Tsaqif --- tests/perf-test/ReportActionsList.perf-test.tsx | 2 -- tests/perf-test/ReportScreen.perf-test.tsx | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/perf-test/ReportActionsList.perf-test.tsx b/tests/perf-test/ReportActionsList.perf-test.tsx index 1447f5f298f2..f0fef9a8e574 100644 --- a/tests/perf-test/ReportActionsList.perf-test.tsx +++ b/tests/perf-test/ReportActionsList.perf-test.tsx @@ -72,7 +72,6 @@ beforeAll(() => const mockOnLayout = jest.fn(); const mockOnScroll = jest.fn(); const mockLoadChats = jest.fn(); -const mockRoute = {params: {reportID: '1', reportActionID: ''}, key: 'Report', name: 'Report' as const}; const mockRef = {current: null, flatListRef: null, scrollPosition: null, setScrollPosition: () => {}}; beforeEach(() => { @@ -92,7 +91,6 @@ function ReportActionsListWrapper() { parentReportActionForTransactionThread={undefined} sortedReportActions={ReportTestUtils.getMockedSortedReportActions(500)} report={LHNTestUtilsModule.getFakeReport()} - route={mockRoute} onLayout={mockOnLayout} onScroll={mockOnScroll} onContentSizeChange={() => {}} diff --git a/tests/perf-test/ReportScreen.perf-test.tsx b/tests/perf-test/ReportScreen.perf-test.tsx index dc71c09424cd..3621566fe56c 100644 --- a/tests/perf-test/ReportScreen.perf-test.tsx +++ b/tests/perf-test/ReportScreen.perf-test.tsx @@ -112,14 +112,13 @@ jest.mock('@src/libs/Navigation/Navigation', () => ({ isDisplayedInModal: jest.fn(() => false), })); -const mockRoute = {params: {reportID: '1', reportActionID: ''}, key: 'Report', name: 'Report' as const}; jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); return { ...actualNav, useFocusEffect: jest.fn(), useIsFocused: () => true, - useRoute: () => mockRoute, + useRoute: () => jest.fn(), useNavigation: () => ({ navigate: jest.fn(), addListener: () => jest.fn(), @@ -213,6 +212,7 @@ function ReportScreenWrapper(props: ReportScreenWrapperProps) { const report = {...createRandomReport(1), policyID: '1'}; const reportActions = ReportTestUtils.getMockedReportActionsMap(1000); +const mockRoute = {params: {reportID: '1', reportActionID: ''}, key: 'Report', name: 'Report' as const}; test('[ReportScreen] should render ReportScreen', async () => { const {addListener} = createAddListenerMock(); From c498c92a873d55d0e51233b385bf6f1d253c8298 Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Thu, 8 Aug 2024 17:17:03 +0700 Subject: [PATCH 12/13] Fix commment linking jumping around and new message button in android Signed-off-by: Tsaqif --- src/components/FlatList/index.android.tsx | 18 ++++++++++++------ src/pages/home/ReportScreen.tsx | 13 +++++++++++-- src/pages/home/ReportScreenContext.ts | 4 ++-- src/pages/home/report/ReportActionsView.tsx | 1 + 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/components/FlatList/index.android.tsx b/src/components/FlatList/index.android.tsx index c8ce7ee10d6b..de41e5fa4697 100644 --- a/src/components/FlatList/index.android.tsx +++ b/src/components/FlatList/index.android.tsx @@ -1,6 +1,6 @@ import {useFocusEffect} from '@react-navigation/native'; import type {ForwardedRef} from 'react'; -import React, {forwardRef, useCallback, useContext} from 'react'; +import React, {forwardRef, useCallback, useContext, useEffect} from 'react'; import type {FlatListProps, NativeScrollEvent, NativeSyntheticEvent} from 'react-native'; import {FlatList} from 'react-native'; import {ActionListContext} from '@pages/home/ReportScreenContext'; @@ -8,23 +8,29 @@ import {ActionListContext} from '@pages/home/ReportScreenContext'; // FlatList wrapped with the freeze component will lose its scroll state when frozen (only for Android). // CustomFlatList saves the offset and use it for scrollToOffset() when unfrozen. function CustomFlatList(props: FlatListProps, ref: ForwardedRef) { - const {scrollPosition, setScrollPosition} = useContext(ActionListContext); + const {setScrollPosition, getScrollPosition} = useContext(ActionListContext); const onScreenFocus = useCallback(() => { + const pos = getScrollPosition(); if (typeof ref === 'function') { return; } - if (!ref?.current || !scrollPosition?.offset) { + if (!ref?.current || !pos?.offset) { return; } - if (ref.current && scrollPosition.offset) { - ref.current.scrollToOffset({offset: scrollPosition.offset, animated: false}); + if (ref.current && pos?.offset) { + ref.current.scrollToOffset({offset: pos.offset, animated: false}); } - }, [scrollPosition?.offset, ref]); + }, [ref]); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps const onMomentumScrollEnd = useCallback((event: NativeSyntheticEvent) => setScrollPosition({offset: event.nativeEvent.contentOffset.y}), []); + useEffect(() => { + setScrollPosition(undefined); + return () => setScrollPosition(undefined); + },[]) + useFocusEffect( useCallback(() => { onScreenFocus(); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 94089f880c92..caa6d4a8d5cb 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -95,6 +95,8 @@ function getParentReportAction(parentReportActions: OnyxEntry({}); + + const setScrollPosition = useCallback((pos) => { + scrollPosition = pos; + }, []); + + const getScrollPosition = useCallback(() => { + return scrollPosition; + }, []); const wasReportAccessibleRef = useRef(false); if (firstRenderRef.current) { @@ -683,7 +692,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro }; }, [report, didSubscribeToReportLeavingEvents, reportIDFromRoute]); - const actionListValue = useMemo((): ActionListContextType => ({flatListRef, scrollPosition, setScrollPosition}), [flatListRef, scrollPosition, setScrollPosition]); + const actionListValue = useMemo((): ActionListContextType => ({flatListRef, setScrollPosition, getScrollPosition}), [flatListRef, setScrollPosition, getScrollPosition]); // This helps in tracking from the moment 'route' triggers useMemo until isLoadingInitialReportActions becomes true. It prevents blinking when loading reportActions from cache. useEffect(() => { diff --git a/src/pages/home/ReportScreenContext.ts b/src/pages/home/ReportScreenContext.ts index e67bf73f7452..5322d4b1de30 100644 --- a/src/pages/home/ReportScreenContext.ts +++ b/src/pages/home/ReportScreenContext.ts @@ -19,12 +19,12 @@ type ScrollPosition = {offset?: number}; type ActionListContextType = { flatListRef: FlatListRefType; - scrollPosition: ScrollPosition | null; setScrollPosition: (position: {offset: number}) => void; + getScrollPosition: () => ScrollPosition; }; type ReactionListContextType = RefObject | null; -const ActionListContext = createContext({flatListRef: null, scrollPosition: null, setScrollPosition: () => {}}); +const ActionListContext = createContext({flatListRef: null, setScrollPosition: () => {}, getScrollPosition: () => {}}); const ReactionListContext = createContext(null); export {ActionListContext, ReactionListContext}; diff --git a/src/pages/home/report/ReportActionsView.tsx b/src/pages/home/report/ReportActionsView.tsx index a8eedcd70661..a670fbd8e09d 100755 --- a/src/pages/home/report/ReportActionsView.tsx +++ b/src/pages/home/report/ReportActionsView.tsx @@ -527,6 +527,7 @@ function arePropsEqual(oldProps: ReportActionsViewProps, newProps: ReportActions if (!lodashIsEqual(oldProps.isReadyForCommentLinking, newProps.isReadyForCommentLinking)) { return false; } + if (!lodashIsEqual(oldProps.reportActions, newProps.reportActions)) { return false; } From 0993bd40e8d736ad7607ea1ee0656fb7aa6b6dca Mon Sep 17 00:00:00 2001 From: Tsaqif Date: Sun, 11 Aug 2024 06:46:14 +0700 Subject: [PATCH 13/13] Revert changes Signed-off-by: Tsaqif --- src/components/FlatList/index.android.tsx | 18 ++++++------------ src/pages/home/ReportScreen.tsx | 13 ++----------- src/pages/home/ReportScreenContext.ts | 4 ++-- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/components/FlatList/index.android.tsx b/src/components/FlatList/index.android.tsx index de41e5fa4697..c8ce7ee10d6b 100644 --- a/src/components/FlatList/index.android.tsx +++ b/src/components/FlatList/index.android.tsx @@ -1,6 +1,6 @@ import {useFocusEffect} from '@react-navigation/native'; import type {ForwardedRef} from 'react'; -import React, {forwardRef, useCallback, useContext, useEffect} from 'react'; +import React, {forwardRef, useCallback, useContext} from 'react'; import type {FlatListProps, NativeScrollEvent, NativeSyntheticEvent} from 'react-native'; import {FlatList} from 'react-native'; import {ActionListContext} from '@pages/home/ReportScreenContext'; @@ -8,29 +8,23 @@ import {ActionListContext} from '@pages/home/ReportScreenContext'; // FlatList wrapped with the freeze component will lose its scroll state when frozen (only for Android). // CustomFlatList saves the offset and use it for scrollToOffset() when unfrozen. function CustomFlatList(props: FlatListProps, ref: ForwardedRef) { - const {setScrollPosition, getScrollPosition} = useContext(ActionListContext); + const {scrollPosition, setScrollPosition} = useContext(ActionListContext); const onScreenFocus = useCallback(() => { - const pos = getScrollPosition(); if (typeof ref === 'function') { return; } - if (!ref?.current || !pos?.offset) { + if (!ref?.current || !scrollPosition?.offset) { return; } - if (ref.current && pos?.offset) { - ref.current.scrollToOffset({offset: pos.offset, animated: false}); + if (ref.current && scrollPosition.offset) { + ref.current.scrollToOffset({offset: scrollPosition.offset, animated: false}); } - }, [ref]); + }, [scrollPosition?.offset, ref]); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps const onMomentumScrollEnd = useCallback((event: NativeSyntheticEvent) => setScrollPosition({offset: event.nativeEvent.contentOffset.y}), []); - useEffect(() => { - setScrollPosition(undefined); - return () => setScrollPosition(undefined); - },[]) - useFocusEffect( useCallback(() => { onScreenFocus(); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index caa6d4a8d5cb..94089f880c92 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -95,8 +95,6 @@ function getParentReportAction(parentReportActions: OnyxEntry { - scrollPosition = pos; - }, []); - - const getScrollPosition = useCallback(() => { - return scrollPosition; - }, []); + const [scrollPosition, setScrollPosition] = useState({}); const wasReportAccessibleRef = useRef(false); if (firstRenderRef.current) { @@ -692,7 +683,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro }; }, [report, didSubscribeToReportLeavingEvents, reportIDFromRoute]); - const actionListValue = useMemo((): ActionListContextType => ({flatListRef, setScrollPosition, getScrollPosition}), [flatListRef, setScrollPosition, getScrollPosition]); + const actionListValue = useMemo((): ActionListContextType => ({flatListRef, scrollPosition, setScrollPosition}), [flatListRef, scrollPosition, setScrollPosition]); // This helps in tracking from the moment 'route' triggers useMemo until isLoadingInitialReportActions becomes true. It prevents blinking when loading reportActions from cache. useEffect(() => { diff --git a/src/pages/home/ReportScreenContext.ts b/src/pages/home/ReportScreenContext.ts index 5322d4b1de30..e67bf73f7452 100644 --- a/src/pages/home/ReportScreenContext.ts +++ b/src/pages/home/ReportScreenContext.ts @@ -19,12 +19,12 @@ type ScrollPosition = {offset?: number}; type ActionListContextType = { flatListRef: FlatListRefType; + scrollPosition: ScrollPosition | null; setScrollPosition: (position: {offset: number}) => void; - getScrollPosition: () => ScrollPosition; }; type ReactionListContextType = RefObject | null; -const ActionListContext = createContext({flatListRef: null, setScrollPosition: () => {}, getScrollPosition: () => {}}); +const ActionListContext = createContext({flatListRef: null, scrollPosition: null, setScrollPosition: () => {}}); const ReactionListContext = createContext(null); export {ActionListContext, ReactionListContext};