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

[data grid] Incorrect cell width calculation when using css border #15472

Open
DanielaDark opened this issue Nov 18, 2024 · 7 comments
Open

[data grid] Incorrect cell width calculation when using css border #15472

DanielaDark opened this issue Nov 18, 2024 · 7 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability MUI X status: waiting for author Issue with insufficient information support: commercial Support request from paid users support: question Community support but can be turned into an improvement

Comments

@DanielaDark
Copy link

DanielaDark commented Nov 18, 2024

Steps to reproduce

Steps:

  1. Open this link to live example: (https://codesandbox.io/p/sandbox/winter-cache-c9vdl3)
SCR-20241119-bedg
  1. Either examine the css properties of cells or observe that the cell border (blue) on right side and bottom is not visible.

Current behavior

When adding css border to row and cell, cell width does not change. As a result cells are wider than provided width.
In provided example Table width is 600px. Row has 1px border, the width of row is 598px (correct behavior).
There are 2 cells in a row, each with 1px border. First cell width 200px (incorrect, should be 198px). Second cell has flex = 1 property, calculated width 398px (incorrect, should be 396px).

Expected behavior

When adding css border to row and cell, cell width is correctly calculated. Meaning cell width shrinks when cell border is added.

Context

Our Style-Guidelines require that both rows and cells have borders. Rows when they are selected and cells when keyboard focus is applied, as example for keyboard navigation. Having both properties active is in our case not unusual.

Your environment

npx @mui/envinfo
Browser used: Firefox

System:
    OS: Windows 10 10.0.19045
  Binaries:
    Node: 14.17.0 - C:\Program Files\nodejs\node.EXE
    npm: 6.14.13 - C:\Program Files\nodejs\npm.CMD
    pnpm: Not Found
  Browsers:
    Chrome: 130.0.6723.117
    Edge: Chromium (131.0.2903.51)
  npmPackages:
    @emotion/react: ^11.10.6 => 11.11.4
    @emotion/styled: ^11.10.6 => 11.11.5
    @mui/base:  5.0.0-alpha.124
    @mui/core-downloads-tracker:  5.15.18
    @mui/icons-material: 5.11.16 => 5.11.16
    @mui/lab: 5.0.0-alpha.125 => 5.0.0-alpha.125
    @mui/material: 5.11.16 => 5.11.16
    @mui/private-theming:  5.15.14
    @mui/styled-engine:  5.15.14
    @mui/styles: 5.11.16 => 5.11.16
    @mui/system:  5.15.15
    @mui/types:  7.2.14
    @mui/utils:  5.15.14
    @mui/x-data-grid:  6.9.2
    @mui/x-data-grid-premium: 6.9.2 => 6.9.2
    @mui/x-data-grid-pro:  6.9.2
    @mui/x-license-pro:  6.9.0
    @types/react: ^18.0.26 => 18.3.2
    react: ^18.2.0 => 18.3.1
    react-dom: ^18.2.0 => 18.3.1
    typescript: 4.4.3 => 4.4.3

Search keywords: DataGrid width
Order ID: 99555

Search keywords:

@DanielaDark DanielaDark added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 18, 2024
@samuelsycamore
Copy link
Contributor

samuelsycamore commented Nov 18, 2024

Is this what you're going for? (I added padding to make it easier to see the borders.)

https://codesandbox.io/p/sandbox/epic-hodgkin-z2gl5d?file=%2Fsrc%2Findex.tsx%3A20%2C26

Check out the MDN box-sizing docs for more info.

(I should note that this is the repo for Material UI, which is separate from the MUI X repo where the Data Grid lives; and we generally reserve GitHub issues for bug reports and feature requests. In the future, if you find a bug in the Data Grid, the MUI X repo is the best place to report it. If you need help with implementation then Stack Overflow is usually the best place to go—be sure to add the mui-x-data-grid tag to your post so others can find it.)

@samuelsycamore samuelsycamore added MUI X support: question Community support but can be turned into an improvement and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 18, 2024
@oliviertassinari oliviertassinari transferred this issue from mui/material-ui Nov 18, 2024
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Nov 18, 2024
@DanielaDark
Copy link
Author

DanielaDark commented Nov 19, 2024

Thank you @samuelsycamore for your fast response!
No, unfortunately adding padding does not solve the problem. The problem is that the contents of the grid do not scale down correctly to fit in the grid. For me it looks like setting the "flex=1" property does not calculate correct cell width when row border is added. (sandbox)
Example both borders:
grafik
Example only row border.
grafik

P.S. Thank you for your hint about the second repo. Should I close this issue here and reopen it in the other repo?

@samuelsycamore
Copy link
Contributor

Sorry for the confusion with the padding—I just added it to make it easier to see the borders with the box-sizing properties, not to solve the issue per se. But I see what you're saying now:box-sizing still doesn't resolve the issue on the right side of the container. Thanks for pointing that out.

Looks like this issue was moved to the correct repo so no action is needed there. I'll leave this for the Data Grid maintainers to take a look at.

@samuelsycamore samuelsycamore changed the title Incorrect cell width calculation when using css border in DataGrid [data grid] Incorrect cell width calculation when using css border Nov 19, 2024
@samuelsycamore samuelsycamore added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: commercial Support request from paid users labels Nov 19, 2024
@michelengelen
Copy link
Member

Mmh, this works for me:

export const CustomThemeOptions = createTheme({
  components: {
    MuiDataGrid: {
      styleOverrides: {
        root: {
          border: "1px solid grey",
        },
        row: {
          boxSizing: "border-box",
          borderWidth: "0px 3px",
          borderColor: "blue",
          borderStyle: "solid",
        },
      },
    },
  },
});

Although I am not sure why we do not set box-sizing: border-box on a row ... might have something to do with the calculations for virtualization.

@romgrk do you know if this might prove to be a problem if we set that property?

@michelengelen michelengelen added customization: css Design CSS customizability status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 20, 2024
@DanielaDark
Copy link
Author

@michelengelen, you are absolutely right, that when only row gets a border, your provided styling works. When also cell gets a Border, it does not work properly.

export const CustomThemeOptions = createTheme({
  components: {
    MuiDataGrid: {
      styleOverrides: {
        row: {
          border: "1px solid red",
          boxSizing: "border-box",
        },
        cell: {
           border: "1px solid blue",
           boxSizing: "border-box",
        },
      },
    },
  },
});

Result:
grafik

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Nov 28, 2024
@michelengelen
Copy link
Member

@michelengelen, you are absolutely right, that when only row gets a border, your provided styling works. When also cell gets a Border, it does not work properly.

export const CustomThemeOptions = createTheme({
  components: {
    MuiDataGrid: {
      styleOverrides: {
        row: {
          border: "1px solid red",
          boxSizing: "border-box",
        },
        cell: {
           border: "1px solid blue",
           boxSizing: "border-box",
        },
      },
    },
  },
});

Result: grafik

All right ... that's correct. we do calculate the width manually and apply it in our rendering process as a CSS variable ... I am not sure if we can change that behavior, so. the best course of action for you would be to use box shadow as a border.

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 2, 2024
@romgrk
Copy link
Contributor

romgrk commented Dec 2, 2024

Sorry I missed this, yes the grid needs to do its own calculations for virtualization, we can't allow borders that haven't been included in the calculations (e.g. with showCellVerticalBorder). You can also superimpose a &::after { position: absolute } element and apply the border on that one, that won't affect layout, but box-shadow or outline would be leaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability MUI X status: waiting for author Issue with insufficient information support: commercial Support request from paid users support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

5 participants