[DataGrid] Support scrolling over column headers under experimental flag #8343
Conversation
cherniavskii
added
component: data grid
Netlify deploy previewNetlify deploy preview: https://deploy-preview-8343--material-ui-x.netlify.app/ Updated pagesNo updates. These are the results for the performance tests:
|
|
What do you think @mui/xgrid ? |
There was a problem hiding this comment.
It's a nice improvement in my opinion, especially if we are shipping it as an experimental feature, I think it won't hurt anyone, rather it could be quite useful in scenarios when header grows in height, let's say with multiple level column grouping or filtering on header row.
One thing I was wondering about. If we could make it work on mobile screens, it'll be great addition too. Currently, it doesn't seem to work great on mobile with header row as it does for normal rows.
| } | ||
|
|
||
| if (event.deltaX !== 0) { | ||
| event.preventDefault(); |
There was a problem hiding this comment.
If the preventDefault is to prevent going to the previous page if the user scrolls left (when already on the leftmost position), I'll vote not to add this, as this will negate the default browser behavior/user expectation.
For example when I try to scroll to the left when already on the leftmost column in the virtual scroller it goes to the previous page, the same is the case somewhere on the page (outside the grid), so my expectation would be the same for the header area too.
There was a problem hiding this comment.
The issue is that deltaX may not be equal to 0, even if the element is not scrolling. This is causing preventDefault to be called unnecessarily every time. To solve this problem, we can replace deltaX with prevScrollLeft.current, which holds the previous scroll position. This ensures that preventDefault is only called when the element is actually scrolling.
if (prevScrollLeft.current !== 0)
event.preventDefault();bandicam.2023-03-24.15-55-35-983.mp4
There was a problem hiding this comment.
I tried to call preventDefault conditionally, and it kind of works, but sometimes calling preventDefault doesn't prevent the default browser action:
Screen.Recording.2023-04-28.at.14.16.05.mov
It's very unreliable and I see two options here:
- Roll back to always call
preventDefault - Close this PR and potentially look for a better solution to the problem
I would prefer option 1, because it's better than nothing, but I understand that it's not perfect :)
What do you think?
@MBilalShafi
@joserodolfofreitas
There was a problem hiding this comment.
That's a great addition indeed. Thanks for pushing this forward!
I personally find it quite annoying not to be able to scroll horizontally over column headers.
In fact, I believe it should be the default behavior (but on v7).
Can we push to release it? Why is it on draft yet?
|
Ah, we'll need docs for it, too, of course! |
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
oliviertassinari
added
design
|
Any news on whether this is happening? I would find it very useful as the default behaviour doesn't feel right. |
|
It's already done thanks to the #10059 feature, we may want to close this PR now. |
Thank you very much for letting me know! |
|
Closed by #10059 |
This is an experiment I suggested previously in #5416 (comment)
We can release it under the
scrollOverColumnHeadersexperimental flag if the team agrees it's a good idea.For demonstration purposes, the feature is enabled by default in this PR.
Preview: https://deploy-preview-8343--material-ui-x.netlify.app/x/react-data-grid/scrolling/