Skip to content
13 changes: 13 additions & 0 deletions src/hooks/useActionListContextValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {useMemo, useRef, useState} from 'react';
import type {FlatList} from 'react-native';
import type {ActionListContextType, ScrollPosition} from '@pages/inbox/ReportScreenContext';

function useActionListContextValue(): ActionListContextType {
const flatListRef = useRef<FlatList>(null);
const [scrollPosition, setScrollPosition] = useState<ScrollPosition>({});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are misusing state here: scrollPosition is an observed property that can change very frequently. Calling setScrollPosition isn't intended to cause re-renders - the list itself should control its own scroll position.

Created a plan to fix it.


Refactor Scroll State Management

Problem Summary

Two inefficiencies exist in the scroll position tracking code:

  1. Unused state causing re-renders: [scrollPosition](src/hooks/useActionListContextValue.ts) is state but never read - only written via setScrollPosition. Every write triggers unnecessary re-renders.
  2. Duplicate scroll tracking: [ReportActionsList.tsx](src/pages/inbox/report/ReportActionsList.tsx) maintains TWO refs tracking the same scroll offset:
  • Local scrollingVerticalOffset (line 219)
  • Context scrollOffsetRef (line 176)
  • Both updated with identical values (lines 369-370)

Changes Required

1. Convert scrollPosition from state to ref

**File: src/hooks/useActionListContextValue.ts**

Change from:

const [scrollPosition, setScrollPosition] = useState<ScrollPosition>({});

To:

const scrollPositionRef = useRef<ScrollPosition>({});

Update return statement to remove setScrollPosition and use scrollPositionRef.

2. Update type definitions

**File: src/pages/inbox/ReportScreenContext.ts**

Change ActionListContextType:

type ActionListContextType = {
    flatListRef: FlatListRefType;
    scrollPositionRef: RefObject<ScrollPosition>;  // Changed from scrollPosition/setScrollPosition
    scrollOffsetRef: RefObject<number>;
};

Update default context value accordingly.

3. Update useReportScrollManager

**File: src/hooks/useReportScrollManager/index.native.ts**

Change from:

const {flatListRef, setScrollPosition} = useContext(ActionListContext);

To:

const {flatListRef, scrollPositionRef} = useContext(ActionListContext);

Update scrollToBottom callback (line 33) to use ref:

scrollPositionRef.current = {offset: 0};

4. Eliminate duplicate scroll tracking in ReportActionsList

**File: src/pages/inbox/report/ReportActionsList.tsx**

Remove local scrollingVerticalOffset ref declaration (line 219).

Replace all usages with scrollOffsetRef:

  • Line 298: Pass scrollOffsetRef.current to getUnreadMarkerIndex
  • Line 364: Pass scrollOffsetRef to useReportUnreadMessageScrollTracking
  • Line 369: Remove scrollingVerticalOffset.current = ... (keep only scrollOffsetRef.current = ...)
  • Line 380: Change condition to use scrollOffsetRef.current

Files Modified

Performance Benefits

  • Eliminates unnecessary re-renders when scroll position changes (no longer triggers state updates)
  • Reduces memory overhead by removing duplicate ref tracking
  • Improves code clarity with single source of truth for scroll offset

diff
diff --git a/src/hooks/useActionListContextValue.ts b/src/hooks/useActionListContextValue.ts
index 7f4c1d1a6b7..ba73ff804cc 100644
--- a/src/hooks/useActionListContextValue.ts
+++ b/src/hooks/useActionListContextValue.ts
@@ -1,13 +1,13 @@
-import {useMemo, useRef, useState} from 'react';
+import {useRef} from 'react';
 import type {FlatList} from 'react-native';
 import type {ActionListContextType, ScrollPosition} from '@pages/inbox/ReportScreenContext';
 
 function useActionListContextValue(): ActionListContextType {
     const flatListRef = useRef<FlatList>(null);
-    const [scrollPosition, setScrollPosition] = useState<ScrollPosition>({});
+    const scrollPositionRef = useRef<ScrollPosition>({});
     const scrollOffsetRef = useRef(0);
 
-    return useMemo(() => ({flatListRef, scrollPosition, setScrollPosition, scrollOffsetRef}), [scrollPosition]);
+    return {flatListRef, scrollPositionRef, scrollOffsetRef};
 }
 
 export default useActionListContextValue;
diff --git a/src/hooks/useReportScrollManager/index.native.ts b/src/hooks/useReportScrollManager/index.native.ts
index 39e5a1b66ee..180196dd0b3 100644
--- a/src/hooks/useReportScrollManager/index.native.ts
+++ b/src/hooks/useReportScrollManager/index.native.ts
@@ -1,44 +1,39 @@
-import {useCallback, useContext} from 'react';
+import {useContext} from 'react';
 // eslint-disable-next-line no-restricted-imports
 import type {ScrollView} from 'react-native';
 import {ActionListContext} from '@pages/inbox/ReportScreenContext';
 import type ReportScrollManagerData from './types';
 
 function useReportScrollManager(): ReportScrollManagerData {
-    const {flatListRef, setScrollPosition} = useContext(ActionListContext);
+    const {flatListRef, scrollPositionRef} = useContext(ActionListContext);
 
     /**
      * Scroll to the provided index.
      */
-    const scrollToIndex = useCallback(
-        (index: number) => {
-            if (!flatListRef?.current) {
-                return;
-            }
-
-            flatListRef.current.scrollToIndex({index});
-        },
-        [flatListRef],
-    );
+    const scrollToIndex = (index: number) => {
+        if (!flatListRef?.current) {
+            return;
+        }
+        flatListRef.current.scrollToIndex({index});
+    };
 
     /**
      * Scroll to the bottom of the inverted FlatList.
      * When FlatList is inverted it's "bottom" is really it's top
      */
-    const scrollToBottom = useCallback(() => {
+    const scrollToBottom = () => {
         if (!flatListRef?.current) {
             return;
         }
 
-        setScrollPosition({offset: 0});
-
+        scrollPositionRef.current = {offset: 0};
         flatListRef.current?.scrollToOffset({animated: false, offset: 0});
-    }, [flatListRef, setScrollPosition]);
+    };
 
     /**
      * Scroll to the end of the FlatList.
      */
-    const scrollToEnd = useCallback(() => {
+    const scrollToEnd = () => {
         if (!flatListRef?.current) {
             return;
         }
@@ -51,18 +46,14 @@ function useReportScrollManager(): ReportScrollManagerData {
         }
 
         flatListRef.current.scrollToEnd({animated: false});
-    }, [flatListRef]);
+    };
 
-    const scrollToOffset = useCallback(
-        (offset: number) => {
-            if (!flatListRef?.current) {
-                return;
-            }
-
-            flatListRef.current.scrollToOffset({offset, animated: false});
-        },
-        [flatListRef],
-    );
+    const scrollToOffset = (offset: number) => {
+        if (!flatListRef?.current) {
+            return;
+        }
+        flatListRef.current.scrollToOffset({offset, animated: false});
+    };
 
     return {ref: flatListRef, scrollToIndex, scrollToBottom, scrollToEnd, scrollToOffset};
 }
diff --git a/src/hooks/useReportScrollManager/index.ts b/src/hooks/useReportScrollManager/index.ts
index 30c383e62c2..6b888584887 100644
--- a/src/hooks/useReportScrollManager/index.ts
+++ b/src/hooks/useReportScrollManager/index.ts
@@ -1,4 +1,4 @@
-import {useCallback, useContext} from 'react';
+import {useContext} from 'react';
 import {ActionListContext} from '@pages/inbox/ReportScreenContext';
 import type ReportScrollManagerData from './types';
 
@@ -8,50 +8,44 @@ function useReportScrollManager(): ReportScrollManagerData {
     /**
      * Scroll to the provided index. On non-native implementations we do not want to scroll when we are scrolling because
      */
-    const scrollToIndex = useCallback(
-        (index: number, isEditing?: boolean) => {
-            if (!flatListRef?.current || isEditing) {
-                return;
-            }
+    const scrollToIndex = (index: number, isEditing?: boolean) => {
+        if (!flatListRef?.current || isEditing) {
+            return;
+        }
 
-            flatListRef.current.scrollToIndex({index, animated: true});
-        },
-        [flatListRef],
-    );
+        flatListRef.current.scrollToIndex({index, animated: true});
+    };
 
     /**
      * Scroll to the bottom of the inverted FlatList.
      * When FlatList is inverted it's "bottom" is really it's top
      */
-    const scrollToBottom = useCallback(() => {
+    const scrollToBottom = () => {
         if (!flatListRef?.current) {
             return;
         }
 
         flatListRef.current.scrollToOffset({animated: false, offset: 0});
-    }, [flatListRef]);
+    };
 
     /**
      * Scroll to the end of the FlatList.
      */
-    const scrollToEnd = useCallback(() => {
+    const scrollToEnd = () => {
         if (!flatListRef?.current) {
             return;
         }
 
         flatListRef.current.scrollToEnd({animated: false});
-    }, [flatListRef]);
-
-    const scrollToOffset = useCallback(
-        (offset: number) => {
-            if (!flatListRef?.current) {
-                return;
-            }
-
-            flatListRef.current.scrollToOffset({animated: true, offset});
-        },
-        [flatListRef],
-    );
+    };
+
+    const scrollToOffset = (offset: number) => {
+        if (!flatListRef?.current) {
+            return;
+        }
+
+        flatListRef.current.scrollToOffset({animated: true, offset});
+    };
 
     return {ref: flatListRef, scrollToIndex, scrollToBottom, scrollToEnd, scrollToOffset};
 }
diff --git a/src/pages/inbox/ReportScreenContext.ts b/src/pages/inbox/ReportScreenContext.ts
index ec56c097eee..50b1e3e707f 100644
--- a/src/pages/inbox/ReportScreenContext.ts
+++ b/src/pages/inbox/ReportScreenContext.ts
@@ -19,13 +19,12 @@ type ScrollPosition = {offset?: number};
 
 type ActionListContextType = {
     flatListRef: FlatListRefType;
-    scrollPosition: ScrollPosition | null;
-    setScrollPosition: (position: {offset: number}) => void;
+    scrollPositionRef: RefObject<ScrollPosition>;
     scrollOffsetRef: RefObject<number>;
 };
 type ReactionListContextType = RefObject<ReactionListRef | null> | null;
 
-const ActionListContext = createContext<ActionListContextType>({flatListRef: null, scrollPosition: null, setScrollPosition: () => {}, scrollOffsetRef: {current: 0}});
+const ActionListContext = createContext<ActionListContextType>({flatListRef: null, scrollPositionRef: {current: {}}, scrollOffsetRef: {current: 0}});
 const ReactionListContext = createContext<ReactionListContextType>(null);
 
 export {ActionListContext, ReactionListContext};
diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx
index 2640683af9e..04016ee50fc 100644
--- a/src/pages/inbox/report/ReportActionsList.tsx
+++ b/src/pages/inbox/report/ReportActionsList.tsx
@@ -216,7 +216,6 @@ function ReportActionsList({
         return unsubscribe;
     }, []);
 
-    const scrollingVerticalOffset = useRef(0);
     const readActionSkipped = useRef(false);
     const hasHeaderRendered = useRef(false);
     const linkedReportActionID = route?.params?.reportActionID;
@@ -295,7 +294,7 @@ function ReportActionsList({
                     currentUserAccountID,
                     prevSortedVisibleReportActionsObjects,
                     unreadMarkerTime,
-                    scrollingVerticalOffset: scrollingVerticalOffset.current,
+                    scrollingVerticalOffset: scrollOffsetRef.current,
                     prevUnreadMarkerReportActionID: prevUnreadMarkerReportActionID.current,
                 });
             if (shouldDisplayNewMarker) {
@@ -361,12 +360,11 @@ function ReportActionsList({
 
     const {isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible, trackVerticalScrolling, onViewableItemsChanged} = useReportUnreadMessageScrollTracking({
         reportID: report.reportID,
-        currentVerticalScrollingOffsetRef: scrollingVerticalOffset,
+        currentVerticalScrollingOffsetRef: scrollOffsetRef,
         readActionSkippedRef: readActionSkipped,
         unreadMarkerReportActionIndex,
         isInverted: true,
         onTrackScrolling: (event: NativeSyntheticEvent<NativeScrollEvent>) => {
-            scrollingVerticalOffset.current = event.nativeEvent.contentOffset.y;
             scrollOffsetRef.current = event.nativeEvent.contentOffset.y;
             onScroll?.(event);
             if (shouldScrollToEndAfterLayout && (!hasCreatedActionAdded || isOffline)) {
@@ -377,7 +375,7 @@ function ReportActionsList({
 
     useEffect(() => {
         if (
-            scrollingVerticalOffset.current < AUTOSCROLL_TO_TOP_THRESHOLD &&
+            scrollOffsetRef.current < AUTOSCROLL_TO_TOP_THRESHOLD &&
             previousLastIndex.current !== lastActionIndex &&
             reportActionSize.current !== sortedVisibleReportActions.length &&
             hasNewestReportAction
diff --git a/tests/perf-test/ReportActionsList.perf-test.tsx b/tests/perf-test/ReportActionsList.perf-test.tsx
index 309e5c289b4..72122f8bf02 100644
--- a/tests/perf-test/ReportActionsList.perf-test.tsx
+++ b/tests/perf-test/ReportActionsList.perf-test.tsx
@@ -70,7 +70,7 @@ beforeAll(() =>
 const mockOnLayout = jest.fn();
 const mockOnScroll = jest.fn();
 const mockLoadChats = jest.fn();
-const mockRef = {current: null, flatListRef: null, scrollPosition: null, setScrollPosition: () => {}, scrollOffsetRef: {current: 0}};
+const mockRef = {current: null, flatListRef: null, scrollPositionRef: {current: {}}, scrollOffsetRef: {current: 0}};
 
 const TEST_USER_ACCOUNT_ID = 1;
 const TEST_USER_LOGIN = 'test@test.com';

const scrollOffsetRef = useRef(0);

return useMemo(() => ({flatListRef, scrollPosition, setScrollPosition, scrollOffsetRef}), [scrollPosition]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is compiled by React Compiler, so we don't need useMemo here. Let's remove it since it's not needed

}

export default useActionListContextValue;
32 changes: 28 additions & 4 deletions src/libs/actions/Report/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@
shouldPlaySound?: boolean;
isInSidePanel?: boolean;
pregeneratedResponseParams?: PregeneratedResponseParams;
reportActionID?: string;
};

type AddActionsParams = {
Expand All @@ -292,6 +293,7 @@
file?: FileObject;
isInSidePanel?: boolean;
pregeneratedResponseParams?: PregeneratedResponseParams;
reportActionID?: string;
};

type AddAttachmentWithCommentParams = {
Expand All @@ -312,7 +314,7 @@
/** @deprecated This value is deprecated and will be removed soon after migration. Use the email from useCurrentUserPersonalDetails hook instead. */
let deprecatedCurrentUserLogin: string | undefined;

Onyx.connect({

Check warning on line 317 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
// When signed out, val is undefined
Expand All @@ -326,7 +328,7 @@
},
});

Onyx.connect({

Check warning on line 331 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.CONCIERGE_REPORT_ID,
callback: (value) => (conciergeReportIDOnyxConnect = value),
});
Expand All @@ -334,7 +336,7 @@
// map of reportID to all reportActions for that report
const allReportActions: OnyxCollection<ReportActions> = {};

Onyx.connect({

Check warning on line 339 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
callback: (actions, key) => {
if (!key || !actions) {
Expand All @@ -346,7 +348,7 @@
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 351 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -355,7 +357,7 @@
});

let allPersonalDetails: OnyxEntry<PersonalDetailsList> = {};
Onyx.connect({

Check warning on line 360 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand All @@ -370,7 +372,7 @@
});

let onboarding: OnyxEntry<Onboarding>;
Onyx.connect({

Check warning on line 375 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_ONBOARDING,
callback: (val) => {
if (Array.isArray(val)) {
Expand All @@ -381,7 +383,7 @@
});

let deprecatedIntroSelected: OnyxEntry<IntroSelected> = {};
Onyx.connect({

Check warning on line 386 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_INTRO_SELECTED,
callback: (val) => (deprecatedIntroSelected = val),
});
Expand Down Expand Up @@ -609,7 +611,18 @@
* @param isInSidePanel - Whether the comment is being added from the side panel
* @param pregeneratedResponseParams - Optional params for pre-generated response (API only, no optimistic action - used when response display is delayed)
*/
function addActions({report, notifyReportID, ancestors, timezoneParam, currentUserAccountID, text = '', file, isInSidePanel = false, pregeneratedResponseParams}: AddActionsParams) {
function addActions({
report,
notifyReportID,
ancestors,
timezoneParam,
currentUserAccountID,
text = '',
file,
isInSidePanel = false,
pregeneratedResponseParams,
reportActionID,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Would an alternative here be to return the reportActionID for the newly created actions? that way it doesn't have to be passed in. NAB

}: AddActionsParams) {
if (!report?.reportID) {
return;
}
Expand All @@ -620,7 +633,7 @@
let commandName: typeof WRITE_COMMANDS.ADD_COMMENT | typeof WRITE_COMMANDS.ADD_ATTACHMENT | typeof WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT = WRITE_COMMANDS.ADD_COMMENT;

if (text && !file) {
const reportComment = buildOptimisticAddCommentReportAction(text, undefined, undefined, undefined, reportID);
const reportComment = buildOptimisticAddCommentReportAction(text, undefined, undefined, undefined, reportID, reportActionID);
reportCommentAction = reportComment.reportAction;
reportCommentText = reportComment.commentText;
}
Expand Down Expand Up @@ -848,11 +861,22 @@
}

/** Add a single comment to a report */
function addComment({report, notifyReportID, ancestors, text, timezoneParam, currentUserAccountID, shouldPlaySound, isInSidePanel, pregeneratedResponseParams}: AddCommentParams) {
function addComment({
report,
notifyReportID,
ancestors,
text,
timezoneParam,
currentUserAccountID,
shouldPlaySound,
isInSidePanel,
pregeneratedResponseParams,
reportActionID,
}: AddCommentParams) {
if (shouldPlaySound) {
playSound(SOUNDS.DONE);
}
addActions({report, notifyReportID, ancestors, timezoneParam, currentUserAccountID, text, isInSidePanel, pregeneratedResponseParams});
addActions({report, notifyReportID, ancestors, timezoneParam, currentUserAccountID, text, isInSidePanel, pregeneratedResponseParams, reportActionID});
}

function reportActionsExist(reportID: string): boolean {
Expand Down
10 changes: 9 additions & 1 deletion src/libs/telemetry/activeSpans.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,16 @@ function cancelAllSpans() {
}
}

function cancelSpansByPrefix(prefix: string) {
for (const [spanId] of activeSpans.entries()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per code style conventions:

Suggested change
for (const [spanId] of activeSpans.entries()) {
for (const [spanID] of activeSpans.entries()) {

if (spanId.startsWith(prefix)) {
cancelSpan(spanId);
}
}
}

function getSpan(spanId: string) {
return activeSpans.get(spanId);
}

export {startSpan, endSpan, getSpan, cancelSpan, cancelAllSpans};
export {startSpan, endSpan, getSpan, cancelSpan, cancelAllSpans, cancelSpansByPrefix};
15 changes: 9 additions & 6 deletions src/pages/Search/SearchMoneyRequestReportPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {PortalHost} from '@gorhom/portal';
import {useIsFocused} from '@react-navigation/native';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import type {FlatList} from 'react-native';
import React, {useCallback, useEffect, useMemo, useRef} from 'react';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import DragAndDropProvider from '@components/DragAndDrop/Provider';
Expand All @@ -10,6 +9,7 @@ import ScreenWrapper from '@components/ScreenWrapper';
import {useSearchContext} from '@components/Search/SearchContext';
import useShowSuperWideRHPVersion from '@components/WideRHPContextProvider/useShowSuperWideRHPVersion';
import WideRHPOverlayWrapper from '@components/WideRHPOverlayWrapper';
import useActionListContextValue from '@hooks/useActionListContextValue';
import useIsReportReadyToDisplay from '@hooks/useIsReportReadyToDisplay';
import useNetwork from '@hooks/useNetwork';
import useOnyx from '@hooks/useOnyx';
Expand All @@ -33,10 +33,10 @@ import {
isMoneyRequestAction,
} from '@libs/ReportActionsUtils';
import {isValidReportIDFromPath} from '@libs/ReportUtils';
import {cancelSpansByPrefix} from '@libs/telemetry/activeSpans';
import {isDefaultAvatar, isLetterAvatar, isPresetAvatar} from '@libs/UserAvatarUtils';
import Navigation from '@navigation/Navigation';
import ReactionListWrapper from '@pages/inbox/ReactionListWrapper';
import type {ActionListContextType, ScrollPosition} from '@pages/inbox/ReportScreenContext';
import {ActionListContext} from '@pages/inbox/ReportScreenContext';
import {createTransactionThreadReport, openReport, updateLastVisitTime} from '@userActions/Report';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -99,9 +99,7 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {

const {isEditingDisabled, isCurrentReportLoadedFromOnyx} = useIsReportReadyToDisplay(report, reportIDFromRoute, isReportArchived);

const [scrollPosition, setScrollPosition] = useState<ScrollPosition>({});
const flatListRef = useRef<FlatList>(null);
const actionListValue = useMemo((): ActionListContextType => ({flatListRef, scrollPosition, setScrollPosition}), [flatListRef, scrollPosition, setScrollPosition]);
const actionListValue = useActionListContextValue();

const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`, {canBeMissing: true});
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {canBeMissing: true});
Expand Down Expand Up @@ -197,6 +195,11 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {

useEffect(() => {
hasCreatedLegacyThreadRef.current = false;

return () => {
// Cancel any pending send-message spans to prevent orphaned spans when navigating away
cancelSpansByPrefix(CONST.TELEMETRY.SPAN_SEND_MESSAGE);
};
}, [reportIDFromRoute]);

// Create transaction thread for legacy transactions that don't have one yet.
Expand Down
13 changes: 7 additions & 6 deletions src/pages/inbox/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {useFocusEffect, useIsFocused} from '@react-navigation/native';
import {accountIDSelector} from '@selectors/Session';
import {deepEqual} from 'fast-equals';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import type {FlatList, ViewStyle} from 'react-native';
import type {ViewStyle} from 'react-native';
// We use Animated for all functionality related to wide RHP to make it easier
// to interact with react-navigation components (e.g., CardContainer, interpolator), which also use Animated.
// eslint-disable-next-line no-restricted-imports
Expand All @@ -22,6 +22,7 @@ import ScreenWrapper from '@components/ScreenWrapper';
import ScrollView from '@components/ScrollView';
import useShowWideRHPVersion from '@components/WideRHPContextProvider/useShowWideRHPVersion';
import WideRHPOverlayWrapper from '@components/WideRHPOverlayWrapper';
import useActionListContextValue from '@hooks/useActionListContextValue';
import useAppFocusEvent from '@hooks/useAppFocusEvent';
import useArchivedReportsIdSet from '@hooks/useArchivedReportsIdSet';
import {useCurrentReportIDState} from '@hooks/useCurrentReportID';
Expand Down Expand Up @@ -87,7 +88,7 @@ import {
isTaskReport,
isValidReportIDFromPath,
} from '@libs/ReportUtils';
import {cancelSpan} from '@libs/telemetry/activeSpans';
import {cancelSpan, cancelSpansByPrefix} from '@libs/telemetry/activeSpans';
import {isNumeric} from '@libs/ValidationUtils';
import type {ReportsSplitNavigatorParamList, RightModalNavigatorParamList} from '@navigation/types';
import {setShouldShowComposeInput} from '@userActions/Composer';
Expand All @@ -113,7 +114,6 @@ import useReportWasDeleted from './hooks/useReportWasDeleted';
import ReactionListWrapper from './ReactionListWrapper';
import ReportActionsView from './report/ReportActionsView';
import ReportFooter from './report/ReportFooter';
import type {ActionListContextType, ScrollPosition} from './ReportScreenContext';
import {ActionListContext} from './ReportScreenContext';

type ReportScreenNavigationProps =
Expand Down Expand Up @@ -167,7 +167,6 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
const prevIsFocused = usePrevious(isFocused);
const [firstRender, setFirstRender] = useState(true);
const isSkippingOpenReport = useRef(false);
const flatListRef = useRef<FlatList>(null);
const hasCreatedLegacyThreadRef = useRef(false);
const {isBetaEnabled} = usePermissions();
const {isOffline} = useNetwork();
Expand Down Expand Up @@ -317,7 +316,6 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
const [childReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${linkedAction?.childReportID}`, {canBeMissing: true});

const [isBannerVisible, setIsBannerVisible] = useState(true);
const [scrollPosition, setScrollPosition] = useState<ScrollPosition>({});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the above state into a reusable hook to use on this page and in SearchMoneyRequestReportPage.tsx.


const viewportOffsetTop = useViewportOffsetTop();

Expand Down Expand Up @@ -652,6 +650,9 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr

// We need to cancel telemetry span when user leaves the screen before full report data is loaded
cancelSpan(`${CONST.TELEMETRY.SPAN_OPEN_REPORT}_${reportID}`);

// Cancel any pending send-message spans to prevent orphaned spans when navigating away
cancelSpansByPrefix(CONST.TELEMETRY.SPAN_SEND_MESSAGE);
};
}, [reportID]);

Expand Down Expand Up @@ -876,7 +877,7 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
};
}, [report?.reportID, didSubscribeToReportLeavingEvents, reportIDFromRoute, report?.pendingFields, currentUserAccountID]);

const actionListValue = useMemo((): ActionListContextType => ({flatListRef, scrollPosition, setScrollPosition}), [flatListRef, scrollPosition, setScrollPosition]);
const actionListValue = useActionListContextValue();

// This helps in tracking from the moment 'route' triggers useMemo until isLoadingInitialReportActions becomes true. It prevents blinking when loading reportActions from cache.
useEffect(() => {
Expand Down
3 changes: 2 additions & 1 deletion src/pages/inbox/ReportScreenContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ type ActionListContextType = {
flatListRef: FlatListRefType;
scrollPosition: ScrollPosition | null;
setScrollPosition: (position: {offset: number}) => void;
scrollOffsetRef: RefObject<number>;
};
type ReactionListContextType = RefObject<ReactionListRef | null> | null;

const ActionListContext = createContext<ActionListContextType>({flatListRef: null, scrollPosition: null, setScrollPosition: () => {}});
const ActionListContext = createContext<ActionListContextType>({flatListRef: null, scrollPosition: null, setScrollPosition: () => {}, scrollOffsetRef: {current: 0}});
const ReactionListContext = createContext<ReactionListContextType>(null);

export {ActionListContext, ReactionListContext};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import lodashDebounce from 'lodash/debounce';
import noop from 'lodash/noop';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import React, {memo, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
import type {BlurEvent, MeasureInWindowOnSuccessCallback, TextInputSelectionChangeEvent} from 'react-native';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -38,6 +38,7 @@ import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import DomUtils from '@libs/DomUtils';
import FS from '@libs/Fullstory';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {rand64} from '@libs/NumberUtils';
import Performance from '@libs/Performance';
import {getLinkedTransactionID, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {
Expand All @@ -64,6 +65,7 @@ import willBlurTextInputOnTapOutsideFunc from '@libs/willBlurTextInputOnTapOutsi
import AgentZeroProcessingRequestIndicator from '@pages/inbox/report/AgentZeroProcessingRequestIndicator';
import ParticipantLocalTime from '@pages/inbox/report/ParticipantLocalTime';
import ReportTypingIndicator from '@pages/inbox/report/ReportTypingIndicator';
import {ActionListContext} from '@pages/inbox/ReportScreenContext';
import {hideEmojiPicker, isActive as isActiveEmojiPickerAction, isEmojiPickerVisible} from '@userActions/EmojiPickerAction';
import {addAttachmentWithComment, setIsComposerFullSize} from '@userActions/Report';
import {isBlockedFromConcierge as isBlockedFromConciergeUserAction} from '@userActions/User';
Expand Down Expand Up @@ -91,7 +93,7 @@ type SuggestionsRef = {

type ReportActionComposeProps = Pick<ComposerWithSuggestionsProps, 'reportID' | 'isComposerFullSize' | 'lastReportAction'> & {
/** A method to call when the form is submitted */
onSubmit: (newComment: string) => void;
onSubmit: (newComment: string, reportActionID?: string) => void;

/** The report currently being looked at */
report: OnyxEntry<OnyxTypes.Report>;
Expand Down Expand Up @@ -165,6 +167,8 @@ function ReportActionCompose({
canBeMissing: true,
});
const ancestors = useAncestors(transactionThreadReport ?? report);
const {scrollOffsetRef} = useContext(ActionListContext);

/**
* Updates the Highlight state of the composer
*/
Expand Down Expand Up @@ -355,16 +359,22 @@ function ReportActionCompose({
});
attachmentFileRef.current = null;
} else {
Performance.markStart(CONST.TIMING.SEND_MESSAGE, {message: newCommentTrimmed});
startSpan(CONST.TELEMETRY.SPAN_SEND_MESSAGE, {
name: 'send-message',
op: CONST.TELEMETRY.SPAN_SEND_MESSAGE,
attributes: {
[CONST.TELEMETRY.ATTRIBUTE_REPORT_ID]: reportID,
[CONST.TELEMETRY.ATTRIBUTE_MESSAGE_LENGTH]: newCommentTrimmed.length,
},
});
onSubmit(newCommentTrimmed);
const reportActionID = rand64();

// The list is inverted, so an offset near 0 means the user is at the bottom (newest messages visible).
const isScrolledToBottom = scrollOffsetRef.current < CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid starting send spans with stale scroll offset

isScrolledToBottom depends on scrollOffsetRef.current, but that ref is only written in src/pages/inbox/report/ReportActionsList.tsx (I verified via rg "scrollOffsetRef.current ="), not in src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx which is used when shouldDisplayMoneyRequestActionsList is true in ReportScreen. In those money-request reports the value remains its default 0, so this condition still starts ManualSendMessage_<actionID> spans even when the user is scrolled away from the latest message; since spans are now ended by exact reportActionID, off-screen sent messages can remain open until navigation cleanup and skew send-message telemetry.

Useful? React with 👍 / 👎.

if (isScrolledToBottom) {
Performance.markStart(CONST.TIMING.SEND_MESSAGE, {message: newCommentTrimmed});
startSpan(`${CONST.TELEMETRY.SPAN_SEND_MESSAGE}_${reportActionID}`, {
Comment on lines +367 to +368

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip starting send spans for task-shortcut submissions

This starts a send-message span before calling onSubmit, but onSubmitComment can return early when handleCreateTask(text) succeeds in ReportFooter, so no comment with that reportActionID is created and TextCommentFragment never calls endSpan for it. In that task-creation path the span is only canceled later during screen cleanup, which produces canceled/orphan send-message telemetry samples.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fine, we only want to measure the text messages

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Delay send span start until comment is actually posted

startSpan() is called before onSubmit(), but onSubmit can short-circuit without posting a comment (for example, ReportFooter.onSubmitComment() returns early when handleCreateTask(text) is true in src/pages/inbox/report/ReportFooter.tsx). In that flow no report action with this generated reportActionID is created, so TextCommentFragment never calls endSpan(...) for it; the span stays active until a later screen teardown and inflates send-message telemetry during long sessions.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cleanup when the user moves away from the report page, so we won't have inflated spans

name: 'send-message',
op: CONST.TELEMETRY.SPAN_SEND_MESSAGE,
attributes: {
[CONST.TELEMETRY.ATTRIBUTE_REPORT_ID]: reportID,
[CONST.TELEMETRY.ATTRIBUTE_MESSAGE_LENGTH]: newCommentTrimmed.length,
},
});
}
onSubmit(newCommentTrimmed, reportActionID);
}
},
[
Expand All @@ -378,6 +388,7 @@ function ReportActionCompose({
personalDetail.timezone,
isInSidePanel,
onSubmit,
scrollOffsetRef,
],
);

Expand Down
5 changes: 4 additions & 1 deletion src/pages/inbox/report/ReportActionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type {ListRenderItemInfo} from '@react-native/virtualized-lists/Lists/Vir
import {useIsFocused, useRoute} from '@react-navigation/native';
import {isUserValidatedSelector} from '@selectors/Account';
import {tierNameSelector} from '@selectors/UserWallet';
import React, {memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react';
import React, {memo, useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react';
import type {LayoutChangeEvent, NativeScrollEvent, NativeSyntheticEvent} from 'react-native';
import {DeviceEventEmitter, InteractionManager, View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -61,6 +61,7 @@ import {
} from '@libs/ReportUtils';
import Visibility from '@libs/Visibility';
import type {ReportsSplitNavigatorParamList} from '@navigation/types';
import {ActionListContext} from '@pages/inbox/ReportScreenContext';
import variables from '@styles/variables';
import {openReport, readNewestAction, subscribeToNewActionEvent} from '@userActions/Report';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -172,6 +173,7 @@ function ReportActionsList({
const {isOffline, lastOfflineAt, lastOnlineAt} = useNetworkWithOfflineStatus();
const route = useRoute<PlatformStackRouteProp<ReportsSplitNavigatorParamList, typeof SCREENS.REPORT>>();
const reportScrollManager = useReportScrollManager();
const {scrollOffsetRef} = useContext(ActionListContext);
const userActiveSince = useRef<string>(DateUtils.getDBTime());
const lastMessageTime = useRef<string | null>(null);
const [isVisible, setIsVisible] = useState(Visibility.isVisible);
Expand Down Expand Up @@ -365,6 +367,7 @@ function ReportActionsList({
isInverted: true,
onTrackScrolling: (event: NativeSyntheticEvent<NativeScrollEvent>) => {
scrollingVerticalOffset.current = event.nativeEvent.contentOffset.y;
scrollOffsetRef.current = event.nativeEvent.contentOffset.y;
onScroll?.(event);
if (shouldScrollToEndAfterLayout && (!hasCreatedActionAdded || isOffline)) {
setShouldScrollToEndAfterLayout(false);
Expand Down
3 changes: 2 additions & 1 deletion src/pages/inbox/report/ReportFooter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ function ReportFooter({
const targetReportAncestors = useAncestors(targetReport);

const onSubmitComment = useCallback(
(text: string) => {
(text: string, reportActionID?: string) => {
const isTaskCreated = handleCreateTask(text);
if (isTaskCreated) {
return;
Expand All @@ -203,6 +203,7 @@ function ReportFooter({
currentUserAccountID: personalDetail.accountID,
shouldPlaySound: true,
isInSidePanel,
reportActionID,
});
},
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
7 changes: 6 additions & 1 deletion src/pages/inbox/report/comment/TextCommentFragment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,13 @@ function TextCommentFragment({fragment, styleAsDeleted, reportActionID, styleAsM

useEffect(() => {
Performance.markEnd(CONST.TIMING.SEND_MESSAGE, {message: text});
endSpan(CONST.TELEMETRY.SPAN_SEND_MESSAGE);
}, [text]);
useEffect(() => {
if (!reportActionID) {
return;
}
endSpan(`${CONST.TELEMETRY.SPAN_SEND_MESSAGE}_${reportActionID}`);
}, [reportActionID]);

// If the only difference between fragment.text and fragment.html is <br /> tags and emoji tag
// on native, we render it as text, not as html
Expand Down
2 changes: 1 addition & 1 deletion tests/perf-test/ReportActionsList.perf-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ beforeAll(() =>
const mockOnLayout = jest.fn();
const mockOnScroll = jest.fn();
const mockLoadChats = jest.fn();
const mockRef = {current: null, flatListRef: null, scrollPosition: null, setScrollPosition: () => {}};
const mockRef = {current: null, flatListRef: null, scrollPosition: null, setScrollPosition: () => {}, scrollOffsetRef: {current: 0}};

const TEST_USER_ACCOUNT_ID = 1;
const TEST_USER_LOGIN = 'test@test.com';
Expand Down
Loading