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

[data grid] Missing `cellMouseLeave` in `apiRef.subscribeEvent`

Open marcpachecog opened this issue 9 months ago • 4 comments

Summary

I'm trying to show a Popup when the text of a cell is truncated and found that there are no cellMouseLeave event like the one that we have for the rowsrowMouseLeave. The rationale behind the use of the apiRef subscribeEvent() instead of slotProps={{ cell: { onMouseLeave } }} is to prevent rerenders.

Examples

No response

Motivation

No response

Search keywords: cellMouseLeave Order ID: 77729

marcpachecog avatar May 08 '24 11:05 marcpachecog

Hey @marcpachecog and thanks for raising this. It's actually a good question why we did not implement a complimentary event listener for mouseOver.

I will add this to the board.

On another note: Why did we go for mouseOver and not mouseEnter? mouseOver will "drill down" the handler to all children and this could potentially impact performance in complicated cell renderers. This W3Schools example schowcases it really well

WDYT @mui/xgrid ?

michelengelen avatar May 08 '24 12:05 michelengelen

This relates to this page

TLDR: We removed those events from apiRef.subscribeEvent so that only people that need them pay the performance price of setting the event on the HTML element. So adding them back is probably not a good solution.

But maybe we can improve the doc or the behaviors on how people can pass other event listeners to a cell / a row.

flaviendelangle avatar May 13 '24 09:05 flaviendelangle

Feels inconsistent that we publish cellMouseOver but not cellMouseLeave.

so that only people that need them pay the performance price of setting the event on the HTML element.

@flaviendelangle I'm curious about the performance justification, isn't react supposed to use event bubbling and only attach event listeners to the root of the tree? Something like this: https://blog.logrocket.com/event-bubbling-capturing-react/ Either way it's good that we remove unused props though.

@marcpachecog Wdym by "prevent rerenders"? You shouldn't be passing inline objects directly, maybe this section of the docs would help: https://mui.com/x/react-data-grid/performance/#extract-static-objects-and-memoize-root-props

romgrk avatar May 15 '24 17:05 romgrk

Feels inconsistent that we publish cellMouseOver but not cellMouseLeave.

I think we just kept the one we were using internally

@flaviendelangle I'm curious about the performance justification, isn't react supposed to use event bubbling and only attach event listeners to the root of the tree? Something like this: https://blog.logrocket.com/event-bubbling-capturing-react/

It was the justification when the change was done, but I never explored the topic tbh. If we publish an event even if there is no-one listening to it, so for event very frequently published (like cellMouseLeave / cellMouseLeave) we have at least the cost of running publishEvent, but this seems pretty low (and cellMouseOver is called a lot more).

Could be worth exploring the performance cost if we want to add some of those events back, but the slotProps approach should work fine as well.

flaviendelangle avatar May 16 '24 07:05 flaviendelangle

The issue has been inactive for 7 days and has been automatically closed.

github-actions[bot] avatar May 23 '24 15:05 github-actions[bot]

I added the waiting-for-author tag because there was a question for @marcpachecog in my last comment, and the other comments pointed out good alternatives. Feel free to reopen.

romgrk avatar May 23 '24 16:05 romgrk