From 005c9f22d5c3e92209d2e71e7b4c432f08f970ab Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 19 Mar 2025 16:58:57 -0500 Subject: [PATCH 01/16] Use state to handle column auto resize --- src/hooks/useColumnWidths.ts | 60 +++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index 2f70109a15..b6e5160c47 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -1,5 +1,4 @@ -import { useLayoutEffect, useRef } from 'react'; -import { flushSync } from 'react-dom'; +import { useLayoutEffect, useRef, useState } from 'react'; import type { CalculatedColumn, StateSetter } from '../types'; import type { DataGridProps } from '../DataGrid'; @@ -16,6 +15,7 @@ export function useColumnWidths( setMeasuredColumnWidths: StateSetter>, onColumnResize: DataGridProps['onColumnResize'] ) { + const [columnToAutoresize, setColumnToAutoResize] = useState(null); const prevGridWidthRef = useRef(gridWidth); const columnsCanFlex: boolean = columns.length === viewportColumns.length; // Allow columns to flex again when... @@ -26,7 +26,10 @@ export function useColumnWidths( const columnsToMeasure: string[] = []; for (const { key, idx, width } of viewportColumns) { - if ( + if (key === columnToAutoresize) { + newTemplateColumns[idx] = 'max-content'; + columnsToMeasure.push(key); + } else if ( typeof width === 'string' && (ignorePreviouslyMeasuredColumns || !measuredColumnWidths.has(key)) && !resizedColumnWidths.has(key) @@ -62,40 +65,53 @@ export function useColumnWidths( return hasChanges ? newMeasuredColumnWidths : measuredColumnWidths; }); + + if (columnToAutoresize !== null) { + setColumnToAutoResize(null); + setResizedColumnWidths((resizedColumnWidths) => { + const newResizedColumnWidths = new Map(resizedColumnWidths); + newResizedColumnWidths.set( + columnToAutoresize, + measureColumnWidth(gridRef, columnToAutoresize)! + ); + return newResizedColumnWidths; + }); + } } function handleColumnResize(column: CalculatedColumn, nextWidth: number | 'max-content') { const { key: resizingKey } = column; - const newTemplateColumns = [...templateColumns]; const columnsToMeasure: string[] = []; - for (const { key, idx, width } of viewportColumns) { - if (resizingKey === key) { - const width = typeof nextWidth === 'number' ? `${nextWidth}px` : nextWidth; - newTemplateColumns[idx] = width; - } else if (columnsCanFlex && typeof width === 'string' && !resizedColumnWidths.has(key)) { - newTemplateColumns[idx] = width; + for (const { key, width } of viewportColumns) { + if ( + resizingKey !== key && + columnsCanFlex && + typeof width === 'string' && + !resizedColumnWidths.has(key) + ) { columnsToMeasure.push(key); } } - gridRef.current!.style.gridTemplateColumns = newTemplateColumns.join(' '); - const measuredWidth = - typeof nextWidth === 'number' ? nextWidth : measureColumnWidth(gridRef, resizingKey)!; + setMeasuredColumnWidths((measuredColumnWidths) => { + const newMeasuredColumnWidths = new Map(measuredColumnWidths); + for (const columnKey of columnsToMeasure) { + newMeasuredColumnWidths.delete(columnKey); + } + return newMeasuredColumnWidths; + }); - // TODO: remove - // need flushSync to keep frozen column offsets in sync - // we may be able to use `startTransition` or even `requestIdleCallback` instead - flushSync(() => { + if (typeof nextWidth === 'number') { setResizedColumnWidths((resizedColumnWidths) => { const newResizedColumnWidths = new Map(resizedColumnWidths); - newResizedColumnWidths.set(resizingKey, measuredWidth); + newResizedColumnWidths.set(resizingKey, nextWidth); return newResizedColumnWidths; }); - updateMeasuredWidths(columnsToMeasure); - }); - - onColumnResize?.(column, measuredWidth); + onColumnResize?.(column, nextWidth); + } else { + setColumnToAutoResize(resizingKey); + } } return { From b66f3bd6204903486f676414dc27bcc65c2c5524 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 19 Mar 2025 18:07:22 -0500 Subject: [PATCH 02/16] Optimize --- src/hooks/useColumnWidths.ts | 69 +++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index b6e5160c47..9775535620 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -15,7 +15,7 @@ export function useColumnWidths( setMeasuredColumnWidths: StateSetter>, onColumnResize: DataGridProps['onColumnResize'] ) { - const [columnToAutoresize, setColumnToAutoResize] = useState(null); + const [columnToAutoResize, setColumnToAutoResize] = useState(null); const prevGridWidthRef = useRef(gridWidth); const columnsCanFlex: boolean = columns.length === viewportColumns.length; // Allow columns to flex again when... @@ -26,7 +26,7 @@ export function useColumnWidths( const columnsToMeasure: string[] = []; for (const { key, idx, width } of viewportColumns) { - if (key === columnToAutoresize) { + if (key === columnToAutoResize) { newTemplateColumns[idx] = 'max-content'; columnsToMeasure.push(key); } else if ( @@ -47,34 +47,37 @@ export function useColumnWidths( }); function updateMeasuredWidths(columnsToMeasure: readonly string[]) { - if (columnsToMeasure.length === 0) return; - - setMeasuredColumnWidths((measuredColumnWidths) => { - const newMeasuredColumnWidths = new Map(measuredColumnWidths); - let hasChanges = false; - - for (const key of columnsToMeasure) { - const measuredWidth = measureColumnWidth(gridRef, key); - hasChanges ||= measuredWidth !== measuredColumnWidths.get(key); - if (measuredWidth === undefined) { - newMeasuredColumnWidths.delete(key); - } else { - newMeasuredColumnWidths.set(key, measuredWidth); + if (columnsToMeasure.length > 0) { + setMeasuredColumnWidths((measuredColumnWidths) => { + const newMeasuredColumnWidths = new Map(measuredColumnWidths); + let hasChanges = false; + + for (const key of columnsToMeasure) { + const measuredWidth = measureColumnWidth(gridRef, key); + hasChanges ||= measuredWidth !== measuredColumnWidths.get(key); + if (measuredWidth === undefined) { + newMeasuredColumnWidths.delete(key); + } else { + newMeasuredColumnWidths.set(key, measuredWidth); + } } - } - return hasChanges ? newMeasuredColumnWidths : measuredColumnWidths; - }); + return hasChanges ? newMeasuredColumnWidths : measuredColumnWidths; + }); + } - if (columnToAutoresize !== null) { + if (columnToAutoResize !== null) { setColumnToAutoResize(null); setResizedColumnWidths((resizedColumnWidths) => { - const newResizedColumnWidths = new Map(resizedColumnWidths); - newResizedColumnWidths.set( - columnToAutoresize, - measureColumnWidth(gridRef, columnToAutoresize)! - ); - return newResizedColumnWidths; + const oldWidth = resizedColumnWidths.get(columnToAutoResize); + const newWidth = measureColumnWidth(gridRef, columnToAutoResize); + if (newWidth !== undefined && oldWidth !== newWidth) { + const newResizedColumnWidths = new Map(resizedColumnWidths); + newResizedColumnWidths.set(columnToAutoResize, newWidth); + onColumnResize?.(viewportColumns.find((c) => c.key === columnToAutoResize)!, newWidth); + return newResizedColumnWidths; + } + return resizedColumnWidths; }); } } @@ -94,13 +97,15 @@ export function useColumnWidths( } } - setMeasuredColumnWidths((measuredColumnWidths) => { - const newMeasuredColumnWidths = new Map(measuredColumnWidths); - for (const columnKey of columnsToMeasure) { - newMeasuredColumnWidths.delete(columnKey); - } - return newMeasuredColumnWidths; - }); + if (columnsToMeasure.length > 0) { + setMeasuredColumnWidths((measuredColumnWidths) => { + const newMeasuredColumnWidths = new Map(measuredColumnWidths); + for (const columnKey of columnsToMeasure) { + newMeasuredColumnWidths.delete(columnKey); + } + return newMeasuredColumnWidths; + }); + } if (typeof nextWidth === 'number') { setResizedColumnWidths((resizedColumnWidths) => { From 51c2bdd209fd0abc3a13ee9adfdec3af1bd9a6e5 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 08:32:50 -0500 Subject: [PATCH 03/16] Cleanup --- src/hooks/useColumnWidths.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index 9775535620..a558c8d4ed 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -43,10 +43,10 @@ export function useColumnWidths( useLayoutEffect(() => { prevGridWidthRef.current = gridWidth; - updateMeasuredWidths(columnsToMeasure); + updateMeasuredWidths(); }); - function updateMeasuredWidths(columnsToMeasure: readonly string[]) { + function updateMeasuredWidths() { if (columnsToMeasure.length > 0) { setMeasuredColumnWidths((measuredColumnWidths) => { const newMeasuredColumnWidths = new Map(measuredColumnWidths); From d4afa2ba342de86b77346e418e98a5c8584b1e7c Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 08:52:18 -0500 Subject: [PATCH 04/16] Test --- src/hooks/useColumnWidths.ts | 1 + test/browser/column/resizable.test.tsx | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index a558c8d4ed..e61a48dd19 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -86,6 +86,7 @@ export function useColumnWidths( const { key: resizingKey } = column; const columnsToMeasure: string[] = []; + // remeasure all columns that can flex and are not resized by the user for (const { key, width } of viewportColumns) { if ( resizingKey !== key && diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index afd227bf25..e5bbb9aef9 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -94,6 +94,7 @@ test('should use the minWidth if specified', async () => { }); test('should auto resize column when resize handle is double clicked', async () => { + const onColumnResize = vi.fn(); setup({ columns, rows: [ @@ -101,13 +102,15 @@ test('should auto resize column when resize handle is double clicked', async () col1: 1, col2: 'a'.repeat(50) } - ] + ], + onColumnResize }); const [, col2] = getHeaderCells(); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); await autoResize(col2); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 327.703px' }); + expect(onColumnResize).toHaveBeenCalledExactlyOnceWith(expect.objectContaining(col2), 327.703); }); test('should use the maxWidth if specified on auto resize', async () => { From ccaa4376e3d0193227f9fc2e3ff9e1281e2545cc Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 09:30:49 -0500 Subject: [PATCH 05/16] Fix tests --- test/browser/column/resizable.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index e5bbb9aef9..8a2dca8e6f 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -110,7 +110,8 @@ test('should auto resize column when resize handle is double clicked', async () await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); await autoResize(col2); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 327.703px' }); - expect(onColumnResize).toHaveBeenCalledExactlyOnceWith(expect.objectContaining(col2), 327.703); + // This is called twice in strict mode + expect(onColumnResize).toHaveBeenCalledWith(expect.objectContaining(columns[1]), 327.703125); }); test('should use the maxWidth if specified on auto resize', async () => { From 69c46a221de67a58f662291d8cd01be8eb43bef1 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 09:56:41 -0500 Subject: [PATCH 06/16] Add flex columns test, make tests more robust --- test/browser/column/resizable.test.tsx | 53 +++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index 8a2dca8e6f..0860456933 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -66,10 +66,10 @@ test('cannot not resize or auto resize column when resizable is not specified', test('should resize column when dragging the handle', async () => { const onColumnResize = vi.fn(); setup({ columns, rows: [], onColumnResize }); - const [, col2] = getHeaderCells(); const grid = getGrid(); expect(onColumnResize).not.toHaveBeenCalled(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); + const [, col2] = getHeaderCells(); await resize({ column: col2, resizeBy: -50 }); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 150px' }); expect(onColumnResize).toHaveBeenCalledExactlyOnceWith(expect.objectContaining(columns[1]), 150); @@ -77,18 +77,18 @@ test('should resize column when dragging the handle', async () => { test('should use the maxWidth if specified', async () => { setup({ columns, rows: [] }); - const [, col2] = getHeaderCells(); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px ' }); + const [, col2] = getHeaderCells(); await resize({ column: col2, resizeBy: 1000 }); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 400px' }); }); test('should use the minWidth if specified', async () => { setup({ columns, rows: [] }); - const [, col2] = getHeaderCells(); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); + const [, col2] = getHeaderCells(); await resize({ column: col2, resizeBy: -150 }); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 100px' }); }); @@ -105,9 +105,9 @@ test('should auto resize column when resize handle is double clicked', async () ], onColumnResize }); - const [, col2] = getHeaderCells(); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); + const [, col2] = getHeaderCells(); await autoResize(col2); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 327.703px' }); // This is called twice in strict mode @@ -124,9 +124,9 @@ test('should use the maxWidth if specified on auto resize', async () => { } ] }); - const [, col2] = getHeaderCells(); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); + const [, col2] = getHeaderCells(); await autoResize(col2); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 400px' }); }); @@ -141,9 +141,50 @@ test('should use the minWidth if specified on auto resize', async () => { } ] }); - const [, col2] = getHeaderCells(); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' }); + const [, col2] = getHeaderCells(); await autoResize(col2); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 100px' }); }); + +test('should remeasure flex columns when resizing a column', async () => { + setup< + { + readonly col1: string; + readonly col2: string; + readonly col3: string; + }, + unknown + >({ + columns: [ + { + key: 'col1', + name: 'col1', + resizable: true + }, + { + key: 'col2', + name: 'col2', + resizable: true + }, + { + key: 'col3', + name: 'col3', + resizable: true + } + ], + rows: [ + { + col1: 'a'.repeat(10), + col2: 'a'.repeat(10), + col3: 'a'.repeat(10) + } + ] + }); + const grid = getGrid(); + await expect.element(grid).toHaveStyle({ gridTemplateColumns: '639.328px 639.328px 639.344px' }); + const [col1] = getHeaderCells(); + await autoResize(col1); + await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); +}); From f60e759e6ea2caf9ce48490f20e5650e5701be1d Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 10:03:07 -0500 Subject: [PATCH 07/16] Improve coverage --- test/browser/column/resizable.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index 0860456933..ea622b5fc9 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -187,4 +187,6 @@ test('should remeasure flex columns when resizing a column', async () => { const [col1] = getHeaderCells(); await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); + await autoResize(col1); + await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); }); From f736a3b0a73251e7b0fd14f99240d29b5cdaf805 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 10:14:04 -0500 Subject: [PATCH 08/16] test `onColumnResize` --- test/browser/column/resizable.test.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index ea622b5fc9..7f2fd8c1fd 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -149,6 +149,7 @@ test('should use the minWidth if specified on auto resize', async () => { }); test('should remeasure flex columns when resizing a column', async () => { + const onColumnResize = vi.fn(); setup< { readonly col1: string; @@ -180,13 +181,18 @@ test('should remeasure flex columns when resizing a column', async () => { col2: 'a'.repeat(10), col3: 'a'.repeat(10) } - ] + ], + onColumnResize }); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '639.328px 639.328px 639.344px' }); const [col1] = getHeaderCells(); await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); + expect(onColumnResize).toHaveBeenCalled(); + onColumnResize.mockClear(); + // if the width is the same, don't call onColumnResize await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); + expect(onColumnResize).not.toHaveBeenCalled(); }); From 0e909061eb7bf3cdf69297d920ecd9f59de1a629 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 10:18:13 -0500 Subject: [PATCH 09/16] Tweak --- src/hooks/useColumnWidths.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index e61a48dd19..a103aa101f 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -89,8 +89,8 @@ export function useColumnWidths( // remeasure all columns that can flex and are not resized by the user for (const { key, width } of viewportColumns) { if ( - resizingKey !== key && columnsCanFlex && + resizingKey !== key && typeof width === 'string' && !resizedColumnWidths.has(key) ) { From 523ec5a827ca931b899fb3f70dc892ff3daeb8ae Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 10:27:44 -0500 Subject: [PATCH 10/16] Rename helper --- src/hooks/useColumnWidths.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index a103aa101f..de095357d9 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -43,10 +43,10 @@ export function useColumnWidths( useLayoutEffect(() => { prevGridWidthRef.current = gridWidth; - updateMeasuredWidths(); + updateMeasuredAndResizedWidths(); }); - function updateMeasuredWidths() { + function updateMeasuredAndResizedWidths() { if (columnsToMeasure.length > 0) { setMeasuredColumnWidths((measuredColumnWidths) => { const newMeasuredColumnWidths = new Map(measuredColumnWidths); From 8a6bb2873ccf76cd340cd6139b0278d4426c1924 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 20 Mar 2025 11:09:22 -0500 Subject: [PATCH 11/16] Tweak comment --- test/browser/column/resizable.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index 7f2fd8c1fd..5a9e8189c8 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -191,7 +191,7 @@ test('should remeasure flex columns when resizing a column', async () => { await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); expect(onColumnResize).toHaveBeenCalled(); onColumnResize.mockClear(); - // if the width is the same, don't call onColumnResize + // onColumnResize is not called if width is not changed await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); expect(onColumnResize).not.toHaveBeenCalled(); From 8bae511c9c9e24ed9fe171485e72b55fc424619f Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 8 Apr 2025 10:33:44 -0500 Subject: [PATCH 12/16] Address comments --- src/hooks/useColumnWidths.ts | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index de095357d9..cd8537cae2 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -47,24 +47,24 @@ export function useColumnWidths( }); function updateMeasuredAndResizedWidths() { - if (columnsToMeasure.length > 0) { - setMeasuredColumnWidths((measuredColumnWidths) => { - const newMeasuredColumnWidths = new Map(measuredColumnWidths); - let hasChanges = false; - - for (const key of columnsToMeasure) { - const measuredWidth = measureColumnWidth(gridRef, key); - hasChanges ||= measuredWidth !== measuredColumnWidths.get(key); - if (measuredWidth === undefined) { - newMeasuredColumnWidths.delete(key); - } else { - newMeasuredColumnWidths.set(key, measuredWidth); - } + if (columnsToMeasure.length === 0) return; + + setMeasuredColumnWidths((measuredColumnWidths) => { + const newMeasuredColumnWidths = new Map(measuredColumnWidths); + let hasChanges = false; + + for (const key of columnsToMeasure) { + const measuredWidth = measureColumnWidth(gridRef, key); + hasChanges ||= measuredWidth !== measuredColumnWidths.get(key); + if (measuredWidth === undefined) { + newMeasuredColumnWidths.delete(key); + } else { + newMeasuredColumnWidths.set(key, measuredWidth); } + } - return hasChanges ? newMeasuredColumnWidths : measuredColumnWidths; - }); - } + return hasChanges ? newMeasuredColumnWidths : measuredColumnWidths; + }); if (columnToAutoResize !== null) { setColumnToAutoResize(null); From 012a2b9d41262abe059055fd851bdeecf70bdbed Mon Sep 17 00:00:00 2001 From: Nicolas Stepien Date: Tue, 8 Apr 2025 17:10:10 +0100 Subject: [PATCH 13/16] tweak test --- test/browser/column/resizable.test.tsx | 95 +++++++++++++++----------- test/browser/utils.tsx | 15 ++-- 2 files changed, 62 insertions(+), 48 deletions(-) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index 5a9e8189c8..85c079a81e 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -1,8 +1,9 @@ -import { commands, userEvent } from '@vitest/browser/context'; +import { useState } from 'react'; +import { commands, page, userEvent } from '@vitest/browser/context'; -import type { Column } from '../../../src'; +import { DataGrid, type Column } from '../../../src'; import { resizeHandleClassname } from '../../../src/HeaderCell'; -import { getGrid, getHeaderCells, setup } from '../utils'; +import { getGrid, getHeaderCells, setup, testGridClassname } from '../utils'; interface Row { readonly col1: number; @@ -149,50 +150,66 @@ test('should use the minWidth if specified on auto resize', async () => { }); test('should remeasure flex columns when resizing a column', async () => { - const onColumnResize = vi.fn(); - setup< + interface Row { + readonly col1: string; + readonly col2: string; + readonly col3: string; + } + + const columns: readonly Column[] = [ { - readonly col1: string; - readonly col2: string; - readonly col3: string; + key: 'col1', + name: 'col1', + resizable: true }, - unknown - >({ - columns: [ - { - key: 'col1', - name: 'col1', - resizable: true - }, - { - key: 'col2', - name: 'col2', - resizable: true - }, - { - key: 'col3', - name: 'col3', - resizable: true - } - ], - rows: [ - { - col1: 'a'.repeat(10), - col2: 'a'.repeat(10), - col3: 'a'.repeat(10) - } - ], - onColumnResize - }); + { + key: 'col2', + name: 'col2', + resizable: true + }, + { + key: 'col3', + name: 'col3', + resizable: true + } + ]; + + const rows: readonly Row[] = [ + { + col1: 'a'.repeat(10), + col2: 'a'.repeat(10), + col3: 'a'.repeat(10) + } + ]; + + let callCount = 0; + + function TestComponent() { + const [, setColWidths] = useState((): ReadonlyMap => new Map()); + + return ( + { + callCount++; + setColWidths((prev) => new Map(prev).set(column.key, width)); + }} + className={testGridClassname} + /> + ); + } + + page.render(); + const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '639.328px 639.328px 639.344px' }); const [col1] = getHeaderCells(); await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); - expect(onColumnResize).toHaveBeenCalled(); - onColumnResize.mockClear(); + expect(callCount).toBe(2); // onColumnResize is not called if width is not changed await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); - expect(onColumnResize).not.toHaveBeenCalled(); + expect(callCount).toBe(2); }); diff --git a/test/browser/utils.tsx b/test/browser/utils.tsx index c25ab87cbe..959ce7abd8 100644 --- a/test/browser/utils.tsx +++ b/test/browser/utils.tsx @@ -4,16 +4,13 @@ import { css } from '@linaria/core'; import { DataGrid } from '../../src'; import type { DataGridProps } from '../../src'; +export const testGridClassname = css` + block-size: 1080px; + scrollbar-width: none; +`; + export function setup(props: DataGridProps) { - page.render( - - ); + page.render(); } export function getGrid() { From 3f0490552bcd02e84abf1e0d0326f0bdd1cdf84b Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 8 Apr 2025 12:13:54 -0500 Subject: [PATCH 14/16] Revert "tweak test" This reverts commit 012a2b9d41262abe059055fd851bdeecf70bdbed. --- test/browser/column/resizable.test.tsx | 95 +++++++++++--------------- test/browser/utils.tsx | 15 ++-- 2 files changed, 48 insertions(+), 62 deletions(-) diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index 85c079a81e..5a9e8189c8 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -1,9 +1,8 @@ -import { useState } from 'react'; -import { commands, page, userEvent } from '@vitest/browser/context'; +import { commands, userEvent } from '@vitest/browser/context'; -import { DataGrid, type Column } from '../../../src'; +import type { Column } from '../../../src'; import { resizeHandleClassname } from '../../../src/HeaderCell'; -import { getGrid, getHeaderCells, setup, testGridClassname } from '../utils'; +import { getGrid, getHeaderCells, setup } from '../utils'; interface Row { readonly col1: number; @@ -150,66 +149,50 @@ test('should use the minWidth if specified on auto resize', async () => { }); test('should remeasure flex columns when resizing a column', async () => { - interface Row { - readonly col1: string; - readonly col2: string; - readonly col3: string; - } - - const columns: readonly Column[] = [ - { - key: 'col1', - name: 'col1', - resizable: true - }, + const onColumnResize = vi.fn(); + setup< { - key: 'col2', - name: 'col2', - resizable: true + readonly col1: string; + readonly col2: string; + readonly col3: string; }, - { - key: 'col3', - name: 'col3', - resizable: true - } - ]; - - const rows: readonly Row[] = [ - { - col1: 'a'.repeat(10), - col2: 'a'.repeat(10), - col3: 'a'.repeat(10) - } - ]; - - let callCount = 0; - - function TestComponent() { - const [, setColWidths] = useState((): ReadonlyMap => new Map()); - - return ( - { - callCount++; - setColWidths((prev) => new Map(prev).set(column.key, width)); - }} - className={testGridClassname} - /> - ); - } - - page.render(); - + unknown + >({ + columns: [ + { + key: 'col1', + name: 'col1', + resizable: true + }, + { + key: 'col2', + name: 'col2', + resizable: true + }, + { + key: 'col3', + name: 'col3', + resizable: true + } + ], + rows: [ + { + col1: 'a'.repeat(10), + col2: 'a'.repeat(10), + col3: 'a'.repeat(10) + } + ], + onColumnResize + }); const grid = getGrid(); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '639.328px 639.328px 639.344px' }); const [col1] = getHeaderCells(); await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); - expect(callCount).toBe(2); + expect(onColumnResize).toHaveBeenCalled(); + onColumnResize.mockClear(); // onColumnResize is not called if width is not changed await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); - expect(callCount).toBe(2); + expect(onColumnResize).not.toHaveBeenCalled(); }); diff --git a/test/browser/utils.tsx b/test/browser/utils.tsx index 959ce7abd8..c25ab87cbe 100644 --- a/test/browser/utils.tsx +++ b/test/browser/utils.tsx @@ -4,13 +4,16 @@ import { css } from '@linaria/core'; import { DataGrid } from '../../src'; import type { DataGridProps } from '../../src'; -export const testGridClassname = css` - block-size: 1080px; - scrollbar-width: none; -`; - export function setup(props: DataGridProps) { - page.render(); + page.render( + + ); } export function getGrid() { From 21205c8ecceb185be1660d4af75352e332ac1568 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 8 Apr 2025 13:26:22 -0500 Subject: [PATCH 15/16] Use `flushSync` --- src/HeaderRow.tsx | 4 +- src/hooks/useColumnWidths.ts | 92 +++++++++++++------------- src/types.ts | 2 + test/browser/column/resizable.test.tsx | 11 +-- test/browser/utils.tsx | 15 ++--- 5 files changed, 61 insertions(+), 63 deletions(-) diff --git a/src/HeaderRow.tsx b/src/HeaderRow.tsx index 7b4c7cb755..d6667a477b 100644 --- a/src/HeaderRow.tsx +++ b/src/HeaderRow.tsx @@ -3,7 +3,7 @@ import { css } from '@linaria/core'; import clsx from 'clsx'; import { getColSpan } from './utils'; -import type { CalculatedColumn, Direction, Position } from './types'; +import type { CalculatedColumn, Direction, Position, ResizedWidth } from './types'; import type { DataGridProps } from './DataGrid'; import HeaderCell from './HeaderCell'; import { cell, cellFrozen } from './style/cell'; @@ -17,7 +17,7 @@ type SharedDataGridProps = Pick< export interface HeaderRowProps extends SharedDataGridProps { rowIdx: number; columns: readonly CalculatedColumn[]; - onColumnResize: (column: CalculatedColumn, width: number | 'max-content') => void; + onColumnResize: (column: CalculatedColumn, width: ResizedWidth) => void; selectCell: (position: Position) => void; lastFrozenColumnIndex: number; selectedCellIdx: number | undefined; diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index cd8537cae2..4c0d743ee6 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -1,6 +1,7 @@ -import { useLayoutEffect, useRef, useState } from 'react'; +import { useLayoutEffect, useState } from 'react'; +import { flushSync } from 'react-dom'; -import type { CalculatedColumn, StateSetter } from '../types'; +import type { CalculatedColumn, ResizedWidth, StateSetter } from '../types'; import type { DataGridProps } from '../DataGrid'; export function useColumnWidths( @@ -15,19 +16,25 @@ export function useColumnWidths( setMeasuredColumnWidths: StateSetter>, onColumnResize: DataGridProps['onColumnResize'] ) { - const [columnToAutoResize, setColumnToAutoResize] = useState(null); - const prevGridWidthRef = useRef(gridWidth); + const [columnToAutoResize, setColumnToAutoResize] = useState<{ + readonly key: string; + readonly width: ResizedWidth; + } | null>(null); + const [prevGridWidth, setPreviousGridWidth] = useState(gridWidth); const columnsCanFlex: boolean = columns.length === viewportColumns.length; // Allow columns to flex again when... const ignorePreviouslyMeasuredColumns: boolean = // there is enough space for columns to flex and the grid was resized - columnsCanFlex && gridWidth !== prevGridWidthRef.current; + columnsCanFlex && gridWidth !== prevGridWidth; const newTemplateColumns = [...templateColumns]; const columnsToMeasure: string[] = []; for (const { key, idx, width } of viewportColumns) { - if (key === columnToAutoResize) { - newTemplateColumns[idx] = 'max-content'; + if (key === columnToAutoResize?.key) { + newTemplateColumns[idx] = + columnToAutoResize.width === 'max-content' + ? columnToAutoResize.width + : `${columnToAutoResize.width}px`; columnsToMeasure.push(key); } else if ( typeof width === 'string' && @@ -41,12 +48,10 @@ export function useColumnWidths( const gridTemplateColumns = newTemplateColumns.join(' '); - useLayoutEffect(() => { - prevGridWidthRef.current = gridWidth; - updateMeasuredAndResizedWidths(); - }); + useLayoutEffect(updateMeasuredWidths); - function updateMeasuredAndResizedWidths() { + function updateMeasuredWidths() { + setPreviousGridWidth(gridWidth); if (columnsToMeasure.length === 0) return; setMeasuredColumnWidths((measuredColumnWidths) => { @@ -67,56 +72,49 @@ export function useColumnWidths( }); if (columnToAutoResize !== null) { - setColumnToAutoResize(null); + const resizingKey = columnToAutoResize.key; setResizedColumnWidths((resizedColumnWidths) => { - const oldWidth = resizedColumnWidths.get(columnToAutoResize); - const newWidth = measureColumnWidth(gridRef, columnToAutoResize); + const oldWidth = resizedColumnWidths.get(resizingKey); + const newWidth = measureColumnWidth(gridRef, resizingKey); if (newWidth !== undefined && oldWidth !== newWidth) { const newResizedColumnWidths = new Map(resizedColumnWidths); - newResizedColumnWidths.set(columnToAutoResize, newWidth); - onColumnResize?.(viewportColumns.find((c) => c.key === columnToAutoResize)!, newWidth); + newResizedColumnWidths.set(resizingKey, newWidth); return newResizedColumnWidths; } return resizedColumnWidths; }); + setColumnToAutoResize(null); } } - function handleColumnResize(column: CalculatedColumn, nextWidth: number | 'max-content') { + function handleColumnResize(column: CalculatedColumn, nextWidth: ResizedWidth) { const { key: resizingKey } = column; - const columnsToMeasure: string[] = []; - - // remeasure all columns that can flex and are not resized by the user - for (const { key, width } of viewportColumns) { - if ( - columnsCanFlex && - resizingKey !== key && - typeof width === 'string' && - !resizedColumnWidths.has(key) - ) { - columnsToMeasure.push(key); + + flushSync(() => { + if (columnsCanFlex) { + // remeasure all the columns that can flex and are not resized by the user + setMeasuredColumnWidths((measuredColumnWidths) => { + const newMeasuredColumnWidths = new Map(measuredColumnWidths); + for (const { key, width } of viewportColumns) { + if (resizingKey !== key && typeof width === 'string' && !resizedColumnWidths.has(key)) { + newMeasuredColumnWidths.delete(key); + } + } + return newMeasuredColumnWidths; + }); } - } - if (columnsToMeasure.length > 0) { - setMeasuredColumnWidths((measuredColumnWidths) => { - const newMeasuredColumnWidths = new Map(measuredColumnWidths); - for (const columnKey of columnsToMeasure) { - newMeasuredColumnWidths.delete(columnKey); - } - return newMeasuredColumnWidths; + setColumnToAutoResize({ + key: resizingKey, + width: nextWidth }); - } + }); - if (typeof nextWidth === 'number') { - setResizedColumnWidths((resizedColumnWidths) => { - const newResizedColumnWidths = new Map(resizedColumnWidths); - newResizedColumnWidths.set(resizingKey, nextWidth); - return newResizedColumnWidths; - }); - onColumnResize?.(column, nextWidth); - } else { - setColumnToAutoResize(resizingKey); + const previousWidth = resizedColumnWidths.get(resizingKey); + const newWidth = + typeof nextWidth === 'number' ? nextWidth : measureColumnWidth(gridRef, resizingKey); + if (newWidth !== undefined && newWidth !== previousWidth) { + onColumnResize?.(column, newWidth); } } diff --git a/src/types.ts b/src/types.ts index ae4246793e..192421625d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -322,3 +322,5 @@ export interface Renderers { } export type Direction = 'ltr' | 'rtl'; + +export type ResizedWidth = number | 'max-content'; diff --git a/test/browser/column/resizable.test.tsx b/test/browser/column/resizable.test.tsx index 5a9e8189c8..1a1f0449ac 100644 --- a/test/browser/column/resizable.test.tsx +++ b/test/browser/column/resizable.test.tsx @@ -110,8 +110,10 @@ test('should auto resize column when resize handle is double clicked', async () const [, col2] = getHeaderCells(); await autoResize(col2); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 327.703px' }); - // This is called twice in strict mode - expect(onColumnResize).toHaveBeenCalledWith(expect.objectContaining(columns[1]), 327.703125); + expect(onColumnResize).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining(columns[1]), + 327.703125 + ); }); test('should use the maxWidth if specified on auto resize', async () => { @@ -189,10 +191,9 @@ test('should remeasure flex columns when resizing a column', async () => { const [col1] = getHeaderCells(); await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); - expect(onColumnResize).toHaveBeenCalled(); - onColumnResize.mockClear(); + expect(onColumnResize).toHaveBeenCalledOnce(); // onColumnResize is not called if width is not changed await autoResize(col1); await expect.element(grid).toHaveStyle({ gridTemplateColumns: '79.1406px 919.422px 919.438px' }); - expect(onColumnResize).not.toHaveBeenCalled(); + expect(onColumnResize).toHaveBeenCalledOnce(); }); diff --git a/test/browser/utils.tsx b/test/browser/utils.tsx index c25ab87cbe..959ce7abd8 100644 --- a/test/browser/utils.tsx +++ b/test/browser/utils.tsx @@ -4,16 +4,13 @@ import { css } from '@linaria/core'; import { DataGrid } from '../../src'; import type { DataGridProps } from '../../src'; +export const testGridClassname = css` + block-size: 1080px; + scrollbar-width: none; +`; + export function setup(props: DataGridProps) { - page.render( - - ); + page.render(); } export function getGrid() { From 06d7e923019bcc155460e0010c93938515016f15 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 8 Apr 2025 13:44:01 -0500 Subject: [PATCH 16/16] Address comments --- src/hooks/useColumnWidths.ts | 12 +++++++----- test/browser/utils.tsx | 15 +++++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/hooks/useColumnWidths.ts b/src/hooks/useColumnWidths.ts index 4c0d743ee6..a9b8382934 100644 --- a/src/hooks/useColumnWidths.ts +++ b/src/hooks/useColumnWidths.ts @@ -110,11 +110,13 @@ export function useColumnWidths( }); }); - const previousWidth = resizedColumnWidths.get(resizingKey); - const newWidth = - typeof nextWidth === 'number' ? nextWidth : measureColumnWidth(gridRef, resizingKey); - if (newWidth !== undefined && newWidth !== previousWidth) { - onColumnResize?.(column, newWidth); + if (onColumnResize) { + const previousWidth = resizedColumnWidths.get(resizingKey); + const newWidth = + typeof nextWidth === 'number' ? nextWidth : measureColumnWidth(gridRef, resizingKey); + if (newWidth !== undefined && newWidth !== previousWidth) { + onColumnResize(column, newWidth); + } } } diff --git a/test/browser/utils.tsx b/test/browser/utils.tsx index 959ce7abd8..c25ab87cbe 100644 --- a/test/browser/utils.tsx +++ b/test/browser/utils.tsx @@ -4,13 +4,16 @@ import { css } from '@linaria/core'; import { DataGrid } from '../../src'; import type { DataGridProps } from '../../src'; -export const testGridClassname = css` - block-size: 1080px; - scrollbar-width: none; -`; - export function setup(props: DataGridProps) { - page.render(); + page.render( + + ); } export function getGrid() {