fluentui icon indicating copy to clipboard operation
fluentui copied to clipboard

[Bug]: TreeItem click events firing navigation

Open george-cz opened this issue 1 year ago • 6 comments

Library

React Components / v9 (@fluentui/react-components)

Bernardo has more context. Creating this now so that I can link it in the TMP repo.

george-cz avatar Jul 17 '24 15:07 george-cz

Here's the context:

TL,DR: Jest does not support (at least not out-of-the-box) actual real click events 🤡, to solve this just emit a focus event after clicking

It seems like TreeItem click event handler is triggering navigations:

https://github.com/microsoft/fluentui/blob/3c9750e01f3e8b55a1e3b36744c2e855c864c78e/packages/react-components/react-tree/library/src/components/TreeItem/useTreeItem.tsx#L130-L136

This is to ensure the correct tabIndex of the focused element.

In browsers once you click on something and that something is focusable the click will propagate a bunch of mouse events and a focus event. So the assumption is that a click event is equivalent to a focus one, which is kind of true for browser environments (since once you click you also focus on a treeitem), but this is not true for the actual .click() method, this method is not equivalent to an actual click, as it does not invoke the same events, missing also the focus event.

So the bug here is:

  1. we're erroneously using a click instead of a focus

And the assumption is:

actions visibility on click. As the docs describes https://react.fluentui.dev/?path=/docs/components-tree--default#actions actions slot is only visible when the treeitem is active (by hovering or by navigating to it = focus)

bsunderhus avatar Jul 18 '24 07:07 bsunderhus

Not sure this is related, but after #31766, there seems to be no longer a navigation event triggered when clicking on a leaf node. Navigating to a leaf node with the keyboard triggers onNavigation but not clicking on a leaf node. In #31766 the click handler seems to be completely bypassed for leaf nodes?

steabert avatar Jul 24 '24 14:07 steabert

Not sure this is related, but after #31766, there seems to be no longer a navigation event triggered when clicking on a leaf node. Navigating to a leaf node with the keyboard triggers onNavigation but not clicking on a leaf node. In #31766 the click handler seems to be completely bypassed for leaf nodes?

That is true @steabert. There's no longer invocation of onOpenChange on a leaf, as there's no change in the open state. This #31766 was a bugfix on this. Apparently this bugfix has some repercussions

bsunderhus avatar Jul 26 '24 12:07 bsunderhus

@bsunderhus should we close this?

george-cz avatar Aug 27 '24 10:08 george-cz

@bsunderhus should we close this?

No @george-cz , we still need to discuss moving navigation from click events to focus events, which is correlated to this issue https://github.com/microsoft/fluentui/pull/32042

At the moment this is not a priority though, as in real world scenarios click and focus are connected through a real click event.

The https://github.com/microsoft/fluentui/pull/32042 PR right now proposes the solution with a breaking change by adding a new navigation type on focus and deprecating the navigation on click

bsunderhus avatar Aug 28 '24 09:08 bsunderhus

I removed this from current iteration and moved it back to the backlog so that we can discuss this further 😁

bsunderhus avatar Aug 28 '24 09:08 bsunderhus

This issue has not had activity for over 180 days! We're adding Soft close label and will close it soon for house-keeping purposes. Still require assistance? Please add comment - "keep open".

Because this reported issue has not had any activity for over 180 days, we're automatically closing it for house-keeping reasons.

Still require assistance? Please, create a new issue with up-to date details and latest version of Fluent.

This issue has not had activity for over 180 days! We're adding Soft close label and will close it soon for house-keeping purposes. Still require assistance? Please add comment - "keep open".

Because this reported issue has not had any activity for over 180 days, we're automatically closing it for house-keeping reasons.

Still require assistance? Please, create a new issue with up-to date details and latest version of Fluent.