revert: Last table resizable column stable width in React 18 (#1741)

cloudscape-design · Nov 17, 2023 · be8a039 · be8a039
1 parent 625842c
commit be8a039
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 139 deletions.
diff --git a/src/anchor-navigation/use-scroll-spy.tsx b/src/anchor-navigation/use-scroll-spy.tsx
@@ -37,7 +37,7 @@ export default function useScrollSpy({
   }, [hrefs]);
 
   // Get the bounding rectangle of an element by href
-  const getRectByHref = useCallback((href: string) => {
+  const getRectByHref = useCallback(href => {
     return document.getElementById(href.slice(1))?.getBoundingClientRect();
   }, []);
 

diff --git a/src/table/__integ__/resizable-columns.test.ts b/src/table/__integ__/resizable-columns.test.ts
@@ -60,20 +60,6 @@ class TablePage extends BasePageObject {
     return element.getAttribute('style');
   }
 
-  getFirstTableHeaderWidths() {
-    return this.browser.execute(() => {
-      const tables = document.querySelectorAll('table');
-      return Array.from(tables[0].querySelectorAll('th')).map(el => el.offsetWidth);
-    });
-  }
-
-  getLastTableHeaderWidths() {
-    return this.browser.execute(() => {
-      const tables = document.querySelectorAll('table');
-      return Array.from(tables[tables.length - 1].querySelectorAll('th')).map(el => el.offsetWidth);
-    });
-  }
-
   async getColumnMinWidth(columnIndex: number) {
     const columnSelector = tableWrapper
       // use internal CSS-selector to always receive the real table header and not a sticky copy
@@ -191,13 +177,13 @@ describe.each([true, false])('StickyHeader=%s', sticky => {
     })
   );
 
-  test.each([1680, 620])('sticky and real column headers must have identical widths for screen width %s', width =>
+  test(
+    'should render "width: auto" for the last on big screens and explicit value on small',
     setupStickyTest(async page => {
-      await page.setWindowSize({ ...defaultScreen, width });
-      const stickyHeaderWidths = await page.getFirstTableHeaderWidths();
-      const realHeaderWidths = await page.getLastTableHeaderWidths();
-      expect(stickyHeaderWidths).toEqual(realHeaderWidths);
-    })()
+      await expect(page.getColumnStyle(4)).resolves.toContain('width: auto;');
+      await page.setWindowSize({ ...defaultScreen, width: 620 });
+      await expect(page.getColumnStyle(4)).resolves.toContain('width: 120px;');
+    })
   );
 
   // The page width of 620px is an empirical value defined for the respective test page in VR

diff --git a/src/table/__tests__/columns-width.test.tsx b/src/table/__tests__/columns-width.test.tsx
@@ -212,10 +212,11 @@ describe('with stickyHeader=true', () => {
       { minWidth: '100px', width: '', maxWidth: '' },
       { minWidth: '', width: '', maxWidth: '300px' },
     ]);
+    // in JSDOM, there is no layout, so copied width is "0px" and no value is ""
     expect(extractSize(fakeHeader)).toEqual([
-      { minWidth: '', width: '200px', maxWidth: '' },
-      { minWidth: '', width: '', maxWidth: '' },
-      { minWidth: '', width: '', maxWidth: '' },
+      { minWidth: '', width: '0px', maxWidth: '' },
+      { minWidth: '', width: '0px', maxWidth: '' },
+      { minWidth: '', width: '0px', maxWidth: '' },
     ]);
   });
 });
diff --git a/src/table/__tests__/resizable-columns.test.tsx b/src/table/__tests__/resizable-columns.test.tsx
@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: Apache-2.0
 import * as React from 'react';
 import times from 'lodash/times';
-import { useResizeObserver } from '@cloudscape-design/component-toolkit/internal';
 import { render, screen } from '@testing-library/react';
 import createWrapper, { TableWrapper } from '../../../lib/components/test-utils/dom';
 import Table, { TableProps } from '../../../lib/components/table';
@@ -20,11 +19,6 @@ jest.mock('../../../lib/components/internal/utils/scrollable-containers', () =>
   }),
 }));
 
-jest.mock('@cloudscape-design/component-toolkit/internal', () => ({
-  ...jest.requireActual('@cloudscape-design/component-toolkit/internal'),
-  useResizeObserver: jest.fn().mockImplementation((_target, cb) => cb({ contentBoxWidth: 0 })),
-}));
-
 interface Item {
   id: number;
   description: string;
@@ -437,14 +431,3 @@ describe('column header content', () => {
     expect(getResizeHandle(1)).toHaveAttribute('aria-roledescription', 'resize button');
   });
 });
