feat: add new tree-view component

[alenaksu]

This PR adds a Tree and TreeItem components that allow user to display a hierarchical list of items, expanding and collapsing the nodes that have nested items.

The sl-tree component implements the API to interact with the whole tree, like the selection and the keyboard navigation. It has just a default slot that can contain sl-tree-item components, that represent the nodes of the tree.

Each sl-tree-item can in turn contain other sl-tree-item, allowing to the user to define nested trees. A tree item with nested nodes, can be expanded and collapsed.

Closes #539

Code example

<sl-tree>
    <sl-tree-item>Node</sl-tree-item>
    <sl-tree-item>
      Parent Node
      <sl-tree-item>Child Node</sl-tree-item>
    </sl-tree-item>
</sl-tree>

Appereance

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
shoelace	✅ Ready (Inspect)	Visit Preview	Sep 1, 2022 at 1:59PM (UTC)

[claviska]

Thanks for submitting this. I know it's a draft, but here are some initial thoughts:

• Great job with following existing patterns and using APIs such as the animations registry! (I know these aren't well documented, but it's clear you're familiar with the codebase.)
• I like that you can nest tree items inside other tree items without having to create a new tree :)
• I like the idea of allowing checkable items for easy selection.
  • Can we move the checkbox on the inside of the tree by default? (The order could be customized by end users with order if we use flexbox here.)
    
  • Should checkable be called selectable instead?
  • Should it be on the tree or the tree item? Are all tree items always selectable, even subtrees?
  • We need to ensure keyboard accessibility works as expected. Here are some details on what users will expect.
