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

joserodolfofreitas · 2022-08-31T14:34:15Z

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:

https://mui.com/x/react-data-grid/column-pinning/
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

The text was updated successfully, but these errors were encountered:

gerdadesign · 2022-08-31T14:54:54Z

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 · 2022-08-31T15:14:37Z

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 · 2022-08-31T15:24:06Z

So, if we list the options, we have:

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.
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.
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.
More?

Janpot · 2022-08-31T15:58:32Z

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.

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

cherniavskii · 2022-08-31T16:13:00Z

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

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

Line 130 in c5cfc78

backgroundColor: theme.palette.background.default,

#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 · 2022-08-31T16:15:30Z

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

cherniavskii · 2022-08-31T16:31:09Z

@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 · 2022-08-31T16:48:03Z

We could set background color on Grid root element (basically what @Janpot suggested in #5979 (comment))

--- 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 · 2022-08-31T17:56:02Z

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

oliviertassinari · 2022-08-31T17:58:25Z

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 · 2022-08-31T18:08:44Z

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

I've never not used CssBaseline

cherniavskii · 2022-08-31T18:15:59Z

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 · 2022-08-31T18:28:43Z

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:

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

Janpot · 2022-09-01T09:20:18Z

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 · 2022-09-01T16:47:35Z

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 · 2022-09-01T16:55:44Z

@oliviertassinari

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 :)

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.

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 · 2022-09-01T20:56:47Z

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 · 2022-09-01T21:05:54Z

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 · 2022-09-02T06:57:34Z

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 · 2022-09-02T18:56:54Z

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 · 2022-09-03T22:49:43Z

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 · 2024-10-22T17:27:09Z

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 · 2024-10-22T18:31:06Z

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 · 2024-10-22T21:05:20Z

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 · 2024-10-23T07:48:56Z

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:

joserodolfofreitas changed the title ~~[data grid] Pinned columns color in dark mode looks like accident~~ [data grid] Pinned columns color in dark mode looks like an accident Aug 31, 2022

oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 31, 2022

joserodolfofreitas added docs Improvements or additions to the documentation bug 🐛 Something doesn't work and removed bug 🐛 Something doesn't work feature: Column pinning Related to the data grid Column pinning feature labels Aug 31, 2022

flaviendelangle changed the title ~~[data grid] Pinned columns color in dark mode looks like an accident~~ [data grid] Pinned columns / rows color in dark mode looks like an accident Sep 1, 2022

gerdadesign mentioned this issue Jul 10, 2023

[DataGrid] Experiment: Remove layout jump when start editing #9612

Draft

1 task

oliviertassinari mentioned this issue Aug 17, 2023

[DataGrid] Sticky headers #10059

Merged

9 tasks

oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ui labels Aug 18, 2023

KenanYusuf closed this as completed Oct 22, 2024

KenanYusuf reopened this Oct 22, 2024

mui deleted a comment from github-actions bot Oct 22, 2024

KenanYusuf mentioned this issue Nov 20, 2024

[data grid] Add a default background color to the data grid #15517

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

joserodolfofreitas commented Aug 31, 2022 •

edited by oliviertassinari

Loading

gerdadesign commented Aug 31, 2022

joserodolfofreitas commented Aug 31, 2022 •

edited

Loading

oliviertassinari commented Aug 31, 2022 •

edited

Loading

Janpot commented Aug 31, 2022 •

edited

Loading

cherniavskii commented Aug 31, 2022

joserodolfofreitas commented Aug 31, 2022

cherniavskii commented Aug 31, 2022

cherniavskii commented Aug 31, 2022

Janpot commented Aug 31, 2022

oliviertassinari commented Aug 31, 2022 •

edited

Loading

Janpot commented Aug 31, 2022 •

edited

Loading

cherniavskii commented Aug 31, 2022

oliviertassinari commented Aug 31, 2022 •

edited

Loading

Janpot commented Sep 1, 2022

cherniavskii commented Sep 1, 2022

cherniavskii commented Sep 1, 2022

Janpot commented Sep 1, 2022

cherniavskii commented Sep 1, 2022

Janpot commented Sep 2, 2022 •

edited

Loading

gerdadesign commented Sep 2, 2022

oliviertassinari commented Sep 3, 2022

cherniavskii commented Oct 22, 2024

KenanYusuf commented Oct 22, 2024

romgrk commented Oct 22, 2024

KenanYusuf commented Oct 23, 2024

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

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

Comments

joserodolfofreitas commented Aug 31, 2022 • edited by oliviertassinari Loading

Current behavior 😯

Expected behavior 🤔

Steps to reproduce 🕹

Context 🔦

Your environment 🌎

Order ID 💳 (optional)

gerdadesign commented Aug 31, 2022

joserodolfofreitas commented Aug 31, 2022 • edited Loading

oliviertassinari commented Aug 31, 2022 • edited Loading

Janpot commented Aug 31, 2022 • edited Loading

cherniavskii commented Aug 31, 2022

joserodolfofreitas commented Aug 31, 2022

cherniavskii commented Aug 31, 2022

cherniavskii commented Aug 31, 2022

Janpot commented Aug 31, 2022

oliviertassinari commented Aug 31, 2022 • edited Loading

Janpot commented Aug 31, 2022 • edited Loading

cherniavskii commented Aug 31, 2022

oliviertassinari commented Aug 31, 2022 • edited Loading

Janpot commented Sep 1, 2022

cherniavskii commented Sep 1, 2022

cherniavskii commented Sep 1, 2022

Janpot commented Sep 1, 2022

cherniavskii commented Sep 1, 2022

Janpot commented Sep 2, 2022 • edited Loading

gerdadesign commented Sep 2, 2022

oliviertassinari commented Sep 3, 2022

cherniavskii commented Oct 22, 2024

KenanYusuf commented Oct 22, 2024

romgrk commented Oct 22, 2024

KenanYusuf commented Oct 23, 2024

joserodolfofreitas commented Aug 31, 2022 •

edited by oliviertassinari

Loading

joserodolfofreitas commented Aug 31, 2022 •

edited

Loading

oliviertassinari commented Aug 31, 2022 •

edited

Loading

Janpot commented Aug 31, 2022 •

edited

Loading

oliviertassinari commented Aug 31, 2022 •

edited

Loading

Janpot commented Aug 31, 2022 •

edited

Loading

oliviertassinari commented Aug 31, 2022 •

edited

Loading

Janpot commented Sep 2, 2022 •

edited

Loading