Merge pull request Expensify#72302 from callstack-internal/bugfix/report-preview-jumping

Bugfix/report preview jumping

Author: tgolen

src/components/InvertedFlatList/BaseInvertedFlatList/RenderTaskQueue.tsx

@@ -13,6 +13,12 @@ class RenderTaskQueue {
 
     private timeout: NodeJS.Timeout | null = null;
 
+    private onIsRenderingChange?: (isRendering: boolean) => void;
+
+    constructor(onIsRenderingChange?: (isRendering: boolean) => void) {
+        this.onIsRenderingChange = onIsRenderingChange;
+    }
+
     add(info: RenderInfo) {
         this.renderInfos.push(info);
 
@@ -30,15 +36,18 @@ class RenderTaskQueue {
             return;
         }
         clearTimeout(this.timeout);
+        this.onIsRenderingChange?.(false);
     }
 
     private render() {
         const info = this.renderInfos.shift();
         if (!info) {
             this.isRendering = false;
+            this.onIsRenderingChange?.(false);
             return;
         }
         this.isRendering = true;
+        this.onIsRenderingChange?.(true);
 
         this.handler(info);
 

src/components/InvertedFlatList/BaseInvertedFlatList/index.tsx

@@ -43,6 +43,8 @@ function BaseInvertedFlatList<T>({ref, ...props}: BaseInvertedFlatListProps<T>)
         return null;
     });
     const [isInitialData, setIsInitialData] = useState(true);
+    const [isQueueRendering, setIsQueueRendering] = useState(false);
+
     const currentDataIndex = useMemo(() => (currentDataId === null ? 0 : data.findIndex((item, index) => keyExtractor(item, index) === currentDataId)), [currentDataId, data, keyExtractor]);
     const displayedData = useMemo(() => {
         if (currentDataIndex <= 0) {
@@ -56,7 +58,7 @@ function BaseInvertedFlatList<T>({ref, ...props}: BaseInvertedFlatListProps<T>)
     const dataIndexDifference = data.length - displayedData.length;
 
     // Queue up updates to the displayed data to avoid adding too many at once and cause jumps in the list.
-    const renderQueue = useMemo(() => new RenderTaskQueue(), []);
+    const renderQueue = useMemo(() => new RenderTaskQueue(setIsQueueRendering), []);
     useEffect(() => {
         return () => {
             renderQueue.cancel();
@@ -88,6 +90,10 @@ function BaseInvertedFlatList<T>({ref, ...props}: BaseInvertedFlatListProps<T>)
     );
 
     const maintainVisibleContentPosition = useMemo(() => {
+        if (!initialScrollKey && (!isInitialData || !isQueueRendering)) {
+            return undefined;
+        }
+
         const config: ScrollViewProps['maintainVisibleContentPosition'] = {
             // This needs to be 1 to avoid using loading views as anchors.
             minIndexForVisible: data.length ? Math.min(1, data.length - 1) : 0,
@@ -98,7 +104,7 @@ function BaseInvertedFlatList<T>({ref, ...props}: BaseInvertedFlatListProps<T>)
         }
 
         return config;
-    }, [data.length, shouldEnableAutoScrollToTopThreshold, isLoadingData, wasLoadingData]);
+    }, [initialScrollKey, isInitialData, isQueueRendering, data.length, shouldEnableAutoScrollToTopThreshold, isLoadingData, wasLoadingData]);
 
     const listRef = useRef<RNFlatList | null>(null);
     useImperativeHandle(ref, () => {

tests/unit/RenderTaskQueueTest.ts

@@ -0,0 +1,51 @@
+import RenderTaskQueue from '../../src/components/InvertedFlatList/BaseInvertedFlatList/RenderTaskQueue';
+
+jest.unmock('../../src/components/InvertedFlatList/BaseInvertedFlatList/RenderTaskQueue');
+
+describe('RenderTaskQueue', () => {
+    beforeEach(() => {
+        jest.useFakeTimers();
+    });
+
+    afterEach(() => {
+        jest.clearAllTimers();
+        jest.useRealTimers();
+    });
+
+    describe('notifyRenderingStateChange callback', () => {
+        it('should notify rendering state changes when a task completes naturally to track the rendering lifecycle', () => {
+            // Given a RenderTaskQueue with an isRendering change callback
+            const mockOnIsRenderingChange = jest.fn();
+            const queue = new RenderTaskQueue(mockOnIsRenderingChange);
+
+            // When a task is added and allowed to complete
+            queue.add({distanceFromStart: 100});
+
+            // Then the callback is invoked with true when rendering starts
+            expect(mockOnIsRenderingChange).toHaveBeenCalledWith(true);
+            jest.advanceTimersByTime(500);
+
+            // Then the callback is invoked with false when rendering completes
+            expect(mockOnIsRenderingChange).toHaveBeenCalledTimes(2);
+            expect(mockOnIsRenderingChange).toHaveBeenCalledWith(false);
+        });
+
+        it('should notify rendering state changes when a task is canceled to ensure proper cleanup', () => {
+            // Given a RenderTaskQueue with an isRendering change callback
+            const mockOnIsRenderingChange = jest.fn();
+            const queue = new RenderTaskQueue(mockOnIsRenderingChange);
+
+            // When a task is added but canceled before completion
+            queue.add({distanceFromStart: 100});
+            queue.cancel();
+
+            // Then the callback is invoked with true when rendering starts
+            expect(mockOnIsRenderingChange).toHaveBeenCalledWith(true);
+            jest.advanceTimersByTime(500);
+
+            // Then the callback is invoked with false even after canceling to ensure proper cleanup
+            expect(mockOnIsRenderingChange).toHaveBeenCalledTimes(2);
+            expect(mockOnIsRenderingChange).toHaveBeenCalledWith(false);
+        });
+    });
+});