[MM-63799] Fix the high number of detached nodes in post list in dynamic-virtualized-list (#30864)

e2e-tests/cypress/tests/integration/channels/collapsed_reply_threads/replies_spec.ts

@@ -71,7 +71,7 @@ describe('Collapsed Reply Threads', () => {
         cy.uiGetPostThreadFooter(rootPost.id).should('not.exist');
 
         // # Post a reply post as current user
-        cy.postMessageAs({sender: testUser, message: 'reply!', channelId: testChannel.id, rootId: rootPost.id});
+        cy.postMessageAs({sender: testUser, message: 'reply to root post', channelId: testChannel.id, rootId: rootPost.id});
 
         // # Get thread footer of last post
         cy.uiGetPostThreadFooter(rootPost.id).within(() => {

e2e-tests/cypress/tests/integration/channels/commands/leave_channel_spec.ts

@@ -27,7 +27,8 @@ describe('Leave Channel Command', () => {
     it('Should be redirected to last channel when user leaves channel with /leave command', () => {
         // # Go to newly created channel
         cy.get('#sidebarItem_' + testChannel.name).click({force: true});
-        cy.findAllByTestId('postView').should('be.visible');
+
+        cy.findAllByTestId('postView').last().scrollIntoView().should('be.visible');
 
         // # Post /leave command in center channel
         cy.postMessage('/leave ');

e2e-tests/cypress/tests/integration/channels/team_settings/teams_spec.js

@@ -139,6 +139,9 @@ describe('Teams Suite', () => {
             });
 
             cy.get('#sidebarItem_off-topic').should('be.visible').click({force: true});
+
+            cy.findAllByTestId('postView').last().scrollIntoView();
+
             cy.getLastPost().wait(TIMEOUTS.HALF_SEC).then(($el) => {
                 cy.wrap($el).get('.user-popover').
                     should('be.visible').

e2e-tests/cypress/tests/support/ui/sidebar_left.ts

@@ -139,7 +139,7 @@ Cypress.Commands.add('uiClickSidebarItem', (name) => {
         });
         cy.get('#tutorial-threads-mobile-header span.Button_label').contains('Followed threads');
     } else {
-        cy.findAllByTestId('postView').should('be.visible');
+        cy.findAllByTestId('postView').last().scrollIntoView().should('be.visible');
     }
 });
 

webapp/channels/src/components/dynamic_virtualized_list/dynamic_virtualized_list.scss

@@ -0,0 +1,17 @@
+.dynamic_virtualized_list {
+    position: absolute;
+    bottom: 0;
+    width: 100%;
+    max-height: 100%;
+    overflow-anchor: none;
+    overflow-y: auto;
+    will-change: transform;
+
+    .innerList[role='list'] {
+        padding: 14px 0px 7px;
+    }
+
+    .listItem[role='listitem'] {
+        position: relative;
+    }
+}

webapp/channels/src/components/dynamic_virtualized_list/index.jsx

@@ -1,13 +1,13 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-/* eslint-disable react/prop-types */
 /* eslint-disable no-underscore-dangle */
+/* eslint-disable react/prop-types */
 
 import memoizeOne from 'memoize-one';
 import {createElement, PureComponent} from 'react';
 
-import ItemMeasurer from './item_measurer';
+import ListItem from './item_row_shared';
 
 const atBottomMargin = 10;
 
@@ -612,11 +612,9 @@ export class DynamicVirtualizedList extends PureComponent {
     };
 
     _renderItems = () => {
-        const {children, direction, itemData, loaderId, visibleId} =
-            this.props;
         const width = this.innerRefWidth;
         const [startIndex, stopIndex] = this._getRangeToRender();
-        const itemCount = itemData.length;
+        const itemCount = this.props.itemData.length;
         const items = [];
         if (itemCount > 0) {
             for (let index = itemCount - 1; index >= 0; index--) {
@@ -636,9 +634,9 @@ export class DynamicVirtualizedList extends PureComponent {
                     index < localOlderPostsToRenderStopIndex + 1 &&
                     localOlderPostsToRenderStartIndex === stopIndex + 1;
 
-                const isLoader = itemData[index] === loaderId;
+                const isLoader = this.props.itemData[index] === this.props.loaderId;
-                const isVisible = itemData[index] === visibleId;
+                const isVisible = this.props.itemData[index] === this.props.visibleId;
-                const itemId = itemData[index];
+                const itemId = this.props.itemData[index];
 
                 // It's important to read style after fetching item metadata.
                 // getItemMetadata() will clear stale styles.
@@ -649,24 +647,22 @@ export class DynamicVirtualizedList extends PureComponent {
                     isLoader ||
                     isVisible
                 ) {
-                    const item = createElement(children, {
+                    const item = createElement(this.props.children, {
-                        data: itemData,
+                        data: this.props.itemData,
                         itemId,
                     });
 
-                    // Always wrap children in a ItemMeasurer to detect changes in size.
+                    // Always wrap children in a ItemRow to detect changes in size.
                     items.push(
-                        createElement(ItemMeasurer, {
+                        createElement(ListItem, {
-                            direction,
+                            key: itemId,
-                            handleNewMeasurements: this._handleNewMeasurements,
                             index,
                             item,
-                            key: itemId,
-                            size,
                             itemId,
+                            height: size,
                             width,
+                            onHeightChange: this._handleNewMeasurements,
                             onUnmount: this._onItemRowUnmount,
-                            itemCount,
                         }),
                     );
                 } else {
@@ -718,6 +714,7 @@ export class DynamicVirtualizedList extends PureComponent {
                 {
                     ref: innerRef,
                     role: 'list',
+                    className: 'innerList',
                     style: innerListStyle,
                 },
                 items,

webapp/channels/src/components/dynamic_virtualized_list/item_measurer.jsx

@@ -1,202 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-/* eslint-disable react/prop-types */
-/* eslint-disable no-underscore-dangle */
-
-import React, {PureComponent} from 'react';
-import {findDOMNode} from 'react-dom';
-
-import {isSafari} from 'utils/user_agent';
-
-const scrollBarWidth = 8;
-const scrollableContainerStyles = {
-    display: 'inline',
-    width: '0px',
-    height: '0px',
-    zIndex: '-1',
-    overflow: 'hidden',
-    margin: '0px',
-    padding: '0px',
-};
-
-const scrollableWrapperStyle = {
-    position: 'absolute',
-    flex: '0 0 auto',
-    overflow: 'hidden',
-    visibility: 'hidden',
-    zIndex: '-1',
-    width: '100%',
-    height: '100%',
-    left: '0px',
-    top: '0px',
-};
-
-const expandShrinkContainerStyles = {
-    flex: '0 0 auto',
-    overflow: 'hidden',
-    zIndex: '-1',
-    visibility: 'hidden',
-    left: `-${scrollBarWidth + 1}px`, //8px(scrollbar width) + 1px
-    bottom: `-${scrollBarWidth}px`, //8px because of scrollbar width
-    right: `-${scrollBarWidth}px`, //8px because of scrollbar width
-    top: `-${scrollBarWidth + 1}px`, //8px(scrollbar width) + 1px
-};
-
-const expandShrinkStyles = {
-    position: 'absolute',
-    flex: '0 0 auto',
-    visibility: 'hidden',
-    overflow: 'scroll',
-    zIndex: '-1',
-    width: '100%',
-    height: '100%',
-};
-
-const shrinkChildStyle = {
-    position: 'absolute',
-    height: '200%',
-    width: '200%',
-};
-
-//values below need to be changed when scrollbar width changes
-const shrinkScrollDelta = (2 * scrollBarWidth) + 1; // 17 = 2* scrollbar width(8px) + 1px as buffer
-
-// 27 = 2* scrollbar width(8px) + 1px as buffer + 10px(this value is based of off lib(Link below). Probably not needed but doesnt hurt to leave)
-//https://github.com/wnr/element-resize-detector/blob/27983e59dce9d8f1296d8f555dc2340840fb0804/src/detection-strategy/scroll.js#L246
-const expandScrollDelta = shrinkScrollDelta + 10;
-
-export default class ItemMeasurer extends PureComponent {
-    _node = null;
-    _resizeSensorExpand = React.createRef();
-    _resizeSensorShrink = React.createRef();
-    _positionScrollbarsRef = null;
-    _measureItemAnimFrame = null;
-
-    componentDidMount() {
-        // eslint-disable-next-line react/no-find-dom-node
-        this._node = findDOMNode(this);
-
-        // Force sync measure for the initial mount.
-        // This is necessary to support the DynamicSizeList layout logic.
-        if (isSafari() && this.props.size) {
-            this._measureItemAnimFrame = requestAnimationFrame(() => {
-                this._measureItem(false);
-            });
-        } else {
-            this._measureItem(false);
-        }
-
-        if (this.props.size) {
-            // Don't wait for positioning scrollbars when we have size
-            // This is needed triggering an event for remounting a post
-            this.positionScrollBars();
-        }
-    }
-
-    componentDidUpdate(prevProps) {
-        if ((prevProps.size === 0 && this.props.size !== 0) || prevProps.size !== this.props.size) {
-            this.positionScrollBars();
-        }
-    }
-
-    _measureItem = (forceScrollCorrection) => {
-        const {handleNewMeasurements, size: oldSize, itemId} = this.props;
-
-        const node = this._node;
-
-        if (node && node.ownerDocument && node.ownerDocument.defaultView && node instanceof node.ownerDocument.defaultView.HTMLElement) {
-            const newSize = Math.ceil(node.offsetHeight);
-
-            if (oldSize !== newSize) {
-                handleNewMeasurements(itemId, newSize, forceScrollCorrection);
-            }
-        }
-    };
-
-    positionScrollBars = (height = this.props.size) => {
-        //we are position these hidden div scroll bars to the end so they can emit
-        //scroll event when height in the div changes
-        //Heavily inspired from https://github.com/marcj/css-element-queries/blob/master/src/ResizeSensor.js
-        //and https://github.com/wnr/element-resize-detector/blob/master/src/detection-strategy/scroll.js
-        //For more info http://www.backalleycoder.com/2013/03/18/cross-browser-event-based-element-resize-detection/#comment-244
-        if (this._positionScrollbarsRef) {
-            window.cancelAnimationFrame(this._positionScrollbarsRef);
-        }
-
-        this._positionScrollbarsRef = window.requestAnimationFrame(() => {
-            this._resizeSensorExpand.current.scrollTop = height + expandScrollDelta;
-            this._resizeSensorShrink.current.scrollTop = (2 * height) + shrinkScrollDelta;
-        });
-    };
-
-    scrollingDiv = (event) => {
-        if (event.target.offsetHeight !== this.props.size) {
-            this._measureItem(event.target.offsetWidth !== this.props.width);
-        }
-    };
-
-    renderItems = () => {
-        const item = this.props.item;
-
-        const expandChildStyle = {
-            position: 'absolute',
-            left: '0',
-            top: '0',
-            height: `${this.props.size + expandScrollDelta}px`,
-            width: '100%',
-        };
-
-        const renderItem = (
-            <div
-                role='listitem'
-                style={{position: 'relative'}}
-            >
-                {item}
-                <div style={scrollableContainerStyles}>
-                    <div
-                        dir='ltr'
-                        style={scrollableWrapperStyle}
-                    >
-                        <div style={expandShrinkContainerStyles}>
-                            <div
-                                style={expandShrinkStyles}
-                                ref={this._resizeSensorExpand}
-                                onScroll={this.scrollingDiv}
-                            >
-                                <div style={expandChildStyle}/>
-                            </div>
-                            <div
-                                style={expandShrinkStyles}
-                                ref={this._resizeSensorShrink}
-                                onScroll={this.scrollingDiv}
-                            >
-                                <div style={shrinkChildStyle}/>
-                            </div>
-                        </div>
-                    </div>
-                </div>
-            </div>
-        );
-        return renderItem;
-    };
-
-    componentWillUnmount() {
-        if (this._positionScrollbarsRef) {
-            window.cancelAnimationFrame(this._positionScrollbarsRef);
-        }
-
-        if (this._measureItemAnimFrame) {
-            window.cancelAnimationFrame(this._measureItemAnimFrame);
-        }
-
-        const {onUnmount, itemId, index} = this.props;
-        if (onUnmount) {
-            onUnmount(itemId, index);
-        }
-    }
-
-    render() {
-        return this.renderItems();
-    }
-}

webapp/channels/src/components/dynamic_virtualized_list/item_row_shared.tsx

@@ -0,0 +1,99 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import debounce from 'lodash/debounce';
+import type {ReactNode} from 'react';
+import React, {memo, useEffect, useRef} from 'react';
+
+import {ListItemSizeObserver} from './item_row_size_observer';
+
+const RESIZE_DEBOUNCE_TIME = 200; // in ms
+
+const listItemSizeObserver = ListItemSizeObserver.getInstance();
+
+interface Props {
+    item: ReactNode;
+    itemId: string;
+    index: number;
+    height: number;
+    width?: number; // Its optional since it may not be available when the parent component is mounted
+    onHeightChange: (itemId: string, height: number, forceScrollCorrection: boolean) => void;
+    onUnmount: (itemId: string, index: number) => void;
+}
+
+/**
+ * This component is used to measure the height of a row and update the height of the row when it changes.
+ * Uses a shared ResizeObserver instance for better performance with many items.
+ */
+const ListItem = (props: Props) => {
+    const rowRef = useRef<HTMLDivElement>(null);
+
+    const heightRef = useRef(props.height);
+    const widthRef = useRef(props.width);
+    const indexRef = useRef(props.index);
+
+    // This prevents stale closures in the ResizeObserver callback
+    // and ensures event handlers always have the latest values
+    // we also update these refs whenever their source values change
+    useEffect(() => {
+        heightRef.current = props.height;
+        widthRef.current = props.width;
+        indexRef.current = props.index;
+    }, [props.itemId, props.height, props.width, props.index]);
+
+    // This effect is used to measure the height of the row as soon as the component mounts
+    useEffect(() => {
+        const newHeight = Math.ceil(rowRef?.current?.offsetHeight ?? 0);
+        props.onHeightChange(props.itemId, newHeight, false);
+    }, [props.itemId]);
+
+    // This effects adds the observer which calls height change callback debounced
+    useEffect(() => {
+        const debouncedOnHeightChange = debounce((changedHeight: number) => {
+            // If width of container has changed then scroll bar position will be out of sync
+            // so we need to force a scroll correction
+            const forceScrollCorrection = rowRef.current?.offsetWidth !== widthRef.current;
+
+            heightRef.current = changedHeight;
+
+            props.onHeightChange(props.itemId, changedHeight, forceScrollCorrection);
+        }, RESIZE_DEBOUNCE_TIME);
+
+        function itemRowSizeObserverCallback(changedHeight: number) {
+            if (!rowRef.current) {
+                return;
+            }
+
+            if (changedHeight !== heightRef.current) {
+                debouncedOnHeightChange(changedHeight);
+            }
+        }
+
+        let cleanupSizeObserver: () => void;
+
+        // We add the observer here to a row
+        if (rowRef.current) {
+            cleanupSizeObserver = listItemSizeObserver.observe(props.itemId, rowRef.current, itemRowSizeObserverCallback);
+        }
+
+        // We remove the observer here from a row
+        return () => {
+            if (cleanupSizeObserver) {
+                cleanupSizeObserver();
+            }
+            props.onUnmount(props.itemId, indexRef.current);
+        };
+    }, [props.itemId]);
+
+    return (
+        <div
+            ref={rowRef}
+            role='listitem'
+            className='item_measurer'
+        >
+            {props.item}
+        </div>
+    );
+};
+
+export default memo(ListItem);

webapp/channels/src/components/dynamic_virtualized_list/item_row_size_observer.ts

@@ -0,0 +1,64 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+type TrackedItemCallback = (changedHeight: number) => void;
+type TrackedItemData = {element: Element; callback: TrackedItemCallback};
+type TrackedItemsMap = Map<string, TrackedItemData>;
+
+export class ListItemSizeObserver {
+    private observer: ResizeObserver;
+
+    private trackedItems: TrackedItemsMap = new Map();
+
+    private static instance: ListItemSizeObserver | null = null;
+
+    private constructor() {
+        this.observer = new ResizeObserver(this.handleResizeObserver);
+    }
+
+    public static getInstance(): ListItemSizeObserver {
+        if (!ListItemSizeObserver.instance) {
+            // Following class based singleton pattern to avoid multiple instances of the observer
+            ListItemSizeObserver.instance = new ListItemSizeObserver();
+        }
+        return ListItemSizeObserver.instance;
+    }
+
+    private handleResizeObserver = (resizeEntries: ResizeObserverEntry[]) => {
+        resizeEntries.forEach((resizeEntry) => {
+            const resizedElement = resizeEntry.target;
+
+            let itemData: TrackedItemData | undefined;
+            for (const [, trackedItemData] of this.trackedItems.entries()) {
+                // Reverse lookup by element to get the item's data
+                if (trackedItemData.element === resizedElement) {
+                    itemData = trackedItemData;
+                    break;
+                }
+            }
+
+            if (!itemData) {
+                return;
+            }
+
+            const changedHeight = Math.ceil(resizeEntry.borderBoxSize[0].blockSize);
+            itemData.callback(changedHeight);
+        });
+    };
+
+    public observe(itemId: string, element: Element, callback: TrackedItemCallback): () => void {
+        this.trackedItems.set(itemId, {element, callback});
+        this.observer.observe(element);
+
+        return () => this.unobserve(itemId);
+    }
+
+    private unobserve(itemId: string): void {
+        const trackedItemToUnobserve = this.trackedItems.get(itemId);
+        if (trackedItemToUnobserve) {
+            this.observer.unobserve(trackedItemToUnobserve.element);
+            this.trackedItems.delete(itemId);
+        }
+    }
+}
+