Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TreeView] only to expand or collapse (onNodeToggle) when clicking on Icon but not select any particular node (onNodeSelect) #22024

Closed
1 task done
flora8984461 opened this issue Aug 1, 2020 · 17 comments 路 Fixed by #22846
Assignees
Labels
component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@flora8984461
Copy link

flora8984461 commented Aug 1, 2020

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 馃挕

This also has been mentioned in #19953, but currently seems no answer for it. Would like to click on expandIcon only expand or collapse, but not selecting the node since I hope to be able to select parent nodes and go into the link.

Examples 馃寛

I have created a code sandbox.
https://codesandbox.io/s/vigorous-davinci-m0mzp?file=/src/Example.js

Motivation 馃敠

Fro my project, I am building a tree view menu select, and the parent nodes can be also selected and click into the associated links, and after selecting a node, the menu will close (that's by design, I need to close the menu after selection). But it is not convenient that when the user just wants to expand the parents while not clicking into it, the current behavior will still lead the user to select the parent nodes and click into them and the menu closes.

I have tried to add another icon in the label, but when I add onClick function, it cannot pass nodeId into it.
I have also tried to not use onNodeSelect and just use onLabelClick, but it does not pass nodeId either.

It would be great to have this feature or could someone give me any suggestions?

Thank you the community.

@flora8984461 flora8984461 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 1, 2020
@oliviertassinari oliviertassinari added the component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! label Aug 2, 2020
@oliviertassinari

This comment has been minimized.

@flora8984461
Copy link
Author

I am building a tree view menu select

@flora8984461 For your use case, aren't you looking for disabling the "selection" feature and enabling the "checkbox" (#17407) one? For instance in https://demos.telerik.com/kendo-ui/treeview/index, what would you use?

Thank you so much for your suggestions, the checkbox looks good. But things are a little bit different for my case. Since it is a single selection, I don't wish to have checkbox. And mui/mui-x#6019 fixed not toggling when click on checkbox, while when clicking on the toggle icon it still selects the parent node. I have tried with e.stopPropagation() in onIconClick(), but it only stops toggle instead of stoping selection.

@joshwooding
Copy link
Member

joshwooding commented Aug 2, 2020

@flora8984461 I've done some investigation into making this and other combinations possible. I wanted to make some changes first (which I've just finished) so it's the next thing I'll work on so stay tuned :)

@flora8984461
Copy link
Author

I seem to make it work by adding a flag to set true or false...
I added the code in the same sandbox: https://codesandbox.io/s/vigorous-davinci-m0mzp?file=/src/Example.js

@flora8984461
Copy link
Author

flora8984461 commented Aug 2, 2020

@flora8984461 I've done some investigation into making this and other combinations possible. I wanted to make some changes first (which I've just finished) so it's the next thing I'll work on so stay tuned :)

Thank you so much. I also have another try to use a flag to make it work, and https://codesandbox.io/s/vigorous-davinci-m0mzp?file=/src/Example.js is the example. But it would be great to have something more flexible like an API.

@joshwooding joshwooding added new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 5, 2020
@joshwooding joshwooding self-assigned this Aug 5, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 8, 2020

@flora8984461 Actually, your feature request, makes me think about how macOS, kendo, and Gdrive tree view works. When you click the arrow, the node is focused, but not selected. This sounds like a great default.

@joshwooding What do you think about we change the behavior?

macos
kendo
gdrive

@eps1lon
Copy link
Member

eps1lon commented Aug 8, 2020

This sounds like a great default.

Why?

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 8, 2020

Why?

@eps1lon I assume that the end-users expectation when clicking on the icon is to change the "expanded" state, not the "selected" state and "expanded" state. The motivation would be: changing 1 state at the time, not 2. This should offer more control, less surprising behavior.

Implementations following this assumption:

Implementations going against:

Implementations not saying:

I didn't cherry-pick the list of tree view to look at for this very problem, I have taken all the ones we have in https://trello.com/c/YsHOLyar/2396-treeview

@joshwooding
Copy link
Member

@joshwooding What do you think about we change the behavior?

I think it might have a detrimental effect on accessibility. The icons are 18x18 on the tree view, relying on interactions with such a small target doesn't seem great to me (the tree items should probably increase in height too but we'll see). I think providing a way for users to have this behaviour is a good middle ground (see: #22096).

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 8, 2020

@joshwooding On the accessibility side, I would argue that:

  • Changing 2 states instead of 1 state with one action is more disorienting for users, it offers them less fine-tune control, it creates more surprises.
  • Why do we have two different, and isolated keys but not two different and isolated click areas? Enter for the "expended" state and Space for the "selected" state. So why not click on the arrow for the "expended" state and click on the label for the "selected" state?

@joshwooding
Copy link
Member

Implementations following this assumption:

Most of these trees only highlight the label on hover/select 3 or 4 of them don't. I think if the label only got highlighted the separate behaviour would make more sense. This isn't me saying we should copy this by the way.

https://www.w3.org/TR/wai-aria-practices-1.1/#TreeView

Arguably the demos in the aria practices select on expansion icon click too.

Changing 2 states instead of 1 state with one action is more disorienting for users, it offers them less fine-tune control, it creates more surprises.

A fair point

Why do we have two different, and isolated keys but not two different and isolated click areas? Enter for the "expended" state and Space for the "selected" state. So why not click on the arrow for the "expended" state and click on the label for the "selected" state?

This is in the aria guidelines, although technically it's a bit fuzzy around what the Enter key should do:

Enter: activates a node, i.e., performs its default action. For parent nodes, one possible default action is to open or close the node. In single-select trees where selection does not follow focus (see note below), the default action is typically to select the focused node.

@joshwooding
Copy link
Member

Another thing I thought of on the weekend. There are no icons on the TreeView by default, we've added the icons for the examples so changing the behaviour to only expand on click of the icon wouldn't work in a situation where there aren't any expansion icons. Unless we had two types of behaviour where if the icon is present then the icons are used as expansion triggers and the whole tree item is when they aren't present, but I'm not sure about that.

@eps1lon
Copy link
Member

eps1lon commented Aug 11, 2020

VSCodes file explorer changes both states. Considering the amount of assumptions made that aren't verified we should not change the behavior but rather offer a customization to add this. Didn't we have an issue to register a click from the icon only? In that handler we could preventDefault to prevent toggling the expanded state.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 12, 2020

VSCodes file explorer changes both states.

@eps1lon In the case of VSCode, I believe the "selected" state only makes sense for leaves (files), VSCode doesn't display the content of folders. As far as I understand it, for them changing one state or two when clicking on the arrow doesn't have an impact on the UX, so I don't think that we can draw conclusions from this source.


Another context we might want to have a look at: Sketch or Figma design tree:

Sketch

figma

@eps1lon
Copy link
Member

eps1lon commented Aug 12, 2020

VSCode selects every type of node which makes sense for most file explorers

@oliviertassinari
Copy link
Member

Oh I see, so developers can, for instance, drag and drop them.

@nathanlambert
Copy link

+1 to this as the default. I did this with MUI on a recent project, but the outcome looked a bit different than flora's codebox in that toggling didn't add selection styles to the items:

mui treeview

I was also worried about usability at first due to the tiny toggles, but that can be solved in part by just padding all the things 馃槃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
5 participants