[TreeView] `onClick` on RichTreeView component is invoked twice about mui-x HOT 8 CLOSED

do you think we should keep this exception and forward the onClick and onMouseDown to the content slot (it's a breaking change but would be marked as a bug fix to keep consistency with TreeItem).
Or do you think we should keep the new behavior and document it correctly (since it's one of the few behavior change).

If there is a tangible benefit to having this behavior change, then it might make sense to keep it.
But at first sight, it seems more of an oversight that could be brought up multiple times by people stumbling upon it. 🙈
I would lean towards fixing it. 🤔

from mui-x.

On both AntDesign and React Arborists the items are not nested (childs are siblings of their parent item, like we have on the grid) so their is no root / content difference...

Kendo UI has an onItemClick (same for drag end, drag over and drag start) props at the TreeView level: https://www.telerik.com/kendo-react-ui/components/treeview/api/TreeViewProps/#toc-onitemclick

from mui-x.

Hi,
Is this only occurring when you click on nested items or also on root items (items with no parent)?

If it's only for nested items, I have one explanation:

The item root contains its children as you can see in the screenshot below:

In TreeItem the onClick and onMouseDown prop are passed to the item content, which do not contain its children:

This explains the difference in behavior.

To be honest, I have totally overlooked this change in behavior and it is not intentional.
With that being said, I think it's very weird to have onClick and onMouseDown passed to an element deep inside the item but not the other callbacks like onMouseUp or onKeyDown.

The general rule in all our components is to forward the event handlers to the root unless we can't for some reason (onChange is forwarded to the input in a TextField, not to the root which is a div, for obvious reasons).

@LukasTy @noraleonte, do you think we should keep this exception and forward the onClick and onMouseDown to the content slot (it's a breaking change but would be marked as a bug fix to keep consistency with TreeItem).
Or do you think we should keep the new behavior and document it correctly (since it's one of the few behavior change).

Until then, you can fix your behavior by passing the onClick to the content:

<RichTreeView
    items={ITEMS}
    slots={{ item: TreeItem2 }}
    slotProps={{
        item: {
            slotProps: {
                content: {
                    onClick: (e) => {
                        console.log("click", e.target);
                    },
                }
            }
        },
    }}
/>

It's super verbose, and once we drop TreeItem I'm in favor to introduce new slots to shorten it:

<RichTreeView
    items={ITEMS}
    slots={{ item: TreeItem2 }}
    slotProps={{
        // Do not exist for now
        itemContent: {
            onClick: (e) => {
                console.log("click", e.target);
            },
        },
    }}
/>

from mui-x.

I agree with Lukas. Since this does not seem to bring any specific value, and it also introduces an inconsistency, we should probably fix it 🤔

from mui-x.

But isn't the fact that onClick / onMouseDown are passed to content and the other event handlers are passed to root an inconsistency on TreeItem?

The current behavior of TreeItem2 is more consistent IMHO

This code behave super weirdly right now,

<TreeItem
  onMouseDown={handleMouseDown}
  onMouseUp={handleMouseUp}
/>

from mui-x.

This relates to #12850

Maybe keeping the behavior of TreeItem2 and add a onItemClick at the Tree View level that targets the content is a good approach.

People have a super easy way to pass an onClick to the content (which is a common use-case and should be easy to do)
AND the behavior are consistent across event handlers.

For onMouseDown I think the current behavior on TreeItem2 is fine, it's coherent with onMouseUp and it's not a super common use-case so the slotProps approach is good enough.

from mui-x.

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

from mui-x.

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@rgavrilov: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

from mui-x.