Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,26 @@ const GridColumnMenuContainer = forwardRef<HTMLUListElement, GridColumnMenuConta
[hideMenu],
);

const handleMenuScrollCapture = React.useCallback(
(event: React.WheelEvent | React.TouchEvent) => {
if (!event.currentTarget.contains(event.target as Node)) {
return;
}

event.stopPropagation();
},
[],
);

return (
<StyledMenuList
as={rootProps.slots.baseMenuList}
id={id}
className={clsx(gridClasses.menuList, className)}
aria-labelledby={labelledby}
onKeyDown={handleListKeyDown}
onWheel={handleMenuScrollCapture}
onTouchMove={handleMenuScrollCapture}
autoFocus={open}
{...other}
ref={ref}
Expand Down
39 changes: 38 additions & 1 deletion packages/x-data-grid/src/tests/columnHeaders.DataGrid.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { createRenderer, screen, waitFor, within } from '@mui/internal-test-utils';
import { createRenderer, fireEvent, screen, waitFor, within } from '@mui/internal-test-utils';
import { expect } from 'chai';
import { DataGrid } from '@mui/x-data-grid';
import { getColumnHeaderCell, getColumnHeadersTextContent } from 'test/utils/helperFn';
Expand Down Expand Up @@ -132,6 +132,43 @@ describe('<DataGrid /> - Column headers', () => {
expect(screen.queryByRole('menu')).to.equal(null);
});
});

it('should prevent wheel scroll event from closing the menu when scrolling within the menu', async () => {
const { user } = render(
<div style={{ width: 300, height: 500 }}>
<DataGrid {...baselineProps} columns={[{ field: 'brand' }]} />
</div>,
);

await user.click(within(getColumnHeaderCell(0)).getByLabelText('brand column menu'));
const menu = screen.getByRole('menu');

fireEvent.wheel(menu);

await waitFor(() => {
expect(screen.queryByRole('menu')).not.to.equal(null);
});
});

it('should close the column menu when the grid is scrolled', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it shouldn't close the menu at all on scroll, seems unconventional to me.

@mui/xgrid keen to get others thoughts on this behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried to get the reason why it was added in the first place
closest I got is this
#4139 (comment)

The column menu should be closed if the user scrolls the grid, that's the rule.

still don't have the answer why exactly

AG Grid does not close the menu on scroll
https://www.ag-grid.com/react-data-grid/filter-advanced/#enabling-advanced-filter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not a great UX if we don't close the menu on horizontal scroll – it first leaves the data grid viewport, and then disappears anyway due to virtualization.
I don't see a reason for closing the menu on vertical scroll, though. Should we keep closing the menu for horizontal scroll only?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On another look, I think that we can leave it as is (only including changes made in this PR)
If you use premium grid with all major features enabled, the menu can get pretty tall and in most cases it will cover all the content from the related column. For me, in that situation, I would prefer to have the current behavior

Screenshot 2025-06-10 at 08 34 44

const { user } = render(
<div style={{ height: 300, width: 300 }}>
<DataGrid
rows={Array.from({ length: 50 }, (_, id) => ({ id, name: id }))}
columns={[{ field: 'brand' }]}
/>
</div>,
);

await user.click(within(getColumnHeaderCell(0)).getByLabelText('brand column menu'));

const scroller = document.querySelector('.MuiDataGrid-virtualScroller')!;
fireEvent.wheel(scroller, { deltaY: 120 });

await waitFor(() => {
expect(screen.queryByRole('menu')).to.equal(null);
});
});
});

it("should use 'headerName' as the aria-label for the menu icon button", async () => {
Expand Down
Loading