search

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.52k stars 1.31k forks source link

[data grid] Unexpected cell text selection highlighting area in v7 in Chromium Browsers #13018

Closed wladimir-k-m closed 5 months ago

wladimir-k-m commented 6 months ago

Steps to reproduce

Steps:

  1. Select text in any cell in v7, e.g. here: https://mui.com/x/react-data-grid/layout/#predefined-dimensions
  2. Notice the text selection area has the same height as the cell: image

In v6 the text selection area is as expected: https://v6.mui.com/x/react-data-grid/layout/#predefined-dimensions image

Current behavior

No response

Expected behavior

The same minimum bounding box for text selection highlighting as in v6.

Context

No response

Your environment

The issue seems to happen only in Chromium based browsers. Tested: Chrome: Version 124.0.6367.119 Edge: Version 124.0.2478.80

It looks fine in Firefox.

Search keywords: data grid, text selection, text highlighting Order ID: 83276

michelengelen commented 6 months ago

Hey @wladimir-k-m thanks for raising this issue. The adjusted area size comes from a change we introduced in d9ac050aea0b9a03a959a67e6a71c923d5562351. It was meant to increase performance of the cell rendering. Not sure about the performance implication of this specific change. That is something @romgrk would be best to answer here.

michelengelen commented 6 months ago

Just a quick question @wladimir-k-m: Does this prove to be a problem in your case?

wladimir-k-m commented 6 months ago

@michelengelen It looks odd and is inconsistent across browsers. It's not a show stopper of course but it does lower the visual quality of an otherwise nice looking grid component.

michelengelen commented 6 months ago

OK, so it is basically a cosmetic issue (with a slight dent in a11y). I will add this to the board, so we can check how to improve this.

romgrk commented 5 months ago

The change in cell structure is similar to what other grid components have. I have inspected a bit what the other components are doing to deal with text selection: most of them just don't allow native text selection. Any selection goes through grid cell/row selection: https://mui.com/x/react-data-grid/cell-selection/. It's just that we allow text selection when there is no row/cell selection.

The line-height is set to 100% height to vertically align text, if we can find an alternative to that we can remove the line-height.

romgrk commented 5 months ago

But other grids are also having the same behavior: https://ag-grid.com/react-data-grid/selection-overview/#cell-text-selection

It would be possible for users to re-add a wrapper using renderCell, but that has performance implications in particular for scrolling.

wladimir-k-m commented 5 months ago

Ok, thanks for the investigation @romgrk. If the visual change has performance implications I think it's better to leave it the way it is. You can close the issue.

github-actions[bot] commented 5 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.

@wladimir-k-m: 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.