HIGH: (Comment linking: step 2) [23220] WEB maintain visible content position

.github/workflows/reassurePerformanceTests.yml

@@ -21,7 +21,6 @@ jobs:
         run: |
             git config --global user.email "[email protected]"
             git config --global user.name "Test"
-
       - name: Run performance testing script
         shell: bash
         run: |
@@ -35,7 +34,6 @@ jobs:
           git merge --no-commit --allow-unrelated-histories "$BASELINE_BRANCH" -X ours
           npm install --force
           npx reassure --branch
-
       - name: Read output.json
         id: reassure
         uses: juliangruber/read-file-action@v1

src/components/FlatList/MVCPFlatList.js

@@ -0,0 +1,207 @@
+/* eslint-disable es/no-optional-chaining, es/no-nullish-coalescing-operators, react/prop-types */
+import PropTypes from 'prop-types';
+import React from 'react';
+import {FlatList} from 'react-native';
+
+function mergeRefs(...args) {
+    return function forwardRef(node) {
+        args.forEach((ref) => {
+            if (ref == null) {
+                return;
+            }
+            if (typeof ref === 'function') {
+                ref(node);
+                return;
+            }
+            if (typeof ref === 'object') {
+                // eslint-disable-next-line no-param-reassign
+                ref.current = node;
+                return;
+            }
+            console.error(`mergeRefs cannot handle Refs of type boolean, number or string, received ref ${String(ref)}`);
+        });
+    };
+}
+
+function useMergeRefs(...args) {
+    return React.useMemo(
+        () => mergeRefs(...args),
+        // eslint-disable-next-line
+        [...args],
+    );
+}
+
+const MVCPFlatList = React.forwardRef(({maintainVisibleContentPosition, horizontal, onScroll, ...props}, forwardedRef) => {
+    const {minIndexForVisible: mvcpMinIndexForVisible, autoscrollToTopThreshold: mvcpAutoscrollToTopThreshold} = maintainVisibleContentPosition ?? {};
+    const scrollRef = React.useRef(null);
+    const prevFirstVisibleOffsetRef = React.useRef(null);
+    const firstVisibleViewRef = React.useRef(null);
+    const mutationObserverRef = React.useRef(null);
+    const lastScrollOffsetRef = React.useRef(0);
+
+    const getScrollOffset = React.useCallback(() => {
+        if (scrollRef.current == null) {
+            return 0;
+        }
+        return horizontal ? scrollRef.current.getScrollableNode().scrollLeft : scrollRef.current.getScrollableNode().scrollTop;
+    }, [horizontal]);
+
+    const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode().childNodes[0], []);
+
+    const scrollToOffset = React.useCallback(
+        (offset, animated) => {
+            const behavior = animated ? 'smooth' : 'instant';
+            scrollRef.current?.getScrollableNode().scroll(horizontal ? {left: offset, behavior} : {top: offset, behavior});
+        },
+        [horizontal],
+    );
+
+    const prepareForMaintainVisibleContentPosition = React.useCallback(() => {
+        if (mvcpMinIndexForVisible == null) {
+            return;
+        }
+
+        const contentView = getContentView();
+        if (contentView == null) {
+            return;
+        }
+
+        const scrollOffset = getScrollOffset();
+
+        const contentViewLength = contentView.childNodes.length;
+        for (let i = mvcpMinIndexForVisible; i < contentViewLength; i++) {
+            const subview = contentView.childNodes[i];
+            const subviewOffset = horizontal ? subview.offsetLeft : subview.offsetTop;
+            if (subviewOffset > scrollOffset || i === contentViewLength - 1) {
+                prevFirstVisibleOffsetRef.current = subviewOffset;
+                firstVisibleViewRef.current = subview;
+                break;
+            }
+        }
+    }, [getContentView, getScrollOffset, mvcpMinIndexForVisible, horizontal]);
+
+    const adjustForMaintainVisibleContentPosition = React.useCallback(() => {
+        if (mvcpMinIndexForVisible == null) {
+            return;
+        }
+
+        const firstVisibleView = firstVisibleViewRef.current;
+        const prevFirstVisibleOffset = prevFirstVisibleOffsetRef.current;
+        if (firstVisibleView == null || prevFirstVisibleOffset == null) {
+            return;
+        }
+
+        const firstVisibleViewOffset = horizontal ? firstVisibleView.offsetLeft : firstVisibleView.offsetTop;
+        const delta = firstVisibleViewOffset - prevFirstVisibleOffset;
+        if (Math.abs(delta) > 0.5) {
+            const scrollOffset = getScrollOffset();
+            prevFirstVisibleOffsetRef.current = firstVisibleViewOffset;
+            scrollToOffset(scrollOffset + delta, false);
+            if (mvcpAutoscrollToTopThreshold != null && scrollOffset <= mvcpAutoscrollToTopThreshold) {
+                scrollToOffset(0, true);
+            }
+        }
+    }, [getScrollOffset, scrollToOffset, mvcpMinIndexForVisible, mvcpAutoscrollToTopThreshold, horizontal]);
+
+    const setupMutationObserver = React.useCallback(() => {
+        const contentView = getContentView();
+        if (contentView == null) {
+            return;
+        }
+
+        mutationObserverRef.current?.disconnect();
+
+        const mutationObserver = new MutationObserver(() => {
+            // This needs to execute after scroll events are dispatched, but
+            // in the same tick to avoid flickering. rAF provides the right timing.
+            requestAnimationFrame(() => {
+                // Chrome adjusts scroll position when elements are added at the top of the
+                // view. We want to have the same behavior as react-native / Safari so we
+                // reset the scroll position to the last value we got from an event.
+                const lastScrollOffset = lastScrollOffsetRef.current;
+                const scrollOffset = getScrollOffset();
+                if (lastScrollOffset !== scrollOffset) {
+                    scrollToOffset(lastScrollOffset, false);
+                }
+
+                adjustForMaintainVisibleContentPosition();
+            });
+        });
+        mutationObserver.observe(contentView, {
+            attributes: true,
+            childList: true,
+            subtree: true,
+        });
+
+        mutationObserverRef.current = mutationObserver;
+    }, [adjustForMaintainVisibleContentPosition, getContentView, getScrollOffset, scrollToOffset]);
+
+    React.useEffect(() => {
+        requestAnimationFrame(() => {
+            prepareForMaintainVisibleContentPosition();
+            setupMutationObserver();
+        });
+    }, [prepareForMaintainVisibleContentPosition, setupMutationObserver]);
+
+    const setMergedRef = useMergeRefs(scrollRef, forwardedRef);
+
+    const onRef = React.useCallback(
+        (newRef) => {
+            // Make sure to only call refs and re-attach listeners if the node changed.
+            if (newRef == null || newRef === scrollRef.current) {
+                return;
+            }
+
+            setMergedRef(newRef);
+            prepareForMaintainVisibleContentPosition();
+            setupMutationObserver();
+        },
+        [prepareForMaintainVisibleContentPosition, setMergedRef, setupMutationObserver],
+    );
+
+    React.useEffect(() => {
+        const mutationObserver = mutationObserverRef.current;
+        return () => {
+            mutationObserver?.disconnect();
+        };
+    }, []);
+
+    const onScrollInternal = React.useCallback(
+        (ev) => {
+            lastScrollOffsetRef.current = getScrollOffset();
+
+            prepareForMaintainVisibleContentPosition();
+
+            onScroll?.(ev);
+        },
+        [getScrollOffset, prepareForMaintainVisibleContentPosition, onScroll],
+    );
+
+    return (
+        <FlatList
+            // eslint-disable-next-line react/jsx-props-no-spreading
+            {...props}
+            maintainVisibleContentPosition={maintainVisibleContentPosition}
+            horizontal={horizontal}
+            onScroll={onScrollInternal}
+            scrollEventThrottle={1}
+            ref={onRef}
+        />
+    );
+});
+
+MVCPFlatList.displayName = 'MVCPFlatList';
+MVCPFlatList.propTypes = {
+    maintainVisibleContentPosition: PropTypes.shape({
+        minIndexForVisible: PropTypes.number.isRequired,
+        autoscrollToTopThreshold: PropTypes.number,
+    }),
+    horizontal: PropTypes.bool,
+};
+
+MVCPFlatList.defaultProps = {
+    maintainVisibleContentPosition: null,
+    horizontal: false,
+};
+
+export default MVCPFlatList;

