Fix column auto resize #3746

amanmahajan7 · 2025-03-19T22:00:43Z

codecov · 2025-03-19T22:05:32Z

Codecov Report

Attention: Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.75%. Comparing base (eaf0691) to head (0bc54d9).

Files with missing lines	Patch %	Lines
src/hooks/useColumnWidths.ts	98.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3746      +/-   ##
==========================================
+ Coverage   98.65%   98.75%   +0.09%     
==========================================
  Files          47       47              
  Lines        3426     3451      +25     
  Branches      742      753      +11     
==========================================
+ Hits         3380     3408      +28     
+ Misses         46       43       -3

Files with missing lines	Coverage Δ
src/hooks/useColumnWidths.ts	`99.11% <98.30%> (+3.66%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/hooks/useColumnWidths.ts

nstepien

Found a bug, when I add maxWidth: 150, to the country column, the grid isn't filled, then I try to reduce its width, and the other columns fill the empty space.

nstepien · 2025-03-20T17:37:41Z

src/hooks/useColumnWidths.ts

  });

-  function updateMeasuredWidths(columnsToMeasure: readonly string[]) {
-    if (columnsToMeasure.length === 0) return;
+  function updateMeasuredAndResizedWidths() {


Could inline this in the layout effect

Hmm, shows a warning. Looks like eslint only finds this issue when inlined

I can add dependencies and useMemo for columnsToMeasure but not sure it is worth. Wdyt?

src/hooks/useColumnWidths.ts

nstepien · 2025-03-20T17:56:12Z

src/hooks/useColumnWidths.ts

-    // 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') {


I'm not sure this will correctly handle min/max-widths
We need to set the width, render, measure the cell, store the measured width instead of trusting nextWidth

It seems like it does. I will double check
https://github.com/adazzle/react-data-grid/blob/main/src/HeaderCell.tsx#L128

It works. We should put the min/max logic in one place

min / max can't overflow in case styles are applied to the measurement cells correctly, and grid re-render is synchronized

I also found it helpful to use styles instead of JS for this, especially when you can have different styles for cells while measuring (when we set its size to "max-content" for example, and will measure its size after):

You want to have a grid with auto-sized columns but with some rules for this auto-size mechanism:
min: 50px
max: 300px

But at the same time, you want to be able to resize columns to exceed these values manually.

In this situation, you will have different rules for measured cells and while measuring:
When measuring cells (like "max-content" or "auto"), we want to apply these rules to that cell
but when we resize manually, we don't

.measuringCellClassname { min-width: 20px; } .measuringCellClassname[measuring] { min-width: 50px; max-width: 300px }

This is why I've added this code also:
https://github.com/adazzle/react-data-grid/pull/3724/files#diff-62e848727f791df23e85ccc2140643630d80c20c5140f073aba8f581eb122019L13-R26

But at the same time, you want to be able to resize columns to exceed these values manually.

We want the same rules for bot cases. If maxWidth is specified then users should not be able to increase the width past that value.

I also found it helpful to use styles instead of JS
We already set min/maxWidth on the measuring cell
https://github.com/adazzle/react-data-grid/blob/main/src/utils/renderMeasuringCells.tsx#L18

Yep, but I'm talking about cases when you have not specified maxWidth for the column, and this max-width only applies to the moment when we calculate the size of the column in cases when both are specified:
column.maxWidth: 100px
measuringCellClassname[measuring] { max-width: 200px; }
then it should not exceed 100px
But in case:
column.maxWidth: 100px
measuringCellClassname[measuring] { max-width: 50px; }
the auto-sized width will not exceed 50px but can be re-sized up to 100px manually

src/hooks/useColumnWidths.ts

nstepien · 2025-03-20T18:14:33Z

test/browser/column/resizable.test.tsx

  const grid = getGrid();
  expect(onColumnResize).not.toHaveBeenCalled();
  await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 200px' });
+  const [, col2] = getHeaderCells();


I don't understand, surely the cell element should still be in the document if we get it earlier, no?

It should be. I will investigate in a separate PR

nstepien · 2025-03-20T18:15:13Z

test/browser/column/resizable.test.tsx

  await autoResize(col2);
  await expect.element(grid).toHaveStyle({ gridTemplateColumns: '100px 327.703px' });
+  // This is called twice in strict mode


Should it? Do we not check to only call the function if the width has changed?

It does call it twice. May be the state updates after the useLayoutEffect has run twice

react-data-grid/src/hooks/useColumnWidths.ts

Line 69 in 8a6bb28

if (columnToAutoResize !== null) {

Wroud · 2025-03-21T12:16:34Z

src/hooks/useColumnWidths.ts

@@ -16,6 +15,7 @@ export function useColumnWidths<R, SR>(
  setMeasuredColumnWidths: StateSetter<ReadonlyMap<string, number>>,
  onColumnResize: DataGridProps<R, SR>['onColumnResize']
 ) {
+  const [columnToAutoResize, setColumnToAutoResize] = useState<string | null>(null);
  const prevGridWidthRef = useRef(gridWidth);


@amanmahajan7
I think you better not use ref for the previous grid width. There is a bug with the initial render when:

table rendered with a different width from the previous

columns calculated + updated

table rendered with a different width from the previous

columns calculated but aren't updated (because of the same widths) -> there are no re-render -> cells will have "measurement" size (like "max-content") and will change size on content size

This issue is reflected in my video from this comment: #3723 (comment)

Can you check if this issues still exists in my PR? I think the issue you are describing was related to #3724 (comment)

Wroud · 2025-03-21T12:37:56Z

src/hooks/useColumnWidths.ts

-    });
-
-    onColumnResize?.(column, measuredWidth);
+      onColumnResize?.(column, nextWidth);


I will suggest moving the onColumnResize call to the setter function to have the same possible behavior for resizing
https://github.com/adazzle/react-data-grid/pull/3746/files#diff-e002f181d41ce4fd6e070724347a44e4220d0332c37e5964d04cb3fc5d60d416R77
(or vice-versa)

amanmahajan7 · 2025-03-21T19:23:10Z

Found a bug, when I add maxWidth: 150, to the country column, the grid isn't filled, then I try to reduce its width, and the other columns fill the empty space.

This exists in main also. I believe the reason is that we are not restricting width calculation using grid columns and instead we are using min/max width on measuring cell. I have created a separate PR to refactor width calculation logic

#3747

Use state to handle column auto resize

005c9f2

amanmahajan7 added 8 commits March 19, 2025 18:07

Optimize

b66f3bd

Cleanup

51c2bdd

Test

d4afa2b

Fix tests

ccaa437

Add flex columns test, make tests more robust

69c46a2

Improve coverage

f60e759

test onColumnResize

f736a3b

Tweak

0e90906

amanmahajan7 marked this pull request as ready for review March 20, 2025 15:19

amanmahajan7 requested a review from nstepien as a code owner March 20, 2025 15:19

amanmahajan7 commented Mar 20, 2025

View reviewed changes

src/hooks/useColumnWidths.ts Show resolved Hide resolved

amanmahajan7 added 2 commits March 20, 2025 10:27

Rename helper

523ec5a

Tweak comment

8a6bb28

nstepien reviewed Mar 20, 2025

View reviewed changes

Wroud reviewed Mar 21, 2025

View reviewed changes

amanmahajan7 requested review from nstepien and Wroud March 21, 2025 19:23

amanmahajan7 mentioned this pull request Mar 24, 2025

Update column width calculation logic #3747

Open

Merge branch 'main' into am-double-double-click

0bc54d9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix column auto resize #3746

Fix column auto resize #3746

amanmahajan7 commented Mar 19, 2025 •

edited

Loading

codecov bot commented Mar 19, 2025 •

edited

Loading

nstepien left a comment •

edited

Loading

nstepien Mar 20, 2025

amanmahajan7 Mar 20, 2025

amanmahajan7 Mar 21, 2025

nstepien Mar 20, 2025

amanmahajan7 Mar 20, 2025

amanmahajan7 Mar 20, 2025

Wroud Mar 21, 2025

amanmahajan7 Mar 21, 2025

Wroud Mar 26, 2025

nstepien Mar 20, 2025

amanmahajan7 Mar 21, 2025

nstepien Mar 20, 2025

amanmahajan7 Mar 20, 2025

Wroud Mar 21, 2025 •

edited

Loading

amanmahajan7 Mar 21, 2025

Wroud Mar 21, 2025

amanmahajan7 commented Mar 21, 2025

Fix column auto resize #3746

Are you sure you want to change the base?

Fix column auto resize #3746

Conversation

amanmahajan7 commented Mar 19, 2025 • edited Loading

codecov bot commented Mar 19, 2025 • edited Loading

Codecov Report

nstepien left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wroud Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanmahajan7 commented Mar 21, 2025

amanmahajan7 commented Mar 19, 2025 •

edited

Loading

codecov bot commented Mar 19, 2025 •

edited

Loading

nstepien left a comment •

edited

Loading

Wroud Mar 21, 2025 •

edited

Loading