mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.53k stars 1.31k forks source link

[data grid] First row is 1px taller than other rows #14195

Closed janeklb closed 2 months ago

janeklb commented 3 months ago

Steps to reproduce

Link to live example: (required)

Steps:

  1. Go to https://mui.com/x/react-data-grid/
  2. Measure the height of the 1st row of any of the examples (do not look at css and/or DOM element dimensions, but actually count the pixels)
  3. Measure the height of any of the other rows (in the same example)

Current behavior

The 1st row is one pixel taller / higher than the other rows in the datagrid.

eg. 1st row in this example has height 52 pixels (measured by taking a screenshot and uploading to https://pixlr.com/editor/ and then using the marquee to select the row height, and then taking a screenshot of that :phew:)

image

and 2nd row has a height of 51 pixels (same as all subsequent rows)

image

Expected behavior

All rows have an equal height (when not using custom / variable row heights)

(clearly this is a minor issue, if it can even be called an issue.. but imo worth noting nonetheless)

Context

No response

Your environment

npx @mui/envinfo - Tested with latest version of Chrome and Brave ``` System: OS: macOS 14.5 Binaries: Node: 20.15.0 - ~/.nvm/versions/node/v20.15.0/bin/node npm: 10.8.2 - ~/xxxxxxxxx/.bin/npm pnpm: Not Found Browsers: Chrome: 127.0.6533.100 Edge: Not Found Safari: 17.5 npmPackages: @emotion/react: 11.11.4 => 11.11.4 @emotion/styled: 11.11.5 => 11.11.5 @mui/base: 5.0.0-beta.40 @mui/core-downloads-tracker: 5.16.6 @mui/icons-material: 5.16.4 @mui/material: ^5.16.6 => 5.16.6 @mui/private-theming: 5.16.6 @mui/styled-engine: 5.16.6 @mui/system: ^5.16.6 => 5.16.6 @mui/types: 7.2.15 @mui/utils: 5.16.6 @mui/x-data-grid: 7.12.0 @mui/x-data-grid-pro: ^7.12.0 => 7.12.0 @mui/x-date-pickers: 7.12.0 @mui/x-date-pickers-pro: ^7.12.0 => 7.12.0 @mui/x-internals: 7.12.0 @mui/x-license: 7.12.0 @mui/x-license-pro: 6.10.2 @types/react: ^17.0.44 => 17.0.80 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 typescript: 5.4.5 => 5.4.5 ```

Search keywords: datagrid row height Order ID: 50436

michelengelen commented 2 months ago

Hey @janeklb I am not able to reproduce this. My screenshots are both at 104px height (retina display), so the values are correct. Could you maybe re-assess and see if you can do this in a clean browser with no plugins? Sometimes those can mess with the layout. Thanks!

janeklb commented 2 months ago

Here's the result with Chrome in incognito mode w/o any extensions (with a retina display)

image

Im measuring 104px height (between the borders) on the 1st row, and 102px between the borders of the subsequent ones.

Same result with safari:

image

arminmeh commented 2 months ago

I think that I found the reason why this happens If you inspect the styles, all rows are 52px, but if you check the cells inside, you can see that in the first row the cell border is transparent, while for the other rows, cells have a visible border. The first one is transparent to avoid creating double border with the header row. We might add negative margin to overlap these two, but it should be checked how we calculate render context and if that update will create a case where that calculation might get wrong

michelengelen commented 2 months ago

@arminmeh That's exactly what happens. I did cut a screenshot and moved the second over the first row and there it shows clearly:

Screenshot 2024-08-15 at 10 40 31

Fixing this is not all that trivial though. Removing the border still keeps the size the same (it basically renders just the same as before) due to the border-box sizing-model. Not sure if we can provide a good enough fix for that.

The only viable solution I found is moving the virtualRenderZone 1px up:

diff --git a/packages/x-data-grid/src/components/virtualization/GridVirtualScrollerRenderZone.tsx b/packages/x-data-grid/src/components/virtualization/GridVirtualScrollerRenderZone.tsx
index 4ecd09eaf..7928134b9 100644
--- a/packages/x-data-grid/src/components/virtualization/GridVirtualScrollerRenderZone.tsx
+++ b/packages/x-data-grid/src/components/virtualization/GridVirtualScrollerRenderZone.tsx
@@ -52,6 +52,7 @@ const GridVirtualScrollerRenderZone = React.forwardRef<
       className={clsx(classes.root, className)}
       ownerState={rootProps}
       style={{
+        marginTop: '-1px',
         transform: `translate3d(0, ${offsetTop}px, 0)`,
       }}
       {...other}

Thoughts @arminmeh ?

arminmeh commented 2 months ago

@michelengelen that looks fine to me We can also remove transparent border color from the styles so that it becomes visible why this is done, in case someone want to remove/refactor the margin later

michelengelen commented 2 months ago

Closing this as we discovered that the issue does not justify the downsides to it. Reference: https://github.com/mui/mui-x/pull/14229#issuecomment-2293533225

github-actions[bot] commented 2 months ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@janeklb: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

janeklb commented 2 months ago

Fair enough. Thank you for taking a look!