src/components/FlatList/index.web.js

@@ -0,0 +1,3 @@
+import MVCPFlatList from './MVCPFlatList';
+
+export default MVCPFlatList;

src/components/InvertedFlatList/BaseInvertedFlatList.tsx

@@ -3,7 +3,6 @@ import React, {forwardRef} from 'react';
 import type {FlatListProps} from 'react-native';
 import FlatList from '@components/FlatList';
 
-const AUTOSCROLL_TO_TOP_THRESHOLD = 128;
 const WINDOW_SIZE = 15;
 
 function BaseInvertedFlatList<T>(props: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
@@ -15,7 +14,6 @@ function BaseInvertedFlatList<T>(props: FlatListProps<T>, ref: ForwardedRef<Flat
             windowSize={WINDOW_SIZE}
             maintainVisibleContentPosition={{
                 minIndexForVisible: 0,
-                autoscrollToTopThreshold: AUTOSCROLL_TO_TOP_THRESHOLD,
             }}
             inverted
         />

src/components/InvertedFlatList/index.native.tsx

@@ -1,18 +1,15 @@
 import type {ForwardedRef} from 'react';
 import React, {forwardRef} from 'react';
 import type {FlatList, FlatListProps} from 'react-native';
-import useThemeStyles from '@hooks/useThemeStyles';
 import BaseInvertedFlatList from './BaseInvertedFlatList';
 import CellRendererComponent from './CellRendererComponent';
 
 function BaseInvertedFlatListWithRef<T>(props: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
-    const styles = useThemeStyles();
     return (
         <BaseInvertedFlatList
             // eslint-disable-next-line react/jsx-props-no-spreading
             {...props}
             ref={ref}
-            contentContainerStyle={styles.justifyContentEnd}
             CellRendererComponent={CellRendererComponent}
             /**
              * To achieve absolute positioning and handle overflows for list items, the property must be disabled

src/components/InvertedFlatList/index.tsx

@@ -4,10 +4,11 @@ import type {FlatList, FlatListProps, NativeScrollEvent, NativeSyntheticEvent} f
 import {DeviceEventEmitter} from 'react-native';
 import CONST from '@src/CONST';
 import BaseInvertedFlatList from './BaseInvertedFlatList';
+import CellRendererComponent from './CellRendererComponent';
 
 // This is adapted from https://codesandbox.io/s/react-native-dsyse
 // It's a HACK alert since FlatList has inverted scrolling on web
-function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, contentContainerStyle, ...props}: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
+function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, ...props}: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
     const lastScrollEvent = useRef<number | null>(null);
     const scrollEndTimeout = useRef<NodeJS.Timeout | null>(null);
     const updateInProgress = useRef<boolean>(false);
@@ -86,8 +87,8 @@ function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, contentContaine
             // eslint-disable-next-line react/jsx-props-no-spreading
             {...props}
             ref={ref}
-            contentContainerStyle={contentContainerStyle}
             onScroll={handleScroll}
+            CellRendererComponent={CellRendererComponent}
         />
     );
 }

src/pages/home/ReportScreen.js

@@ -298,13 +298,6 @@ function ReportScreen({
     const onSubmitComment = useCallback(
         (text) => {
             Report.addComment(getReportID(route), text);
-
-            // We need to scroll to the bottom of the list after the comment is added
-            const refID = setTimeout(() => {
-                flatListRef.current.scrollToOffset({animated: false, offset: 0});
-            }, 10);
-
-            return () => clearTimeout(refID);
         },
         [route],
     );

src/pages/home/report/ReportActionsList.js

@@ -2,7 +2,7 @@ import {useRoute} from '@react-navigation/native';
 import lodashGet from 'lodash/get';
 import PropTypes from 'prop-types';
 import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
-import {DeviceEventEmitter} from 'react-native';
+import {DeviceEventEmitter, InteractionManager} from 'react-native';
 import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
 import _ from 'underscore';
 import InvertedFlatList from '@components/InvertedFlatList';
@@ -138,7 +138,6 @@ function ReportActionsList({
     isComposerFullSize,
 }) {
     const styles = useThemeStyles();
-    const reportScrollManager = useReportScrollManager();
     const {translate} = useLocalize();
     const {isOffline} = useNetwork();
     const route = useRoute();
@@ -151,6 +150,7 @@ function ReportActionsList({
         }
         return cacheUnreadMarkers.get(report.reportID);
     };
+    const reportScrollManager = useReportScrollManager();
     const [currentUnreadMarker, setCurrentUnreadMarker] = useState(markerInit);
     const scrollingVerticalOffset = useRef(0);
     const readActionSkipped = useRef(false);
@@ -252,6 +252,13 @@ function ReportActionsList({
         });
     }, [report.reportID]);
 
+    useEffect(() => {
+        InteractionManager.runAfterInteractions(() => {
+            reportScrollManager.scrollToBottom();
+        });
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
+
     useEffect(() => {
         // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function?
         // Answer: On web, when navigating to another report screen, the previous report screen doesn't get unmounted,
@@ -273,7 +280,7 @@ function ReportActionsList({
             if (!isFromCurrentUser) {
                 return;
             }
-            reportScrollManager.scrollToBottom();
+            InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom());
         });
 
         const cleanup = () => {

src/styles/index.ts

@@ -1699,7 +1699,6 @@ const styles = (theme: ThemeColors) =>
         },
 
         chatContentScrollView: {
-            flexGrow: 1,
             justifyContent: 'flex-end',
             paddingBottom: 16,
         },