fix: DH-20319: Stuck to bottom of table not sticking (#2547)

- I believe the functionality stopped working after
9f87985,
where an added `setState` call at the end of `componentDidUpdate` would
overwrite any scroll position state state changes made in
`checkStickyScroll()`
- Now the relevant scroll position state changes are made in the same
`setState` call at the end of `componentDidUpdate`
- Added an e2e test to test the stuck to bottom scrolling functionality

Author: ericlln

packages/grid/src/Grid.tsx

@@ -516,12 +516,11 @@ class Grid extends PureComponent<GridProps, GridState> {
     // apply on mount, so that it works with a static model
     // https://github.com/deephaven/web-client-ui/issues/581
     const { isStuckToBottom, isStuckToRight } = this.props;
-    if (isStuckToBottom) {
-      this.scrollToBottom();
-    }
-    if (isStuckToRight) {
-      this.scrollToRight();
-    }
+    const { top, left } = this.getStickyScrollPosition(
+      isStuckToBottom,
+      isStuckToRight
+    );
+    this.setState({ top, left });
   }
 
   componentDidUpdate(prevProps: GridProps, prevState: GridState): void {
@@ -611,7 +610,14 @@ class Grid extends PureComponent<GridProps, GridState> {
 
     this.requestUpdateCanvas();
 
-    this.checkStickyScroll();
+    if (this.needToUpdateScroll()) {
+      const { top, left } = this.getStickyScrollPosition(
+        isStickyBottom,
+        isStickyRight
+      );
+      updatedState.top = top;
+      updatedState.left = left;
+    }
 
     if (this.validateSelection()) {
       this.checkSelectionChange(prevState);
@@ -776,29 +782,41 @@ class Grid extends PureComponent<GridProps, GridState> {
     this.setState({ isStuckToBottom: false });
   }
 
-  /**
-   * Scrolls to bottom, if not already at bottom
-   */
-  scrollToBottom(): void {
-    if (!this.metrics) return;
-    const { bottomVisible, rowCount, top, lastTop } = this.metrics;
-    if ((bottomVisible < rowCount - 1 && bottomVisible > 0) || top > lastTop) {
-      this.setState({ top: lastTop });
-    }
-  }
+  getStickyScrollPosition(
+    isStickyBottom: boolean,
+    isStickyRight: boolean
+  ): { top: VisibleIndex; left: VisibleIndex } {
+    // eslint-disable-next-line react/destructuring-assignment
+    if (!this.metrics) return { left: this.state.left, top: this.state.top };
 
-  /**
-   * Scrolls to right, if not already at right
-   */
-  scrollToRight(): void {
-    if (!this.metrics) return;
-    const { rightVisible, columnCount, left, lastLeft } = this.metrics;
+    const {
+      bottomVisible,
+      rowCount,
+      top,
+      lastTop,
+      rightVisible,
+      columnCount,
+      left,
+      lastLeft,
+    } = this.metrics;
+
+    let newTop = top;
+    let newLeft = left;
+
+    if (
+      isStickyBottom &&
+      ((bottomVisible < rowCount - 1 && bottomVisible > 0) || top > lastTop)
+    ) {
+      newTop = lastTop;
+    }
     if (
-      (rightVisible < columnCount - 1 && rightVisible > 0) ||
-      left > lastLeft
+      isStickyRight &&
+      ((rightVisible < columnCount - 1 && rightVisible > 0) || left > lastLeft)
     ) {
-      this.setState({ left: lastLeft });
+      newLeft = lastLeft;
     }
+
+    return { top: newTop, left: newLeft };
   }
 
   /**
@@ -931,35 +949,32 @@ class Grid extends PureComponent<GridProps, GridState> {
   /**
    * Compares the current metrics with the previous metrics to see if we need to scroll when it is stuck to the bottom or the right
    */
-  checkStickyScroll(): void {
-    if (!this.metrics) {
-      return;
+  needToUpdateScroll(): boolean {
+    if (!this.metrics || !this.prevMetrics) {
+      return false;
     }
 
-    if (this.prevMetrics) {
-      const { rowCount, columnCount, height, width } = this.metrics;
-      const {
-        rowCount: prevRowCount,
-        columnCount: prevColumnCount,
-        height: prevHeight,
-        width: prevWidth,
-      } = this.prevMetrics;
-
-      if (prevRowCount !== rowCount || height !== prevHeight) {
-        const { isStuckToBottom } = this.state;
-        if (isStuckToBottom) {
-          this.scrollToBottom();
-        }
+    const { rowCount, columnCount, height, width } = this.metrics;
+    const {
+      rowCount: prevRowCount,
+      columnCount: prevColumnCount,
+      height: prevHeight,
+      width: prevWidth,
+    } = this.prevMetrics;
+
+    if (prevRowCount !== rowCount || height !== prevHeight) {
+      const { isStuckToBottom } = this.state;
+      if (isStuckToBottom) {
+        return true;
       }
-
-      if (prevColumnCount !== columnCount || width !== prevWidth) {
-        const { isStuckToRight } = this.state;
-        if (isStuckToRight) {
-          this.scrollToRight();
-        }
+    }
+    if (prevColumnCount !== columnCount || width !== prevWidth) {
+      const { isStuckToRight } = this.state;
+      if (isStuckToRight) {
+        return true;
       }
     }
-    this.prevMetrics = this.metrics;
+    return false;
   }
 
   updateMetrics(state = this.state): GridMetrics {

tests/docker-scripts/data/app.d/table_generators.py

@@ -0,0 +1,26 @@
+from deephaven import empty_table, function_generated_table
+from deephaven.execution_context import get_exec_ctx
+
+# Creates a table that can be shrunk and grown in size
+def create_shrink_grow_table():
+    ctx = get_exec_ctx()
+    i = 50
+
+    def make_table():
+        return empty_table(i).update(["X = i"])
+
+    def shrink_table():
+        nonlocal i
+        i = 30;
+
+    def grow_table():
+        nonlocal i
+        i = 70;
+
+    table = function_generated_table(
+        table_generator=make_table,
+        refresh_interval_ms=1000,
+        exec_ctx=ctx
+    )
+
+    return table, shrink_table, grow_table

tests/docker-scripts/data/app.d/test.app

@@ -6,3 +6,4 @@ name=Web UI Test Application
 file_0=common_figures.py
 file_1=common_tables.py
 file_2=multiselect_tables.py
+file_3=table_generators.py

tests/table-scroll.spec.ts

@@ -4,6 +4,8 @@ import {
   openTableOption,
   openTable,
   gotoPage,
+  pasteInMonaco,
+  generateId,
 } from './utils';
 
 // Scroll the table by scrolling the mouse wheel, whilst moving the mouse up and down to test hovering functionality
@@ -129,3 +131,80 @@ test('scroll expanded rollup while moving mouse over rows', async ({
     });
   });
 });
+
+test('stuck to bottom scroll growing table', async ({ page }) => {
+  // Need to generate a new shrink grow table for each test
+  const id = generateId();
+  const tableName = `_${id}_shrink_grow_table`;
+  const growFunction = `func_${id}`;
+  const consoleInput = page.locator('.console-input');
+
+  await pasteInMonaco(
+    consoleInput,
+    `${tableName}, _, ${growFunction} = create_shrink_grow_table()`
+  );
+  await page.keyboard.press('Enter');
+  await waitForLoadingDone(page);
+
+  // Open the newly created table
+  const openButton = page.getByRole('button', { name: tableName, exact: true });
+  await openButton.click();
+  await waitForLoadingDone(page);
+
+  const gridCanvas = page.locator('.iris-grid .grid-wrapper');
+  await gridCanvas.click({ position: { x: 10, y: 80 } });
+  // Scroll to end of table to get stuck at bottom
+  await page.mouse.wheel(0, 1000);
+  await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
+
+  // Add more rows to the table and take another screenshot
+  await pasteInMonaco(consoleInput, `${growFunction}()`);
+  await page.keyboard.press('Enter');
+
+  // No reliable way to know when the added rows have loaded, so just wait
+  await page.waitForTimeout(2000);
+  await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
+});
+
+test('stuck to bottom scroll shrinking and growing table', async ({ page }) => {
+  // Need to generate a new shrink grow table for each test
+  const id = generateId();
+  const tableName = `_${id}_shrink_grow_table`;
+  const growFunction = `grow_${id}`;
+  const shrinkFunction = `shrink_${id}`;
+  const consoleInput = page.locator('.console-input');
+
+  await pasteInMonaco(
+    consoleInput,
+    `${tableName}, ${shrinkFunction}, ${growFunction} = create_shrink_grow_table()`
+  );
+  await page.keyboard.press('Enter');
+  await waitForLoadingDone(page);
+
+  // Open the newly created table
+  const openButton = page.getByRole('button', { name: tableName, exact: true });
+  await openButton.click();
+  await waitForLoadingDone(page);
+
+  const gridCanvas = page.locator('.iris-grid .grid-wrapper');
+  await gridCanvas.click({ position: { x: 10, y: 80 } });
+  // Scroll to end of table to get stuck at bottom
+  await page.mouse.wheel(0, 1000);
+  await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
+
+  // Shrink the table and take another screenshot
+  await pasteInMonaco(consoleInput, `${shrinkFunction}()`);
+  await page.keyboard.press('Enter');
+
+  // No reliable way to know when the removed rows have loaded, so just wait
+  await page.waitForTimeout(2000);
+  await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
+
+  // Add more rows to the table and take another screenshot
+  await pasteInMonaco(consoleInput, `${growFunction}()`);
+  await page.keyboard.press('Enter');
+
+  // No reliable way to know when the added rows have loaded, so just wait
+  await page.waitForTimeout(2000);
+  await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
+});