Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DataGrid] Fix resizing right pinned column #15107

Merged
merged 13 commits into from
Nov 5, 2024
75 changes: 74 additions & 1 deletion packages/x-data-grid-pro/src/tests/columns.DataGridPro.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
GridAutosizeOptions,
} from '@mui/x-data-grid-pro';
import { useGridPrivateApiContext } from '@mui/x-data-grid-pro/internals';
import { getColumnHeaderCell, getCell, microtasks } from 'test/utils/helperFn';
import { getColumnHeaderCell, getCell, microtasks, getRow } from 'test/utils/helperFn';

const isJSDOM = /jsdom/.test(window.navigator.userAgent);

Expand Down Expand Up @@ -237,6 +237,79 @@ describe('<DataGridPro /> - Columns', () => {
expect(bottomPinnedRowCell?.getBoundingClientRect().width).to.equal(150);
});

// https://github.com/mui/mui-x/issues/12852
it('should work with right pinned column', () => {
render(
<Test
columns={[
{ field: 'id', width: 100 },
{ field: 'brand', width: 100 },
]}
initialState={{ pinnedColumns: { right: ['brand'] } }}
/>,
);

const pinnedHeaderCell = getColumnHeaderCell(1);
const pinnedCell = getCell(1, 1);
const pinnedSeparator = pinnedHeaderCell.querySelector(
`.${gridClasses['columnSeparator--resizable']}`,
)!;
const pinnedRightPosition = pinnedHeaderCell.getBoundingClientRect().right;

// resize right pinned column to the right
fireEvent.mouseDown(pinnedSeparator, { clientX: 100 });
fireEvent.mouseMove(pinnedSeparator, { clientX: 150, buttons: 1 });

// check that the right pinned column has shrunk and is in the same position
expect(pinnedHeaderCell.getBoundingClientRect().width).to.equal(50);
expect(pinnedCell.getBoundingClientRect().width).to.equal(50);
expect(pinnedHeaderCell.getBoundingClientRect().right).to.equal(pinnedRightPosition);

// release the mouse and check that the right pinned column is still in the same position
fireEvent.mouseUp(pinnedSeparator);
expect(pinnedHeaderCell.getBoundingClientRect().width).to.equal(50);
expect(pinnedCell.getBoundingClientRect().width).to.equal(50);
expect(pinnedHeaderCell.getBoundingClientRect().right).to.equal(pinnedRightPosition);

// resize the right pinned column to the left
fireEvent.mouseDown(pinnedSeparator, { clientX: 150 });
fireEvent.mouseMove(pinnedSeparator, { clientX: 50, buttons: 1 });

// check that the right pinned column has grown and is in the same position
expect(pinnedHeaderCell.getBoundingClientRect().width).to.equal(150);
expect(pinnedCell.getBoundingClientRect().width).to.equal(150);
expect(pinnedHeaderCell.getBoundingClientRect().right).to.equal(pinnedRightPosition);

// release the mouse and check that the right pinned column is still in the same position
fireEvent.mouseUp(pinnedSeparator);
expect(pinnedHeaderCell.getBoundingClientRect().width).to.equal(150);
expect(pinnedCell.getBoundingClientRect().width).to.equal(150);
expect(pinnedHeaderCell.getBoundingClientRect().right).to.equal(pinnedRightPosition);
});

// https://github.com/mui/mui-x/issues/13548
it('should fill remaining horizontal space in a row with an empty cell', () => {
render(<Test columns={[{ field: 'id', width: 100 }]} />);

const row = getRow(0);
const rowWidth = row.getBoundingClientRect().width;
const headerCell = getColumnHeaderCell(0);
const separator = headerCell.querySelector(`.${gridClasses['columnSeparator--resizable']}`)!;
const emptyCell = row.querySelector(`.${gridClasses.cellEmpty}`)!;

// check that empty cell takes up the remaining width in a row
expect(emptyCell.getBoundingClientRect().width).to.equal(rowWidth - 100);

// check that empty cell takes up the remaining width when the column is resized
fireEvent.mouseDown(separator, { clientX: 100 });
fireEvent.mouseMove(separator, { clientX: 50, buttons: 1 });
expect(emptyCell.getBoundingClientRect().width).to.equal(rowWidth - 50);

// release the mouse and check that the empty cell still takes up the remaining width
fireEvent.mouseUp(separator);
expect(emptyCell.getBoundingClientRect().width).to.equal(rowWidth - 50);
});

