[data grid] Pinned columns / rows color in dark mode looks like an accident

[joserodolfofreitas]

Current behavior 😯

In dark mode, the background of the pinned columns is gray, and although it may work better in "blacker" backgrounds, it doesn't really match the theme we use in our demos.

Expected behavior 🤔

A different style to display pinned columns that better match different dark themes.

Steps to reproduce 🕹

Link to live example:

Steps:

1. https://mui.com/x/react-data-grid/column-pinning/
2. Change your settings to display in dark mode

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo

``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Order ID 💳 (optional)

No response

[gerdadesign]

It seems we have conflicting styles of our docs, which render a dark navy background, mixing with the demo pinned column background that is a shade of pure grey. We don't see an issue in light mode because MUI's background color and Material Design's background color are both pure white.

When I add a pure black background color to the demo in dark mode, the columns don't appear as jarring. Is this a feasible change to use throughout the dark demos?

[joserodolfofreitas]

Ideally, we should go for something that looks good on different dark themes.

Maybe we can try a style for the pinned columns that focus on the borders only. Or, if we want to keep the elevation using gray (as per material design guidelines), maybe we can evaluate changing the blackness of the color using HSL color scheme. But, in any case, I think we can explore some possibilities in design, @gerdadesign.

[oliviertassinari]

So, if we list the options, we have:

1. Change the column pinning implementation to match https://mui-plus.vercel.app/components/DataGrid/columns#pinned-columns and https://ag-grid.com/javascript-data-grid/column-pinning/. We could remove the need to add a custom background color on the pinned columns, improving customizability.
2. Add a custom logic for our pinned demos, to force the background color on the pinned columns to match the background color of the docs. This is a workaround for 1.
3. Do the opposite for 2. use the Material Design colors for the background of the demos in the docs. But this will fail with the Joy UI demos.
4. More?

[Janpot]

About 1. Just to note that there is a slight difference between the two implementations: mui-plus was inspired by ag-grid for the overflow behavior, but it kept the position of the horizontal scrollbar of MUI X.

4. we could also add the background to all the columns that are currently transparent

[cherniavskii]

It seems like there's an issue with custom theme in MUI X:

Here's window.theme:

And here's theme that is passed to pinned column element in https://github.com/mui/mui-x/blob/c5cfc7808523dd4b1d54f4a577710c79c5517c3d/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualScroller.tsx#L130

#121212 seems to come from default theme defined https://github.com/mui/material-ui/blob/660b9da2485257c80ba8be5e1079756ccb07f85d/packages/mui-material/src/styles/createPalette.js#L62

My guess is that we have more than 1 theme provider that messes it up

[joserodolfofreitas]

@Janpot's made a demo that also puts on evidence that the gray BG comes from docs theme, not the data grid.

[cherniavskii]

@joserodolfofreitas it's actually the other way around - grey comes from default Material UI theme, not our custom docs theme. This is intentional

The difference is that our demo sandbox doesn't use CssBaseline component, therefore there's no default background color. Here's grid with default dark theme with CssBaseline: https://codesandbox.io/s/agitated-dewdney-uixlzr Here's grid with default dark theme without CssBaseline: https://codesandbox.io/s/unruffled-black-8bw0p6?file=/demo.tsx <--- this is the reason why we see both blue and gray

[cherniavskii]

