mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[DataGrid] Fix `GridPanelAnchor` positioning

Open k-rajat19 opened this issue 1 year ago • 2 comments

Closes #15005

Before: https://codesandbox.io/p/sandbox/condescending-tereshkova-ytknq9 After: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-sh3fw2?file=%2Fsrc%2Fdemo.tsx

k-rajat19 avatar Oct 18 '24 19:10 k-rajat19

Deploy preview: https://deploy-preview-15022--material-ui-x.netlify.app/

Generated by :no_entry_sign: dangerJS against 971a563fc4e61f31588105aacc228bc5deb07815

mui-bot avatar Oct 18 '24 19:10 mui-bot

The change makes sense, but if we look at the root, it seems strange that we need this gridPanelAnchor DOM node. I wonder if we couldn't remove it and instead rely on an existing DOM, e.g. .MuiDataGrid-topContainer.

oliviertassinari avatar Oct 19 '24 21:10 oliviertassinari

Hi @oliviertassinari , this approach has been decided to use here before that we relied on column header DOM which is inside the top container. and I feel like we should stick to the current approach, though I haven't given any try to whether we can get rid of this extra element or not.

k-rajat19 avatar Oct 20 '24 15:10 k-rajat19

this approach has been decided to use here

Ok, so the child .MuiDataGrid-columnHeaders doesn't work. Now, why would its parent .MuiDataGrid-topContainer not work?

SCR-20241020-segk

oliviertassinari avatar Oct 20 '24 18:10 oliviertassinari

On surface it looks like this approach was chosen because of the positioning issues of the preference panel if we reference panel to the top container, then in case of horizontal scroll panel will be scrolled also and becomes invisible. @romgrk can you confirm this? if you still remember the details since the change was made more than one year ago 😅

k-rajat19 avatar Oct 21 '24 06:10 k-rajat19

@k-rajat19 Correct – topContainer didn't work, that's why I added panel anchor:

cherniavskii avatar Oct 21 '24 10:10 cherniavskii

Ok, thanks, so nothing else can be used here, nice regression fix 👍.


Off-topic. One thing we could consider in the future is to change the anchor of the filter panel bested on the trigger. There is quite a long distance for the mouse to travel: https://mui.com/x/react-data-grid/filtering/

https://github.com/user-attachments/assets/8166c209-6e9c-463c-8787-5b55067655b7

Related to https://github.com/mui/mui-x/issues/6419.

oliviertassinari avatar Oct 21 '24 14:10 oliviertassinari

So there is no solution?

93carlosmesa avatar Oct 22 '24 07:10 93carlosmesa

Off-topic. One thing we could consider in the future is to change the anchor of the filter panel bested on the trigger. There is quite a long distance for the mouse to travel: https://mui.com/x/react-data-grid/filtering/

The various grid management forms are all rendered inside a single panel. That logic is ugly and complex for no reason. Each toolbar button should manage its own popover panel, like a Select does, then they would open right below them.
And the grid menus can still open the panel centered at the anchor.

romgrk avatar Oct 26 '24 11:10 romgrk

So there is no solution?

This PR should have fixed your issue, test with the latest version.

romgrk avatar Oct 26 '24 11:10 romgrk