describe('flex resizing', () => {
before(function beforeHook() {
if (isJSDOM) {
Expand Down
29 changes: 1 addition & 28 deletions packages/x-data-grid/src/components/GridRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,6 @@ export interface GridRowProps extends React.HTMLAttributes<HTMLDivElement> {
[x: string]: any; // Allow custom attributes like data-* and aria-*
}

function EmptyCell({ width }: { width: number }) {
if (!width) {
return null;
}

return (
<div
role="presentation"
className={clsx(gridClasses.cell, gridClasses.cellEmpty)}
style={{ '--width': `${width}px` } as React.CSSProperties}
/>
);
}

EmptyCell.propTypes = {
// ----------------------------- Warning --------------------------------
// | These PropTypes are generated from the TypeScript type definitions |
// | To update them edit the TypeScript types and run "pnpm proptypes" |
// ----------------------------------------------------------------------
width: PropTypes.number.isRequired,
} as any;

const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(props, refProp) {
const {
selected,
Expand Down Expand Up @@ -454,10 +432,6 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(
}
: null;

const expandedWidth =
dimensions.viewportOuterSize.width - dimensions.columnsTotalWidth - scrollbarWidth;
const emptyCellWidth = Math.max(0, expandedWidth);

return (
<div
ref={handleRef}
Expand All @@ -477,8 +451,7 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(
style={{ width: offsetLeft }}
/>
{cells}
{emptyCellWidth > 0 && <EmptyCell width={emptyCellWidth} />}
{rightCells.length > 0 && <div role="presentation" className={gridClasses.filler} />}
Copy link
Member Author

@KenanYusuf KenanYusuf Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we need this + the empty cell. Perhaps I'm missing something.

Copy link
Contributor

@romgrk romgrk Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember :| You should build a test case with all the variations of:

  • pinned or no pinned columns
  • scrollbar or no scrollbar
  • empty space or no empty space (when columns don't fill the width)

And then check if all the cases behave correctly. Make sure to enable vertical borders on all cells, we need to make sure that borders are where they're supposed to be (by default the grid only has vertical border on pinned sections).

We should probably create a visual regression test based on that.

<div role="presentation" className={clsx(gridClasses.cell, gridClasses.cellEmpty)} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rendering the empty cell all the time improves resizing of the right-pinned cells

Before

before.mov

After

after.mov

Copy link
Member Author

@KenanYusuf KenanYusuf Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the actions column would also move with the right pinned column...

{rightCells}
{scrollbarWidth !== 0 && <ScrollbarFiller pinnedRight={pinnedColumns.right.length > 0} />}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ export const GridRootStyles = styled('div', {
lineHeight: 'inherit',
},
[`& .${c.cellEmpty}`]: {
flex: 1,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way the empty cell always takes up the right amount of space in a row. Shrinks when the row is full of cells, grows when there aren't enough to fill a row.

padding: 0,
height: 'unset',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,14 @@ export const useGridColumnResize = (
const prevWidth = refs.columnHeaderElement!.offsetWidth;
const widthDiff = newWidth - prevWidth;
const columnWidthDiff = newWidth - refs.initialColWidth;
const newTotalWidth = refs.initialTotalWidth + columnWidthDiff;

apiRef.current.rootElementRef?.current?.style.setProperty(
'--DataGrid-rowWidth',
`${newTotalWidth}px`,
);
if (columnWidthDiff > 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before

before.mov

After

after.mov

const newTotalWidth = refs.initialTotalWidth + columnWidthDiff;
apiRef.current.rootElementRef?.current?.style.setProperty(
'--DataGrid-rowWidth',
`${newTotalWidth}px`,
);
}

refs.colDef!.computedWidth = newWidth;
refs.colDef!.width = newWidth;
Expand Down
4 changes: 3 additions & 1 deletion packages/x-data-grid/src/tests/rows.DataGrid.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
GridRenderCellParams,
useGridApiRef,
GridApi,
gridClasses,
} from '@mui/x-data-grid';
import { getBasicGridData } from '@mui/x-data-grid-generator';
import {
Expand All @@ -31,6 +32,7 @@ import {
getActiveCell,
getCell,
microtasks,
$$,
} from 'test/utils/helperFn';
import { fireUserEvent } from 'test/utils/fireUserEvent';
import Dialog from '@mui/material/Dialog';
Expand Down Expand Up @@ -750,7 +752,7 @@ describe('<DataGrid /> - Rows', () => {
width={100}
/>,
);
expect(document.querySelectorAll('.MuiDataGrid-cell')).to.have.length(2);
expect($$(`.${gridClasses.cell}:not(.${gridClasses.cellEmpty})`)).to.have.length(2);
});

it('should measure rows while scrolling', async () => {
Expand Down
26 changes: 26 additions & 0 deletions test/regressions/data-grid/DataGridBordered.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import * as React from 'react';
import { DataGridPro } from '@mui/x-data-grid-pro';
import { randomTraderName, randomEmail } from '@mui/x-data-grid-generator';

const columns = [
{ field: 'name', headerName: 'Name', width: 160 },
{ field: 'email', headerName: 'Email', width: 200 },
{ field: 'age', headerName: 'Age', type: 'number' },
];

const rows = [
{
id: 1,
name: randomTraderName(),
email: randomEmail(),
age: 25,
},
];

export default function DataGridBordered() {
return (
<div style={{ height: 400, width: '100%' }}>
<DataGridPro rows={rows} columns={columns} showCellVerticalBorder showColumnVerticalBorder />
</div>
);
}
32 changes: 32 additions & 0 deletions test/regressions/data-grid/DataGridPinnedColumnsBordered.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import * as React from 'react';
import { DataGridPro } from '@mui/x-data-grid-pro';
import { randomTraderName, randomEmail } from '@mui/x-data-grid-generator';

const columns = [
{ field: 'name', headerName: 'Name', width: 160 },
{ field: 'email', headerName: 'Email', width: 200 },
{ field: 'age', headerName: 'Age', type: 'number' },
];

const rows = [
{
id: 1,
name: randomTraderName(),
email: randomEmail(),
age: 25,
},
];

export default function DataGridPinnedColumnsBordered() {
return (
<div style={{ height: 400, width: '100%' }}>
<DataGridPro
rows={rows}
columns={columns}
initialState={{ pinnedColumns: { left: ['name'], right: ['age'] } }}
showCellVerticalBorder
showColumnVerticalBorder
/>
</div>
);
}
134 changes: 134 additions & 0 deletions test/regressions/data-grid/DataGridPinnedColumnsScrollbars.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import * as React from 'react';
import DeleteIcon from '@mui/icons-material/Delete';
import EditIcon from '@mui/icons-material/Edit';
import { DataGridPro, GridActionsCellItem } from '@mui/x-data-grid-pro';
import {
randomCreatedDate,
randomTraderName,
randomEmail,
randomUpdatedDate,
} from '@mui/x-data-grid-generator';

const columns = [
{ field: 'name', headerName: 'Name', width: 160, editable: true },
{ field: 'email', headerName: 'Email', width: 200, editable: true },
{ field: 'age', headerName: 'Age', type: 'number', editable: true },
{
field: 'dateCreated',
headerName: 'Date Created',
type: 'date',
width: 180,
editable: true,
},
{
field: 'lastLogin',
headerName: 'Last Login',
type: 'dateTime',
width: 220,
editable: true,
},
{
field: 'actions',
type: 'actions',
width: 100,
getActions: () => [
<GridActionsCellItem icon={<EditIcon />} label="Edit" />,
<GridActionsCellItem icon={<DeleteIcon />} label="Delete" />,
],
},
];

const rows = [
{
id: 1,
name: randomTraderName(),
email: randomEmail(),
age: 25,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 2,
name: randomTraderName(),
email: randomEmail(),
age: 32,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 3,
name: randomTraderName(),
email: randomEmail(),
age: 45,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 4,
name: randomTraderName(),
email: randomEmail(),
age: 28,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 5,
name: randomTraderName(),
email: randomEmail(),
age: 39,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 6,
name: randomTraderName(),
email: randomEmail(),
age: 52,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 7,
name: randomTraderName(),
email: randomEmail(),
age: 33,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 8,
name: randomTraderName(),
email: randomEmail(),
age: 41,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 9,
name: randomTraderName(),
email: randomEmail(),
age: 36,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
{
id: 10,
name: randomTraderName(),
email: randomEmail(),
age: 29,
dateCreated: randomCreatedDate(),
lastLogin: randomUpdatedDate(),
},
];

export default function DataGridPinnedColumnsScrollbars() {
return (
<div style={{ height: 400, width: '100%' }}>
<DataGridPro
rows={rows}
columns={columns}
initialState={{ pinnedColumns: { left: ['name'], right: ['actions'] } }}
/>
</div>
);
}
Loading