feat/tree-view-select

[Mahmoud-zino]

Linked Issue

Closes #1773,
Closes #1784,
Closes #1785

Description

reference the summary bellow.

Tasks

• Change docs icons example
• Hide default arrow on WebKit-based browsers
• Support Multi/Single select
• Support relational selection
• Non-expanding row should not react to clicks (dash animation)
• Add Disable TreeViewItem
• Add ExpandAll / SelectAll export functions
• Support data-driven tree
• Expand docs to cover recursive tree view (data-driven-docs) -> Add a recursive Tree View example to the docs. #1785
• add full a11y support + keyboard

Changsets

feat: Added tree-view single/multi selection mode, Enabled data-driven for tree-view.

Checklist

Please read and apply all contribution requirements.

• This PR targets the dev branch (NEVER master)
• Documentation reflects all relevant changes
• Branch is prefixed with: docs/, feat/, chore/, bugfix/
• Ensure Svelte and Typescript linting is current - run pnpm check
• Ensure Prettier linting is current - run pnpm format
• All test cases are passing - run pnpm test
• Includes a changeset (if relevant; see above)

[changeset-bot]

🦋 Changeset detected

Latest commit: 02a58b2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@skeletonlabs/skeleton	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

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

Name	Status	Preview	Updated (UTC)
skeleton-docs	✅ Ready (Inspect)	Visit Preview	Aug 9, 2023 7:00pm

[endigo9740]

@Mahmoud-zino just saw this popup - no rush but can you address this in this PR as well?

Either change the icons or change the text to something like Subitem 1|2.

We definitely don't want Skeleton to be associated with dead children heh

[Mahmoud-zino]

@endigo9740 Can you please test the default arrow on Safari, I am testing on a WebKit-based browser from Cypress, but I can't run Safari on Linux or Windows -_-

[Mahmoud-zino]

@endigo9740 As mentioned in Discord:
updating the checkbox using Dom-manipulation is going great, but when I change a checkbox value using the DOM It won't apply the changes to the "selected" prop because we are bypassing Svelte's reactivity system...
I have checked other libraries, no one has implemented the tree in a declarative way, everyone uses a data-driven (with an array) tree view.
I was planning to support both ways but I am blocked on how to do this the declarative way.
If it is not possible, we will have to switch to the data-driven tree-view

[endigo9740]

@Mahmoud-zino unrelated to any other issue - I've noted that even on production you can tap a non-expanding row and the dash icon flips. Let's put that on the list to resolve as well please.

[endigo9740]

@Mahmoud-zino from what I'm seeing the tree view item selection state is not following our standard pattern for embedded form elements. The closest frame of reference is the ListBox/ListBox item. It works like this:

• Listbox - defaults to single selection unless multiple attribute is provided
• ListBox Item
  • Uses the Svelte Context API to read and and apply single/multiple state
  • We use a Svelte if statement to to toggle between two inputs, one for checkbox (single select) or radio (multi)
  • Each item requires three props:
    • bind:group: contains the reference to the variable that holds the current value
    • name: the shared group name (ex: medium)
    • value: the specific item value (ex: books, movies, tv)

One difference between listboxs and tree views - the input should be VISIBLE, not hidden. But ONLY when users want selection to be present. So we should set a selection prop on the root parent tree and pass that down the tree via the Context API to know whether or not the input and selection state should be maintained.

Here's how I'd imagine that would work in execution:

<!-- Single Selection - Flat List -->

<script lang="ts">
	const groupTopLevel = 'item-1';
</script>

<TreeView selection>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-1">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
	</TreeViewItem>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-2">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
	</TreeViewItem>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-3">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
	</TreeViewItem>
</TreeView>

<!-- Multi Selection - Flat List -->

<script lang="ts">
	const groupTopLevel = ['item-1'];
</script>

<TreeView selection multiple>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-1">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
	</TreeViewItem>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-2">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
	</TreeViewItem>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-3">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
	</TreeViewItem>
</TreeView>

<!-- Single Selection - Sub List -->

<script lang="ts">
	const groupTopLevel = 'item-1';
	const groupSubLevel = 'subitem-1';
</script>

<TreeView selection>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-1">
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
		<svelte:fragment slot="children">
			<TreeViewItem bind:group={groupSubLevel} name="sub-level" value="subitem-1">
				<svelte:fragment slot="lead">(icon)</svelte:fragment>
				(item 1)
			</TreeViewItem>
			<TreeViewItem bind:group={groupSubLevel} name="sub-level" value="subitem-2">
				<svelte:fragment slot="lead">(icon)</svelte:fragment>
				(item 1)
			</TreeViewItem>
			<TreeViewItem bind:group={groupSubLevel} name="sub-level" value="subitem-3">
				<svelte:fragment slot="lead">(icon)</svelte:fragment>
				(item 1)
			</TreeViewItem>
		</svelte:fragment>
	</TreeViewItem>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-2">...</TreeViewItem>
	<!-- ... -->
</TreeView>

<!-- Multi Selection - Sub List -->

<script lang="ts">
	const groupTopLevel = ['item-1'];
	const groupdItemOneSubLevel = ['subitem-1'];
</script>

