-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[docs] Add a recipe for drag and drop row grouping #17638
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
Conversation
Thanks for adding a type label to the PR! 👍 |
Please add one type label to categorize the purpose of this PR appropriately:
|
Not sure a recipe fits into any of these categories 🤷 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very nice 👍🏻
}; | ||
|
||
return ( | ||
<Chip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we disable these chips when pivot mode is active? They won't work anyway.
docs/data/data-grid/recipes-row-grouping/RowGroupingToolbar.tsx
Outdated
Show resolved
Hide resolved
|
||
return ( | ||
<div style={{ height: 400, width: '100%' }}> | ||
<DataGridPremium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a much better UX if you could use it with useKeepGroupedColumnsHidden
so that when a column gets added to the toolbar, it gets removed from the list of columns and you don't end up with duplicates.
The issue is when a column header gets added to the toolbar on drag over, it is no longer in the DOM, and so the onDragEnd
event does not fire. In the demo below you can see how the dragged column state does not get reset.
Screen.Recording.2025-05-02.at.16.40.58.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is when a column header gets added to the toolbar on drag over, it is no longer in the DOM, and so the onDragEnd event does not fire.
Is this because we add grouping criteria on dragOver?
I was actually wondering if adding grouping criteria on dragEnd would be a better UX 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually wondering if adding grouping criteria on dragEnd would be a better UX 🤔
Wouldn't it have to be onDrop
on the toolbar? If so, we encounter the same issue 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add an API method that we could call in user-land to reset the dragged column state once the column header has been dropped in the toolbar, but it feels very niche and not like something that should be in the public API.
I'm wondering if we should just build this feature into the grid properly so we can share state between the two features and have private API methods where necessary. We could provide a better and more complete UX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Together with @cherniavskii, we found that if we use a regular dragend
event listener to run the handleDragEnd
function, the state properly resets when a column header is dropped on the toolbar.
@@ -0,0 +1,28 @@ | |||
/* eslint-disable no-bitwise */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
const handleKeyDown = ( | ||
event: React.KeyboardEvent<HTMLDivElement>, | ||
) => { | ||
if (event.key === 'ArrowRight' && event.shiftKey) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor weird behavior I noticed: when reordering with the Shift + Arrow key, the navigation order of keys gets reversed, which is fixed when reordering again with the Shift + Arrow key.
Screen.Recording.2025-05-20.at.12.16.17.AM.mov
I suspect focusableItem.index
is not getting properly updated in the Toolbar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very strange... it doesn't behave like this in dev mode locally, only on the built version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect focusableItem.index is not getting properly updated in the Toolbar.
This was indeed the problem, I couldn't find a nice way of reliably tracking the order of toolbar items — instead, I've updated it to get the sort order at the various points it is needed.
keyboard.mp4
docs/data/data-grid/recipes-row-grouping/recipes-row-grouping.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Bilal Shafi <[email protected]> Co-authored-by: Andrew Cherniavskii <[email protected]> Signed-off-by: Kenan Yusuf <[email protected]>
Adds a recipe for #5235.
Preview: https://deploy-preview-17638--material-ui-x.netlify.app/x/react-data-grid/components/toolbar/#row-grouping-bar