[DataGrid] Fix column resize not working with special character #13069
Conversation
oukunan
force-pushed
the
fix-column-resize-special-characters
branch
from
059ce2f
to
23c6693
Compare
oukunan
changed the title
oukunan
changed the title
zannager
added
the
component: data grid
|
Deploy preview: https://deploy-preview-13069--material-ui-x.netlify.app/ |
packages/x-data-grid/src/hooks/features/columnResize/useGridColumnResize.tsx
Outdated
Show resolved
Hide resolved
|
@cherniavskii I'll let you approve & merge. |
oliviertassinari
changed the title
packages/x-data-grid/src/hooks/features/columnResize/useGridColumnResize.tsx
Outdated
Show resolved
Hide resolved
| // HACK: The jsdom test is failing because CSS.escape doesn't exist in that context. | ||
| // We have decided that we want to preserve the bug in jsdom | ||
| // https://github.com/mui/mui-x/pull/13069#discussion_r1603270444 | ||
| const escapedColDefField = typeof CSS === 'undefined' ? colDef.field : CSS.escape(colDef.field); |
There was a problem hiding this comment.
This is inconsistent with #2033. To normalize to have one solution for one problem.
There was a problem hiding this comment.
Agreed, use the existing escapeOperandAttributeSelector function instead of CSS.escape.
There was a problem hiding this comment.
Why not standardize on CSS.escape?
There was a problem hiding this comment.
Alright, I have added this commit ed3472b as the behavior might not be 100% correct right now looking at https://github.com/mathiasbynens/CSS.escape/blob/master/css.escape.js.
The spec https://drafts.csswg.org/cssom/#the-css.escape()-method
There was a problem hiding this comment.
I did argue we should include a ponyfill for CSS.escape but I was defeated. Read the full discussion here. We did go for the existing escape method here though, which made the whole discussion irrelevant as we already had it in the codebase.
There was a problem hiding this comment.
Right, so from what I understand, there are two distinct discussions:
- Using
escapeOperandAttributeSelectoris incorrect, it should useCSS.escape. ButescapeOperandAttributeSelectorhas been good enough so far, so we continue with is, waiting for the problem to be surfaced. Simpler for now. When will be the right time? - jsdom doesn't support
CSS.escapeby default. Should the data grid do something about it? The strategy used so far has been to support jsdom with no need for custom setup for developers in Material UI, Base UI, MUI X. e.g. [data grid] Tests fail with: "IntersectionObserver is not defined" after upgrading to x-data-grid v7.3.2 #12983. Should we change this?
oukunan
force-pushed
the
fix-column-resize-special-characters
branch
from
c65edbb
to
0ee6603
Compare
|
@oukunan Could you merge master in this branch? That should fix the tests. |
oukunan
force-pushed
the
fix-column-resize-special-characters
branch
from
0ee6603
to
97cb2a0
Compare
|
Hello @romgrk, this branch was rebased using master. Could you verify this branch again? |
Head branch was pushed to by a user without write access
oukunan
force-pushed
the
fix-column-resize-special-characters
branch
from
78b6894
to
9f91302
Compare
|
Thanks for the PR! |
LukasTy
changed the title
Closes: #12954
The support of the CSS API seems common enough: https://caniuse.com/mdn-api_css_escape_static.Before: https://codesandbox.io/p/sandbox/wild-surf-ygtjk6
After: https://codesandbox.io/p/sandbox/upbeat-grass-9h7tfd