<TreeView selection multiple>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-1" multiple>
		<svelte:fragment slot="lead">(icon)</svelte:fragment>
		(item 1)
		<svelte:fragment slot="children">
			<TreeViewItem bind:group={groupSubLevel} name="sub-level" value="subitem-1">
				<svelte:fragment slot="lead">(icon)</svelte:fragment>
				(item 1)
			</TreeViewItem>
			<TreeViewItem bind:group={groupSubLevel} name="sub-level" value="subitem-2">
				<svelte:fragment slot="lead">(icon)</svelte:fragment>
				(item 1)
			</TreeViewItem>
			<TreeViewItem bind:group={groupSubLevel} name="sub-level" value="subitem-3">
				<svelte:fragment slot="lead">(icon)</svelte:fragment>
				(item 1)
			</TreeViewItem>
		</svelte:fragment>
	</TreeViewItem>
	<TreeViewItem bind:group={groupTopLevel} name="top-level" value="item-2">...</TreeViewItem>
	<!-- ... -->
</TreeView>

[Mahmoud-zino]

@endigo9740

Summary

Change docs icons example

This is now changed to a folder-file structure example with appropriate icons.

Hide default arrow on WebKit-based browsers

done.

Support Multi/Single select

this is done like you suggested (using HTML inputs), it is very similar to the ListBox, just the group/name/value are not required so we support both selectable and non-selectable trees.

Support relational selection

Since in a tree, most of the users would want the check-boxes - check state to depend on its children, I've gone ahead and implemented it. this is only possible when the user includes all the children's references on an item.
I have considered automating this, but it was not possible because we have to collect the children from the DOM and somehow get svelte-component references from them.
also, I introduced the prop indeterminate to indicate that some of the children are checked but not all.
handling relational checking is somewhat complex, but I commented on all related code so this should not be hard to follow (I hope 😄).

Non-expanding row should not react to clicks (dash animation)

Done.

Add Disable TreeViewItem

Done.
Note: I had to use prevent default so the details won't expand or do any action when it is disabled since there is no disable option from the HTMLDetailsElement.

Add ExpandAll / SelectAll export functions.

Done
Note: We discussed the documentation of these, SVELD is displaying the body of the function in the table because it is exported. we might want to change this behavior.

Support data-driven tree

This was the real challenge between all the tasks...
I created an extra component TreeViewDataDrivenItem.svelte that will call itself recursively to include its children.
The user can now supply a list of TreeViewNode which contains all data needed for one node/item.
also to make this work with selection, the user should provide a value and checked in the node, the name is randomly auto-generated and the group is managed internally to include the checked nodes.

Expand docs to cover recursive tree view (data-driven-docs)

This is not needed anymore, since we are providing an easier solution for data-driven.

add full a11y support + keyboard

I added only keyboard support for keys that don't interfere with our inputs. the A11y suggests a lot more since they consider baking the selection in the element instead of using HTML inputs but this doesn't apply to us.

---

Please let me know what you think in general about the changes, this is somewhat a new approach in Skeleton so I expect you or other contributors to not agree with some of the implemented stuff, but I am happy to discuss and change them when needed.

[endigo9740]

@Mahmoud-zino just took a peek at this and holy cow, what an update! So many features!

Given the complexity of this feature I'm going to encourage reviews by other contributors. I'd love to have this by the v2 release, but I cannot promise I'll personally have a chance before the v2 release.

That said, the feature is indicated to be in a beta state, which means it can remain in flux and contains breaking issues. In that case, we might consider allowing end users to help us out here.

[PrivOci]

I'm using the 'Tree View' component as a sidebar menu, similar to what Flowbite is offering. One feature it lacks is having a "selected" state, similar to what 'TabAnchor' provides. Will this PR add the feature?

[Mahmoud-zino]

@PrivOci Yes this was the target of this PR, you can test what is being added from https://skeleton-docs-git-fork-mahmoud-zino-feat-t-50ee43-skeleton-labs.vercel.app/components/tree-views
Please let us know if you found any issues with it 👍

[PrivOci]

In selection, you mean this one?

But I mean selection something like this when we move to a page:

[Mahmoud-zino]

@PrivOci This is also possible using styling, we are using native input so we decided to show them, but you can also hide them and change the highlight of the active element through props.
However, if you only have one level you could use list boxes instead https://www.skeleton.dev/components/listboxes.

[endigo9740]

@Mahmoud-zino I've done a pass through the PR and submitted a few updates to the page documentation. I've adjusted, regrouped, and renamed a few sections to be more semantic and provide more instruction. I've also tried to boil down the examples to the minimum information required.

Per the logic for the feature itself - there's too much for me to be able to dive into that right now. However, from the user-end everything seems to be operating as expected. The properties and APIs you've implemented for implementing and enable features all work as expected.

The only constructive criticism I might give here is to evaluate if we need to provide so many features in a single PR. I know folks will ask for the world, but it's ok to limit scope if it means complexity stays low. The more complex something is, the more room there is for things to go wrong. Right now, you might be the only personal qualified to understand what's happening under the hood for this feature.

Rather than trying to prune out features, what I might recommend in the future is we revisit and do the following:

1. Implement more and much more detailed comments to break down the inner workings of this component. Code alone is not always enough documentation.
2. Evaluate semantic naming of variables and functions. Naming is the one of the hardest things to nail down, but might be easier in a pair programming environment.
3. Consider abstracting some of of the logic outside the component and evaluating if there's room for reuse. Svelte components are great in that they house everything, but also kind of terrible when you hit a certain level of complexity because everything is housed in one place heh

For now, everything appears to be operating as I would expect as an end user, so we'll merge this with the condition that the feature will still be designated as a beta. We'll gather feedback from end users. Then we'll make any last minute changes in a future release before this moves to stable.

[Mahmoud-zino]

@endigo9740 I totally get it. actually, when I was implementing it, I was constantly worried about how other contributors will have so much time and effort to review. in the future, I will make sure to break things down to make it easier for others 👍

as for your 3 suggestions, I will be happy to apply them whenever we have time for that 😄