[DataGridPro] Fix missing border in right-pinned columns

[cherniavskii]

Fixes #4194

Codesandbox: https://codesandbox.io/s/datagridpro-v5-quick-start-forked-qx8vfl

Changelog

• The showCellRightBorder was renamed to showCellVerticalBorder
• The showColumnRightBorder was renamed to showColumnVerticalBorder
• The .MuiDataGrid-withBorder CSS class was renamed to .MuiDataGrid-withBorderColor and it only sets border-color CSS property now.

[mui-bot]

	Messages
📖	Netlify deploy preview: https://deploy-preview-4197--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	655.1	1,554.8	655.1	981.76	327.233
Sort 100k rows	ms	619.4	1,371.5	1,203.6	1,095.38	254.05
Select 100k rows	ms	219.1	335	302.4	278.66	46.53
Deselect 100k rows	ms	200.1	374.6	254.4	263.2	62.365

Generated by 🚫 dangerJS against 09856d8

[m4theushw]

I'm not sure if this is the right approach. Currently, if showCellRightBorder is on the cells receive the .MuiDataGrid-withBorder class. This class is specifically to change the border of the cells. Here we're changing the meaning of showCellRightBorder by applying a border not to the pinned cells but to their container. If the user wants to change the color of the cell border he will have to change in two places: here and in .MuiDataGrid-withBorder.

My proposal is to change .MuiDataGrid-withBorder to only set the border color, then the cells will set the border width/style on the appropriate side.

For v6 I agree to rename this prop to showCellVerticalBorder or other name.

[cherniavskii]

Good point!

If the user wants to change the color of the cell border he will have to change in two places: here and in .MuiDataGrid-withBorder.

But there's more places with border color anyway:

• mui-x/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts

  Line 66 in 140f394

  border: `1px solid ${borderColor}`,

• mui-x/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts

  Line 169 in 140f394

  color: borderColor,

• mui-x/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts

  Line 249 in 140f394

  borderBottom: `1px solid ${borderColor}`,

Maybe we should add separate borderColor class and apply it everywhere border is used?

[m4theushw]

Yeah but these other borders affect other parts, not the cells. Currently, .MuiDataGrid-withBorder only impacts the cell border.

Maybe we should add separate borderColor class and apply it everywhere border is used?

We'll have two classes for borders. In v6 I think we can unify this behavior and apply MuiDataGrid-withBorder to the grid instead, not the cells. Then, we can have a single border color. It could also be a CSS variable, to be used in color.

[cherniavskii]

Ok, I've split withBorder styles into two classes:

• withBorder that applies border-color only
• withRightBorder that applies border-right-width and border-right-style only

I've also used withBorder to apply border color to right-pinned container.

[m4theushw]

We miss a withLeftBorder applying border-left-width and border-right-style to the cell, not to the right pinned columns container.

[cherniavskii]

There are 2 problems with that:

1. I don't see a way to set showLeftBorder={true} only for first cells in the right-pinned section.

2. Applying left border to cell instead of right pinned columns container results in double border:

   borderLeft on right-pinned columns container
   
   borderLeft on cell
   
   I didn't find a way to avoid this double border.

[cherniavskii]

I've renamed showCellRightBorder to showCellVerticalBorder, showColumnRightBorder to showColumnVerticalBorder and added withBorderColor CSS class that sets the border color for both cells and column headers

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[m4theushw]

I think we're almost there.

If I change the color of MuiDataGrid-withBorder it doesn't apply to the bottom border of the cells in the right pinned columns.

I think the cells should have a showLeftBorder prop. If either showRightBorder or showLeftBorder is enabled, we add MuiDataGrid-withBorder and the respective border class. To set this prop to true is tricky. One way is to reuse getRowProps and pass this prop to GridRow and GridRow pass it only to the first cell.

mui-x/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualScroller.tsx

Line 174 in 3ba334d

const getRowProps = (id: GridRowId) => {

Maybe getRowProps needs to know which side the row will be rendered, we only know the ID.

[m4theushw]

Have you considered removing the borderColor above and also use the border class here?

Suggested change

	[`.${gridClasses.withBorderColor}`]: {
	borderColor,
	},
	[`&.${gridClasses.withBorderColor}`]: {
	borderColor,
	},
	[`.${gridClasses.withBorderColor}`]: {
	borderColor,
	},

[cherniavskii]

Yes, I did consider it, but it breaks this demo: https://mui.com/x/react-data-grid/style/#using-the-sx-prop
This means that you won't be able to override the border of the grid with:

<DataGrid
  sx={{ borderColor: 'primary.light' }}
/>

Because the default border color has higher specificity:

[`&.${gridClasses.withBorderColor}`]: {
  borderColor,
},

So I decided to not to do this and allow to override grid external border color with sx instead.