We could set background color on Grid root element (basically what @Janpot suggested in https://github.com/mui/mui-x/issues/5979#issuecomment-1233127091)

--- a/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts
+++ b/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts
@@ -89,6 +89,7 @@ export const GridRootStyles = styled('div', {
     height: '100%',
     display: 'flex',
     flexDirection: 'column',
+    backgroundColor: theme.palette.background.default,
     [`&.${gridClasses.autoHeight}`]: {
       height: 'auto',
       [`& .${gridClasses['row--lastVisible']} .${gridClasses.cell}`]: {

[Janpot]

If the demos are supposed to render with the default theme, shouldn't we add a <ScopedCssBaseline> around the demo sandbox?

[oliviertassinari]

shouldn't we add a <ScopedCssBaseline> around the demo sandbox?

@Janpot This CSS baseline component is supposed to be optional. I think that it's important to be optional, it's designed so components can be progressively adopted in a codebase where you can't apply global resets. We later introduced <ScopedCssBaseline> for cases where a whole subtree is using Material UI.

So I think that there is a significant advantage of having it not present in the sandbox: it helps finding cases where we are missing a default style.

[Janpot]

how can it be optional in dark mode if it's responsible for providing the background?

I've never not used CssBaseline

[cherniavskii]

I think that it's important to be optional

Besides that, it won't make a difference in the demo anyway, because it only sets background color of body

[oliviertassinari]

how can it be optional in dark mode if it's responsible for providing the background?

@Janpot The use cases I'm considering with "progressive adoption" is: I have an existing application, build with Bootstrap/Tailwind CSS. I need a data grid/a date picker, I can't use MUI's CSS Baseline, it would break the rest of my application. The applications the component is added into would already be taking care of setting the background for the dark mode.

We could set background color on Grid root element

@cherniavskii Very ingesting option. A few thoughts:

1. not a change to make in a patch release, a minor is a bit borderline but I think it would fly with semver.
2. It looks like it's still buggy in the screenshot, the background is not homogeneous, compared with MUI Plus and AG Grid.
3. It feels like we are missing a light box shadow/border to distinguish where the separation of the pinned columns is.

[Janpot]

Yep, progressive adoption makes sense. What I really wanted to get at is that the root of the issue here seems to be that we're missing a background in the demo sandbox.

We could set background color on Grid root element

One annoyance I could see with that is that if you want to put the DataGrid inside a Paper, you'll have to also wrap it in a ThemeProvider that sets the default background to the paper background of the surrounding theme.

[cherniavskii]

One annoyance I could see with that is that if you want to put the DataGrid inside a Paper, you'll have to also wrap it in a ThemeProvider that sets the default background to the paper background of the surrounding theme.

@Janpot If you use default theme - then both Paper and DataGrid will have same white background. If you use custom theme - then should already have ThemeProvider. Does it make sense?

[cherniavskii]

@oliviertassinari

1. not a change to make in a patch release, a minor is a bit borderline but I think it would fly with semver.

We can even do it in upcoming v6 :)

2. It looks like it's still buggy in the screenshot, the background is not homogeneous, compared with MUI Plus and AG Grid.

This is the current approach of highlighting pinned columns with a different background color.

3. It feels like we are missing a light box shadow/border to distinguish where the separation of the pinned columns is.

Yeah, we can experiment with shadows and borders instead. @gerdadesign would you like to look into it?

[Janpot]

If you use default theme - then both Paper and DataGrid will have same white background.

In the default dark mode, paper has a different background https://mui.com/material-ui/react-paper/#elevation

[cherniavskii]

In the default dark mode, paper has a different background mui.com/material-ui/react-paper/#elevation

But default theme is light, and to enable dark mode you have to create theme and use ThemeProvider. Please correct me if I'm wrong :)

[Janpot]

If the body background is palette.background.default and the Paper background is palette.background.paper, and the DataGrid background is palette.background.default, then wouldn't a DataGrid inside a Paper have the wrong background when rendered under the default dark theme? (or any custom theme that sets paper background different from default background)

My point was that one way to solve that is to wrap the DataGrid in its own ThemeProvider that sets the palette.background.default to the palette.background.paper of the surrounding theme.

Ofcourse, with a transparent DataGrid, the problem goes away entirely.

[gerdadesign]

It seems like there are two parts at play here. There's the question of this demo in the screenshot not showing up as expected and there's also an ask to be able to customize the way pinned columns display with a border?

Using shadows in light mode vs. different background color opacities in dark mode is how Material Design defines surfaces. In dark mode, shadows aren't as visible so they also adjusted the surface opacity.

It still seems to me that the core issue of the pinned column backgrounds in the demos is that there is a mix of styles. With Material's reasoning, the pinned column would have two backgrounds — 1. the same background as the rest of the grid 2. the lower opacity white on top to show elevation. But with the grid's transparent background, this lower opacity overlay should be sitting on top of the page background color instead of the current black.

I've created a few prototypes in Figma to show how I understand this is expected to work:

This mockup defines two versions: 1. the background of the pinned column + grid against black and MUI's dark blue. 2. using only a border on the pinned column and transparent background.

Interactive Figma prototype showing the expected functionality of changing page backgrounds with elevated background on the pinned column.

Interactive Figma prototype showing the expected functionality of changing page backgrounds with transparent background on the grid + border on the pinned column

[oliviertassinari]

With Material's reasoning, the pinned column would have two backgrounds

@gerdadesign Interesting, then I think that from "it looks like an accident", we could create a sub-issue that is about the default look & feel of Material UI. IMHO, the current one is not as optimal as it could be. I thought it was a bug. I'm expanding, when I look at the light mode, It feels like we want to use elevation=1:

https://mui.com/material-ui/react-paper/#elevation.

So when I look at the dark mode, I would expect the same:

The above screenshot feels better on my end. Note that I have updated the background of the docs page only for the sake of feeling how it will be for end-users. It was implemented with:

diff --git a/packages/grid/x-data-grid-pro/src/components/DataGridProColumnHeaders.tsx b/packages/grid/x-data-grid-pro/src/components/DataGridProColumnHeaders.tsx
index 6112388a9..2aa644d12 100644
--- a/packages/grid/x-data-grid-pro/src/components/DataGridProColumnHeaders.tsx
+++ b/packages/grid/x-data-grid-pro/src/components/DataGridProColumnHeaders.tsx
@@ -23,7 +23,7 @@ import {
   GridPinnedPosition,
   GridPinnedColumns,
 } from '../hooks/features/columnPinning';
-import { filterColumns } from './DataGridProVirtualScroller';
+import { filterColumns, getBoxShadow } from './DataGridProVirtualScroller';

 type OwnerState = {
   classes?: DataGridProProcessedProps['classes'];
@@ -52,8 +52,9 @@ interface GridColumnHeadersPinnedColumnHeadersProps {
   side: GridPinnedPosition;
 }

-// Inspired by https://github.com/material-components/material-components-ios/blob/bca36107405594d5b7b16265a5b0ed698f85a5ee/components/Elevation/src/UIColor%2BMaterialElevation.m#L61
-const getOverlayAlpha = (elevation: number) => {
+// TODO: MUI Core should make this public
+// A clone of https://github.com/mui/material-ui/blob/ac8b1ef2e68799de77f4607fb36c13cb6c2da309/packages/mui-material/src/Paper/Paper.js#L12-L21
+export const getOverlayAlpha = (elevation: number): number => {
   let alphaValue;
   if (elevation < 1) {
     alphaValue = 5.11916 * elevation ** 2;
@@ -78,16 +79,21 @@ const GridColumnHeadersPinnedColumnHeaders = styled('div', {
   zIndex: 1,
   display: 'flex',
   flexDirection: 'column',
-  boxShadow: theme.shadows[2],
   backgroundColor: theme.palette.background.default,
   ...(theme.palette.mode === 'dark' && {
-    backgroundImage: `linear-gradient(${alpha('#fff', getOverlayAlpha(2))}, ${alpha(
+    backgroundImage: `linear-gradient(${alpha('#fff', getOverlayAlpha(1))}, ${alpha(
       '#fff',
-      getOverlayAlpha(2),
+      getOverlayAlpha(1),
     )})`,
   }),
-  ...(ownerState.side === GridPinnedPosition.left && { left: 0 }),
-  ...(ownerState.side === GridPinnedPosition.right && { right: 0 }),
+  ...(ownerState.side === GridPinnedPosition.left && {
+    left: 0,
+    boxShadow: getBoxShadow(theme, ownerState.side),
+  }),
+  ...(ownerState.side === GridPinnedPosition.right && {
+    right: 0,
+    boxShadow: getBoxShadow(theme, ownerState.side),
+  }),
 }));

 interface DataGridProColumnHeadersProps extends React.HTMLAttributes<HTMLDivElement> {
diff --git a/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualScroller.tsx b/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualSc
roller.tsx
index 7acc3f904..a07c89ecb 100644
--- a/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualScroller.tsx
+++ b/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualScroller.tsx
@@ -97,9 +97,26 @@ const getOverlayAlpha = (elevation: number) => {
 };

 const getBoxShadowColor = (theme: Theme) => {
+  if (theme.palette.mode === 'dark') {
+    return '#000';
+  }
+
   return alpha(theme.palette.common.black, 0.21);
 };

+export const getBoxShadow = (theme: Theme, side: GridPinnedPosition) => {
+  const boxShadowColor = getBoxShadowColor(theme);
+  let boxShadow;
+
+  if (side === GridPinnedPosition.left) {
+    boxShadow = `2px 0px 4px -2px ${boxShadowColor}`;
+  } else if(side === GridPinnedPosition.right) {
+    boxShadow = `-2px 0px 4px -2px ${boxShadowColor}`;
+  }
+
+  return boxShadow;
+};
+
 const VirtualScrollerDetailPanels = styled('div', {
   name: 'MuiDataGrid',
   slot: 'DetailPanels',
@@ -122,7 +139,6 @@ const VirtualScrollerPinnedColumns = styled('div', {
     styles.pinnedColumns,
   ],
 })<{ ownerState: VirtualScrollerPinnedColumnsProps }>(({ theme, ownerState }) => {
-  const boxShadowColor = getBoxShadowColor(theme);
   return {
     position: 'sticky',
     overflow: 'hidden',
@@ -132,12 +148,12 @@ const VirtualScrollerPinnedColumns = styled('div', {
     ...(ownerState.side === GridPinnedPosition.left && {
       left: 0,
       float: 'left',
-      boxShadow: `2px 0px 4px -2px ${boxShadowColor}`,
+      boxShadow: getBoxShadow(theme, ownerState.side),
     }),
     ...(ownerState.side === GridPinnedPosition.right && {
       right: 0,
       float: 'right',
-      boxShadow: `-2px 0px 4px -2px ${boxShadowColor}`,
+      boxShadow: getBoxShadow(theme, ownerState.side),
     }),
   };
 });
diff --git a/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts b/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts
index 086d6eff1..bd93d6cc5 100644
--- a/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts
+++ b/packages/grid/x-data-grid/src/components/containers/GridRootStyles.ts
@@ -90,6 +90,7 @@ export const GridRootStyles = styled('div', {
     height: '100%',
     display: 'flex',
     flexDirection: 'column',
+    backgroundColor: theme.palette.background.default,
     [`&.${gridClasses.autoHeight}`]: {
       height: 'auto',
       [`& .${gridClasses['row--lastVisible']} .${gridClasses.cell}`]: {

The only limit is that the visual illusion doesn't work this well, on the edges that are at the border of the data grid.

[cherniavskii]

It doesn't seem to be the case anymore: https://mui.com/x/react-data-grid/column-pinning/ @joserodolfofreitas Should we close this issue?

[KenanYusuf]

It doesn't seem to be the case anymore: https://mui.com/x/react-data-grid/column-pinning/ @joserodolfofreitas Should we close this issue?

I thought so too but there is a subtle difference in color still 😅

I do think we should set a background color on the whole grid to avoid these cases.

[romgrk]

The problem isn't the color difference, it's that the hue/saturation doesn't match the background color. The color difference can be nice while we don't have the pinned shadows.

[KenanYusuf]

The color difference can be nice while we don't have the pinned shadows

Sure, if it's intentional.

My point is that we cannot safely assume that the data grid is displayed on a color that works well with our default pinned/container background color. i.e. it's a safer bet to present users this:

As opposed to this: