Skip to content

Commit

Permalink
fix: Last table resizable column stable width in React 18 (#1718)
Browse files Browse the repository at this point in the history
  • Loading branch information
pan-kot authored Nov 10, 2023
1 parent cd2b12f commit effb083
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 70 deletions.
2 changes: 1 addition & 1 deletion src/anchor-navigation/use-scroll-spy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default function useScrollSpy({
}, [hrefs]);

// Get the bounding rectangle of an element by href
const getRectByHref = useCallback(href => {
const getRectByHref = useCallback((href: string) => {
return document.getElementById(href.slice(1))?.getBoundingClientRect();
}, []);

Expand Down
26 changes: 20 additions & 6 deletions src/table/__integ__/resizable-columns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,20 @@ 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
Expand Down Expand Up @@ -177,13 +191,13 @@ describe.each([true, false])('StickyHeader=%s', sticky => {
})
);

test(
'should render "width: auto" for the last on big screens and explicit value on small',
test.each([1680, 620])('sticky and real column headers must have identical widths for screen width %s', width =>
setupStickyTest(async page => {
await expect(page.getColumnStyle(4)).resolves.toContain('width: auto;');
await page.setWindowSize({ ...defaultScreen, width: 620 });
await expect(page.getColumnStyle(4)).resolves.toContain('width: 120px;');
})
await page.setWindowSize({ ...defaultScreen, width });
const stickyHeaderWidths = await page.getFirstTableHeaderWidths();
const realHeaderWidths = await page.getLastTableHeaderWidths();
expect(stickyHeaderWidths).toEqual(realHeaderWidths);
})()
);

// The page width of 620px is an empirical value defined for the respective test page in VR
Expand Down
7 changes: 3 additions & 4 deletions src/table/__tests__/columns-width.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,10 @@ 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: '0px', maxWidth: '' },
{ minWidth: '', width: '0px', maxWidth: '' },
{ minWidth: '', width: '0px', maxWidth: '' },
{ minWidth: '', width: '200px', maxWidth: '' },
{ minWidth: '', width: '', maxWidth: '' },
{ minWidth: '', width: '', maxWidth: '' },
]);
});
});
17 changes: 17 additions & 0 deletions src/table/__tests__/resizable-columns.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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';
Expand All @@ -19,6 +20,11 @@ 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;
Expand Down Expand Up @@ -431,3 +437,14 @@ 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']);
});
19 changes: 19 additions & 0 deletions src/table/column-widths-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,25 @@ 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') {
Expand Down
13 changes: 9 additions & 4 deletions src/table/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ const InternalTable = React.forwardRef(
const isMobile = useMobile();

const [containerWidth, wrapperMeasureRef] = useContainerQuery<number>(rect => rect.contentBoxWidth);
const wrapperRefObject = useRef(null);
const wrapperMeasureRefObject = useRef(null);
const wrapperMeasureMergedRef = useMergeRefs(wrapperMeasureRef, wrapperMeasureRefObject);

const [tableWidth, tableMeasureRef] = useContainerQuery<number>(rect => rect.contentBoxWidth);
const tableRefObject = useRef(null);
Expand All @@ -134,6 +135,7 @@ const InternalTable = React.forwardRef(
[cancelEdit]
);

const wrapperRefObject = useRef(null);
const handleScroll = useScrollSync([wrapperRefObject, scrollbarRef, secondaryWrapperRef]);

const { moveFocusDown, moveFocusUp, moveFocus } = useSelectionFocusMove(selectionType, items.length);
Expand Down Expand Up @@ -205,7 +207,6 @@ const InternalTable = React.forwardRef(
const tableRole = hasEditableCells ? 'grid-default' : 'table';

const theadProps: TheadProps = {
containerWidth,
selectionType,
getSelectAllProps,
columnDefinitions: visibleColumnDefinitions,
Expand Down Expand Up @@ -257,7 +258,11 @@ const InternalTable = React.forwardRef(

return (
<LinkDefaultVariantContext.Provider value={{ defaultVariant: 'primary' }}>
<ColumnWidthsProvider visibleColumns={visibleColumnWidthsWithSelection} resizableColumns={resizableColumns}>
<ColumnWidthsProvider
visibleColumns={visibleColumnWidthsWithSelection}
resizableColumns={resizableColumns}
containerRef={wrapperMeasureRefObject}
>
<InternalContainer
{...baseProps}
__internalRootRef={__internalRootRef}
Expand Down Expand Up @@ -333,7 +338,7 @@ const InternalTable = React.forwardRef(
onScroll={handleScroll}
{...wrapperProps}
>
<div className={styles['wrapper-content-measure']} ref={wrapperMeasureRef}></div>
<div className={styles['wrapper-content-measure']} ref={wrapperMeasureMergedRef}></div>
{!!renderAriaLive && !!firstIndex && (
<LiveRegion>
<span>
Expand Down
2 changes: 1 addition & 1 deletion src/table/sticky-columns/use-sticky-columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export function useStickyCellStyles({

// refCallback updates the cell ref and sets up the store subscription
const refCallback = useCallback(
cellElement => {
(cellElement: null | HTMLElement) => {
if (unsubscribeRef.current) {
// Unsubscribe before we do any updates to avoid leaving any subscriptions hanging
unsubscribeRef.current();
Expand Down
24 changes: 3 additions & 21 deletions src/table/thead.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ 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;
Expand Down Expand Up @@ -47,7 +46,6 @@ export interface TheadProps {
const Thead = React.forwardRef(
(
{
containerWidth,
selectionType,
getSelectAllProps,
columnDefinitions,
Expand Down Expand Up @@ -91,7 +89,7 @@ const Thead = React.forwardRef(
isVisualRefresh && styles['is-visual-refresh']
);

const { columnWidths, totalWidth, updateColumn, setCell } = useColumnWidths();
const { getColumnStyles, columnWidths, updateColumn, setCell } = useColumnWidths();

return (
<thead className={clsx(!hidden && styles['thead-active'])}>
Expand Down Expand Up @@ -133,27 +131,11 @@ 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}
Expand All @@ -170,7 +152,7 @@ const Thead = React.forwardRef(
onClick={detail => fireNonCancelableEvent(onSortingChange, detail)}
isEditable={!!column.editConfig}
stickyState={stickyState}
cellRef={node => setCell(columnId, node)}
cellRef={node => setCell(sticky, columnId, node)}
tableRole={tableRole}
resizerRoleDescription={resizerRoleDescription}
/>
Expand Down
84 changes: 66 additions & 18 deletions src/table/use-column-widths.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// 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;
}

Expand Down Expand Up @@ -47,14 +50,14 @@ function updateWidths(
}

interface WidthsContext {
totalWidth: number;
getColumnStyles(sticky: boolean, columnId: PropertyKey): React.CSSProperties;
columnWidths: Record<PropertyKey, number>;
updateColumn: (columnId: PropertyKey, newWidth: number) => void;
setCell: (columnId: PropertyKey, node: null | HTMLElement) => void;
setCell: (sticky: boolean, columnId: PropertyKey, node: null | HTMLElement) => void;
}

const WidthsContext = createContext<WidthsContext>({
totalWidth: 0,
getColumnStyles: () => ({}),
columnWidths: {},
updateColumn: () => {},
setCell: () => {},
Expand All @@ -63,26 +66,76 @@ const WidthsContext = createContext<WidthsContext>({
interface WidthProviderProps {
visibleColumns: readonly ColumnWidthDefinition[];
resizableColumns: boolean | undefined;
containerRef: React.RefObject<HTMLElement>;
children: React.ReactNode;
}

export function ColumnWidthsProvider({ visibleColumns, resizableColumns, children }: WidthProviderProps) {
const visibleColumnsRef = useRef<(PropertyKey | undefined)[] | null>(null);
const [columnWidths, setColumnWidths] = useState<Record<PropertyKey, number>>({});
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);

const cellsRef = useRef<Record<PropertyKey, HTMLElement>>({});
const stickyCellsRef = useRef<Record<PropertyKey, HTMLElement>>({});
const getCell = (columnId: PropertyKey): null | HTMLElement => cellsRef.current[columnId] ?? null;
const setCell = (columnId: PropertyKey, node: null | HTMLElement) => {
const setCell = (sticky: boolean, columnId: PropertyKey, node: null | HTMLElement) => {
const ref = sticky ? stickyCellsRef : cellsRef;
if (node) {
cellsRef.current[columnId] = node;
ref.current[columnId] = node;
} else {
delete cellsRef.current[columnId];
delete ref.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;
}
Expand All @@ -91,7 +144,7 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, childre
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;
}
}
Expand All @@ -100,7 +153,7 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, childre
}
}
visibleColumnsRef.current = visibleColumns.map(column => column.id);
}, [columnWidths, resizableColumns, visibleColumns]);
}, [columnWidths, resizableColumns, visibleColumns, updateColumnWidths]);

// Read the actual column widths after the first render to employ the browser defaults for
// those columns without explicit width.
Expand All @@ -114,16 +167,11 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, childre
}, []);

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={{ columnWidths, totalWidth, updateColumn, setCell }}>
<WidthsContext.Provider value={{ getColumnStyles, columnWidths: columnWidths ?? {}, updateColumn, setCell }}>
{children}
</WidthsContext.Provider>
);
Expand Down
Loading

0 comments on commit effb083

Please sign in to comment.