-
-test('should set last column width to "auto" when container width exceeds total column width', () => {
-  const totalColumnsWidth = 150 + 300;
-  jest
-    .mocked(useResizeObserver)
-    .mockImplementation((_target, cb) => cb({ contentBoxWidth: totalColumnsWidth + 1 } as any));
-
-  const { wrapper } = renderTable(<Table {...defaultProps} />);
-
-  expect(wrapper.findColumnHeaders().map(w => w.getElement().style.width)).toEqual(['150px', 'auto']);
-});
diff --git a/src/table/column-widths-utils.ts b/src/table/column-widths-utils.ts
@@ -11,25 +11,6 @@ export function checkColumnWidths(columnDefinitions: ReadonlyArray<TableProps.Co
   }
 }
 
-export function setElementWidths(element: undefined | HTMLElement, styles: React.CSSProperties) {
-  function setProperty(property: 'width' | 'minWidth' | 'maxWidth') {
-    const value = styles[property];
-    let widthCssValue = '';
-    if (typeof value === 'number') {
-      widthCssValue = value + 'px';
-    }
-    if (typeof value === 'string') {
-      widthCssValue = value;
-    }
-    if (element && element.style[property] !== widthCssValue) {
-      element.style[property] = widthCssValue;
-    }
-  }
-  setProperty('width');
-  setProperty('minWidth');
-  setProperty('maxWidth');
-}
-
 function checkProperty(column: TableProps.ColumnDefinition<any>, name: 'width' | 'minWidth') {
   const value = column[name];
   if (typeof value !== 'number' && typeof value !== 'undefined') {

diff --git a/src/table/internal.tsx b/src/table/internal.tsx
@@ -114,8 +114,7 @@ const InternalTable = React.forwardRef(
     const isMobile = useMobile();
 
     const [containerWidth, wrapperMeasureRef] = useContainerQuery<number>(rect => rect.contentBoxWidth);
-    const wrapperMeasureRefObject = useRef(null);
-    const wrapperMeasureMergedRef = useMergeRefs(wrapperMeasureRef, wrapperMeasureRefObject);
+    const wrapperRefObject = useRef(null);
 
     const [tableWidth, tableMeasureRef] = useContainerQuery<number>(rect => rect.contentBoxWidth);
     const tableRefObject = useRef(null);
@@ -135,7 +134,6 @@ const InternalTable = React.forwardRef(
       [cancelEdit]
     );
 
-    const wrapperRefObject = useRef(null);
     const handleScroll = useScrollSync([wrapperRefObject, scrollbarRef, secondaryWrapperRef]);
 
     const { moveFocusDown, moveFocusUp, moveFocus } = useSelectionFocusMove(selectionType, items.length);
@@ -207,6 +205,7 @@ const InternalTable = React.forwardRef(
     const tableRole = hasEditableCells ? 'grid-default' : 'table';
 
     const theadProps: TheadProps = {
+      containerWidth,
       selectionType,
       getSelectAllProps,
       columnDefinitions: visibleColumnDefinitions,
@@ -258,11 +257,7 @@ const InternalTable = React.forwardRef(
 
     return (
       <LinkDefaultVariantContext.Provider value={{ defaultVariant: 'primary' }}>
-        <ColumnWidthsProvider
-          visibleColumns={visibleColumnWidthsWithSelection}
-          resizableColumns={resizableColumns}
-          containerRef={wrapperMeasureRefObject}
-        >
+        <ColumnWidthsProvider visibleColumns={visibleColumnWidthsWithSelection} resizableColumns={resizableColumns}>
           <InternalContainer
             {...baseProps}
             __internalRootRef={__internalRootRef}
@@ -338,7 +333,7 @@ const InternalTable = React.forwardRef(
               onScroll={handleScroll}
               {...wrapperProps}
             >
-              <div className={styles['wrapper-content-measure']} ref={wrapperMeasureMergedRef}></div>
+              <div className={styles['wrapper-content-measure']} ref={wrapperMeasureRef}></div>
               {!!renderAriaLive && !!firstIndex && (
                 <LiveRegion>
                   <span>

diff --git a/src/table/sticky-columns/use-sticky-columns.ts b/src/table/sticky-columns/use-sticky-columns.ts
@@ -156,7 +156,7 @@ export function useStickyCellStyles({
 
   // refCallback updates the cell ref and sets up the store subscription
   const refCallback = useCallback(
-    (cellElement: null | HTMLElement) => {
+    cellElement => {
       if (unsubscribeRef.current) {
         // Unsubscribe before we do any updates to avoid leaving any subscriptions hanging
         unsubscribeRef.current();

diff --git a/src/table/thead.tsx b/src/table/thead.tsx
@@ -18,6 +18,7 @@ import { TableThElement } from './header-cell/th-element';
 import { findUpUntil } from '@cloudscape-design/component-toolkit/dom';
 
 export interface TheadProps {
+  containerWidth: null | number;
   selectionType: TableProps.SelectionType | undefined;
   columnDefinitions: ReadonlyArray<TableProps.ColumnDefinition<any>>;
   sortingColumn: TableProps.SortingColumn<any> | undefined;
@@ -46,6 +47,7 @@ export interface TheadProps {
 const Thead = React.forwardRef(
   (
     {
+      containerWidth,
       selectionType,
       getSelectAllProps,
       columnDefinitions,
@@ -89,7 +91,7 @@ const Thead = React.forwardRef(
       isVisualRefresh && styles['is-visual-refresh']
     );
 
-    const { getColumnStyles, columnWidths, updateColumn, setCell } = useColumnWidths();
+    const { columnWidths, totalWidth, updateColumn, setCell } = useColumnWidths();
 
     return (
       <thead className={clsx(!hidden && styles['thead-active'])}>
@@ -131,11 +133,27 @@ const Thead = React.forwardRef(
 
           {columnDefinitions.map((column, colIndex) => {
             const columnId = getColumnKey(column, colIndex);
+
+            let widthOverride;
+            if (resizableColumns) {
+              if (columnWidths) {
+                // use stateful value if available
+                widthOverride = columnWidths[columnId];
+              }
+              if (colIndex === columnDefinitions.length - 1 && containerWidth && containerWidth > totalWidth) {
+                // let the last column grow and fill the container width
+                widthOverride = 'auto';
+              }
+            }
             return (
               <TableHeaderCell
                 key={columnId}
-                style={getColumnStyles(sticky, columnId)}
                 className={headerCellClass}
+                style={{
+                  width: widthOverride || column.width,
+                  minWidth: sticky ? undefined : column.minWidth,
+                  maxWidth: resizableColumns || sticky ? undefined : column.maxWidth,
+                }}
                 tabIndex={sticky ? -1 : 0}
                 focusedComponent={focusedComponent}
                 column={column}
@@ -152,7 +170,7 @@ const Thead = React.forwardRef(
                 onClick={detail => fireNonCancelableEvent(onSortingChange, detail)}
                 isEditable={!!column.editConfig}
                 stickyState={stickyState}
-                cellRef={node => setCell(sticky, columnId, node)}
+                cellRef={node => setCell(columnId, node)}
                 tableRole={tableRole}
                 resizerRoleDescription={resizerRoleDescription}
               />

diff --git a/src/table/use-column-widths.tsx b/src/table/use-column-widths.tsx
@@ -1,15 +1,12 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import { useResizeObserver, useStableCallback } from '@cloudscape-design/component-toolkit/internal';
 import React, { useEffect, useRef, useState, createContext, useContext } from 'react';
-import { setElementWidths } from './column-widths-utils';
 
 export const DEFAULT_COLUMN_WIDTH = 120;
 
 export interface ColumnWidthDefinition {
   id: PropertyKey;
   minWidth?: string | number;
-  maxWidth?: string | number;
   width?: string | number;
 }
 
@@ -50,14 +47,14 @@ function updateWidths(
 }
 
 interface WidthsContext {
-  getColumnStyles(sticky: boolean, columnId: PropertyKey): React.CSSProperties;
+  totalWidth: number;
   columnWidths: Record<PropertyKey, number>;
   updateColumn: (columnId: PropertyKey, newWidth: number) => void;
-  setCell: (sticky: boolean, columnId: PropertyKey, node: null | HTMLElement) => void;
+  setCell: (columnId: PropertyKey, node: null | HTMLElement) => void;
 }
 
 const WidthsContext = createContext<WidthsContext>({
-  getColumnStyles: () => ({}),
+  totalWidth: 0,
   columnWidths: {},
   updateColumn: () => {},
   setCell: () => {},
@@ -66,76 +63,26 @@ const WidthsContext = createContext<WidthsContext>({
 interface WidthProviderProps {
   visibleColumns: readonly ColumnWidthDefinition[];
   resizableColumns: boolean | undefined;
-  containerRef: React.RefObject<HTMLElement>;
   children: React.ReactNode;
 }
 
-export function ColumnWidthsProvider({ visibleColumns, resizableColumns, containerRef, children }: WidthProviderProps) {
-  const visibleColumnsRef = useRef<PropertyKey[] | null>(null);
-  const containerWidthRef = useRef(0);
-  const [columnWidths, setColumnWidths] = useState<null | Record<PropertyKey, number>>(null);
+export function ColumnWidthsProvider({ visibleColumns, resizableColumns, children }: WidthProviderProps) {
+  const visibleColumnsRef = useRef<(PropertyKey | undefined)[] | null>(null);
+  const [columnWidths, setColumnWidths] = useState<Record<PropertyKey, number>>({});
 
   const cellsRef = useRef<Record<PropertyKey, HTMLElement>>({});
-  const stickyCellsRef = useRef<Record<PropertyKey, HTMLElement>>({});
   const getCell = (columnId: PropertyKey): null | HTMLElement => cellsRef.current[columnId] ?? null;
-  const setCell = (sticky: boolean, columnId: PropertyKey, node: null | HTMLElement) => {
-    const ref = sticky ? stickyCellsRef : cellsRef;
+  const setCell = (columnId: PropertyKey, node: null | HTMLElement) => {
     if (node) {
-      ref.current[columnId] = node;
+      cellsRef.current[columnId] = node;
     } else {
-      delete ref.current[columnId];
+      delete cellsRef.current[columnId];
     }
   };
 
-  const getColumnStyles = (sticky: boolean, columnId: PropertyKey): React.CSSProperties => {
-    const column = visibleColumns.find(column => column.id === columnId);
-    if (!column) {
-      return {};
-    }
-
-    if (sticky) {
-      return { width: cellsRef.current[column.id]?.offsetWidth || (columnWidths?.[column.id] ?? column.width) };
-    }
-
-    if (resizableColumns && columnWidths) {
-      const isLastColumn = column.id === visibleColumns[visibleColumns.length - 1]?.id;
-      const totalWidth = visibleColumns.reduce((sum, { id }) => sum + (columnWidths[id] || DEFAULT_COLUMN_WIDTH), 0);
-      if (isLastColumn && containerWidthRef.current > totalWidth) {
-        return { width: 'auto', minWidth: column?.minWidth };
-      } else {
-        return { width: columnWidths[column.id], minWidth: column?.minWidth };
-      }
-    }
-    return {
-      width: column.width,
-      minWidth: column.minWidth,
-      maxWidth: !resizableColumns ? column.maxWidth : undefined,
-    };
-  };
-
-  // Imperatively sets width style for a cell avoiding React state.
-  // This allows setting the style as soon container's size change is observed.
-  const updateColumnWidths = useStableCallback(() => {
-    for (const column of visibleColumns) {
-      setElementWidths(cellsRef.current[column.id], getColumnStyles(false, column.id));
-    }
-    // Sticky column widths must be synchronized once all real column widths are assigned.
-    for (const id of Object.keys(stickyCellsRef.current)) {
-      setElementWidths(stickyCellsRef.current[id], getColumnStyles(true, id));
-    }
-  });
-
-  // Observes container size and requests an update to the last cell width as it depends on the container's width.
-  useResizeObserver(containerRef, ({ contentBoxWidth: containerWidth }) => {
-    containerWidthRef.current = containerWidth;
-    updateColumnWidths();
-  });
-
   // The widths of the dynamically added columns (after the first render) if not set explicitly
   // will default to the DEFAULT_COLUMN_WIDTH.
   useEffect(() => {
-    updateColumnWidths();
-
     if (!resizableColumns) {
       return;
     }
@@ -144,7 +91,7 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, contain
     if (lastVisible) {
       for (let index = 0; index < visibleColumns.length; index++) {
         const column = visibleColumns[index];
-        if (!columnWidths?.[column.id] && lastVisible.indexOf(column.id) === -1) {
+        if (!columnWidths[column.id] && lastVisible.indexOf(column.id) === -1) {
           updates[column.id] = (column.width as number) || DEFAULT_COLUMN_WIDTH;
         }
       }
@@ -153,7 +100,7 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, contain
       }
     }
     visibleColumnsRef.current = visibleColumns.map(column => column.id);
-  }, [columnWidths, resizableColumns, visibleColumns, updateColumnWidths]);
+  }, [columnWidths, resizableColumns, visibleColumns]);
 
   // Read the actual column widths after the first render to employ the browser defaults for
   // those columns without explicit width.
@@ -167,11 +114,16 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, contain
   }, []);
 
   function updateColumn(columnId: PropertyKey, newWidth: number) {
-    setColumnWidths(columnWidths => updateWidths(visibleColumns, columnWidths ?? {}, newWidth, columnId));
+    setColumnWidths(columnWidths => updateWidths(visibleColumns, columnWidths, newWidth, columnId));
   }
 
+  const totalWidth = visibleColumns.reduce(
+    (total, column) => total + (columnWidths[column.id] || DEFAULT_COLUMN_WIDTH),
+    0
+  );
+
   return (
-    <WidthsContext.Provider value={{ getColumnStyles, columnWidths: columnWidths ?? {}, updateColumn, setCell }}>
+    <WidthsContext.Provider value={{ columnWidths, totalWidth, updateColumn, setCell }}>
       {children}
     </WidthsContext.Provider>
   );