• Tree item should probably have a loading state to facilitate lazy loading of subtrees. An example of this in the docs would also be helpful since users ask for this a lot (years ago I managed a jQuery-based tree plugin and this was a top request). It's probably preferable to use events here.
• Events/slots/dependencies/etc. need to be documented (I like to do this as I go so I don't miss anything)
• Can we remove the skeletons from the first example? It makes the example feel broken like it never loads.
• I want to make sure we get accessibility right here, so let's make sure to follow the guidelines here for treeitems, e.g. "No more than one node can be selected at a time unless the tree node has aria-multiselectable="true" set." The Shoelace version should also announce the same way in screen readers.
• The icon slot should probably be prefix for consistency with other components.
  • If we have a prefix slot, a suffix slot will also be desirable.
  • Now that I typed that out, do we even needs slots for this or should it be up to the user to put all this into the default slot? 🤔
• We should think about what parts (and exported parts) need to be exposed

I'm sure I'll have more feedback as this progresses, but you're off to a great start here!

[alenaksu]

Thank you for taking time to review!

I updated the PR following your points, here are the changes I made so far, some of them might need more discussion tho:

• Added some parts, still TBD
• Moved the checkbox inside the tree, it can be moved using the order css property
• Removed checkable property in favor of selectable. For now is an internal property that is used by the tree component. Setting it to true, will just show the checkbox. To note that an item can be selected even if selectable is false, so this property serves just to give a different appearance (see multi-select).
• Added a multi-select to the Tree component with which is possible to select multiple items. Setting it to true, will automatically set true to selectable to all the items, in order to display the checkbox.
• Added aria attributes and keyboard navigation. (not tested yet with screen reader)
• Removed skeletons from the example.
• Removed the icon slot. As you noticed, maybe is up to the user to display the icon in the proper way.
• Started to document events/slots/dependencies.

Should it be on the tree or the tree item? Are all tree items always selectable, even subtrees?

Regarding this topic, I think the behaviour follows what is described by the ARIA spec, especially if we think about the multiple selection. I'm not sure if there is a use case where just a subset of child items need to be selectable, so I kept the implementation into the tree component for now.

[claviska]

A couple comments on styles:

Can we remove the border/padding/background on the base? It appears to be in a well which is nice for some use cases, but I think the default appearance should be less opinionated, i.e. the user should be able to style the host element to add a border/background/padding should they want it.

I'm also not sure about the light gray highlight + primary text color on selected tree items.

Menu uses a primary background and white text, for example. I don't think anything in the library uses a light gray background for selections, so we might need to play with this a bit. Not sure if making it look like menu items would be too bold — maybe it's just the primary text throwing it off.

I think the behaviour follows what is described by the ARIA spec, especially if we think about the multiple selection. I'm not sure if there is a use case where just a subset of child items need to be selectable, so I kept the implementation into the tree component for now.

That's fair.

Regarding lazy loading of nodes, if tree items had a loading attribute that might be sufficient to facilitate async loading as long as screen readers announce it (maybe through aria-busy or aria-live).

I see some new commits since your last message, so let me know when you're ready for the next review/steps.

[alenaksu]

I'm also not sure about the light gray highlight + primary text color on selected tree items.

The reason why I didn't follow the same pattern used for the menu is because I thought it would have been a bit too "flashy" when using multiple selection. What do you think?

[alenaksu]

I've updated the PR:

• Removed all the styles from the base element in the tree component
• Removed the gray background when highlighting a tree item, now the hover/selected item has only the primary color set (let me know what you think about my previous comment)
• Implemented a possible solution for the lazy loading: the tree-item component has a new lazy-loading property that indicates that the content is not loaded yet, so it displays the expand button even if there are no children. When the user tries to expand the node, the loading state is set to true and the sl-load-data event is emitted. When the children slot changes (new items added), we can assume that the content has been loaded so the loaded state is set to false.
• Added some more documentation

I think the lazy loading is quite simple to use in this way, but I'm curious to know what you think.

[claviska]

Accessibility

It seems like screen reader compat is very, very close! Here are some examples I referenced using VoiceOver:

• https://dequeuniversity.com/library/aria/tree-view
• http://accessibleculture.org/articles/2013/02/not-so-simple-aria-tree-views-and-screen-readers/

One thing I noticed is that it keeps announcing "button, 1 of 1" and I think it's because of this role="button" highlighted below:

It's also not currently possible to tab into the tree. It looks like a roving tab index is called for, so tabbing once lands focus in the tree and another leaves it, and subsequent tabs back into the tree should focus on the last selected tree item. From that point, arrow keys are used to navigate and expand/collapse. (FYI — neither example allows Enter to expand/collapse a node, and it seems like Left|Right are the correct keys to use here so we should probably keep those and remove Enter.)

Styles

Removed the gray background when highlighting a tree item, now the hover/selected item has only the primary color set (let me know what you think about my previous comment)

Do we even need a selected style? Mouse users don't need to see it, and keyboard users will see :focus-visible styles so we can use that for them and do a normal focus ring outline (see other components for reference). The only time a selection needs to be visible (aside from using the keyboard) is when there's a checkbox, in which case the checkbox serves that purpose.

Note that users should be able to add a selected state through custom styles if they want it. The part could look like part="item item--selected" so they'd target the latter.

Lazy Loading

Nice approach! Can we rename lazy-loading to lazy and rename sl-load-data to sl-lazy-load so they're more tightly coupled? And maybe update the docs for the event to say something like:

Emitted when a lazy item is selected. Use this event to asynchronously load data and append items to the tree before expanding.

Question: the docs only show the lazy demo loading once. I assumed it would load every time the node expands unless the user removes the lazy attribute themself. Can we make this happen? It's likely we'll have two use cases for async nodes:

• load on every expand
• load once on first expand

And it feels like too strong an opinion to have the component mutate the lazy attribute for them.

[alenaksu]

I've got just a couple of questions:

(FYI — neither example allows Enter to expand/collapse a node, and it seems like Left|Right are the correct keys to use here so we should probably keep those and remove Enter.)

The ARIA Authoring Practices Guide states that particular behaviour for the Enter key. Do you think it's fine to remove it?

Do we even need a selected style?

To be honest I think it would be useful, image if you are implementing a sort of file browser, and you want to keep the selected item highlighted even if you move the focus away.
This for example, is the file tree present in the Changed Files tab on the Github PRs.

Does it make sense?

[claviska]

The ARIA Authoring Practices Guide states that particular behaviour for the Enter key. Do you think it's fine to remove it?

I stand corrected. I was basing that off of the two examples, neither of which expand on Enter. Carry on! 😂

To be honest I think it would be useful, image if you are implementing a sort of file browser, and you want to keep the selected item highlighted even if you move the focus away.

I agree, that’s a great use case. But should the visible selection be on by default for all users? It feels a lot better to me if the tree starts without a selection, and for users who want to show one, they can easily target ::part(item--selected) to opt in and customize it to their taste.

[break-stuff]

I didn't change the behavior to stay consistent with what is described in the ARIA APG

I believe they should have the same behavior. If the row is selectable, both "space" and "enter" should select the row. If it is expandable, they both should toggle the row. If it is both expandable and selectable, they both should select the row and the arrow keys should be used to expand and collapse. Check out their example here.

I haven't added these attributes since are not mandatory but only suggested when the browser is not able to compute these informations, for example in the case the treeitems are not yet present in the DOM.

It looks like the examples in the docs read fine for Narrator and NVDA on Windows, but VoiceOver does not correctly read the position. I tried adding the aria attributes and it didn't seem to help. 🤷‍♂️

[claviska]

Spent some time on this today trying to get it over the finish line. I have a branch with my updates here. It's probably easiest to merge my changes from that branch into this PR.

Branch: https://github.com/shoelace-style/shoelace/tree/alenaksu-feat/tree-view
Preview: https://shoelace-bzzb25yo3-shoelace.vercel.app/components/tree

The good news is that it's just about there! Here's what I updated:

• Removed selection="none" since there's no obvious use case for a tree that isn't selectable. (Sorry, that was originally a suggestion from me but single|multiple|leaf seem sufficient!)
• Added a more obvious hover/selection style. You were right on this one too. 😅
• Added missing icon dependency to <sl-tree>
• Fixed RTL styles
• Fixed keyboard support when using RTL
• Various style tweaks
• Renamed the sl-selected-change event to --sl-selection-change
• Renamed the --indentation-size custom property to --indent-size.
• Added --indent-guide-color|offset|style|width custom properties so indent guides can be more easily set.
• Updated examples in the docs to be more generic
• Added remaining React examples
• Updated the changelog

Here's what's left on the wishlist:

• Disabled items should accept focus but shouldn't expand or accept selection. (This is the recommended behavior for disabled items, and I recently updated Tabs and Menus to do this.)
• An example with a plus/minus icon instead of chevrons. But rather than bake in specific icons, it would be nice if the user can configure them.
  • In Breadcrumb, there's a hidden slot called called separator that the user can insert their own icon into. The component then clones the node, removes ids, and injects it into each breadcrumb item on slotchange.
  • I don't want this to make things more difficult for you, so I'm more than happy to take this on myself in the next iteration if you're not interesting in tackling it. This is really more of a "nice to have."

And there's one known bug I found:

• If a child node is expanded, tabbing/keyboard stops working when the parent node collapses. I suspect the roving tab index needs to be updated when a child node is hidden, but I didn't have time to look into it today.
  1. To repro, go to the first example in this preview and tab into the tree.
  2. Press Right to expand Deciduous."
  3. Press Down until you land on "Maple."
  4. Press Right to expand "Maple."
  5. Leaving "Maple" open, press Up a couple times to go back to "Deciduous."
  6. Press Left to collapse "Deciduous."
  7. Now try pressing Up or Down. The selection remains unchanged and other items can no longer be selected.

[alenaksu]

Thank you for your changes, I've merged your branch into mine.
I also pushed some other stuff:

• The tree now supports customizable icons for the expand/collapse button
• Disabled items are now focusable but cannot be selected
• Fixed the bug regarding roving tabindex and hidden nodes