feat: tree view (pending convention update) #3675
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
packages/skeleton-svelte/src/components/treeview/anatomy/treeview-branch-content.svelte
Outdated
Show resolved
Hide resolved
packages/skeleton-svelte/src/components/treeview/anatomy/treeview-branch-indent-guide.svelte
Outdated
Show resolved
Hide resolved
packages/skeleton-svelte/src/components/treeview/anatomy/treeview-branch.svelte
Outdated
Show resolved
Hide resolved
packages/skeleton-svelte/src/components/treeview/modules/treeview-anatomy.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Hugo Korte <[email protected]>
|
@dev-viinz @Hugos68 this is on my agenda for review. Just won't have time today. Will aim for tomorrow! |
There was a problem hiding this comment.
Overall this is in a great place Viinz. You've adapted to the v4 conventions very well.
In addition to my specific comments below. We need a couple things prior to moving on to React please:
- We need icons implemented. See Zag's doc example for placement. Follow our example from the recently updated RatingsGroup component (PR may still be pending) where internalized Lucide SVGs to serve as the defaults. Then provide entry (snippets, etc) for replacing these with user-defined alternatives. This should include a directory, file, and arrow. And the arrow should be able to toggle state (read: rotate) like Accordions.
- The design is functional, but let once all feedback in this pass is applied I'll do another pass pass. Then you can carry that directly to the React iteration
- Once all changes are in place, go ahead and setup the Documentation pages and start generating examples. It's fine to limit these to Svelte only for now. Just create a placeholder mdx page for React.
@Hugos68 FYI for all this
| * | ||
| * @defaultValue 3 | ||
| */ | ||
| level?: 1 | 2 | 3 | 4 | 5 | 6; |
There was a problem hiding this comment.
I believe we had a similar discussion around the implementation of the label for the past iteration. For examples, make sure to set a Skeleton .h{1-6} class on the element itself. And remember Tailwind classes cannot be generated, so if they add 1 for the level, apply h1 verbatim.
There was a problem hiding this comment.
We actually don't do this for the Accordion.Heading, should that be implemented there aswell?
There was a problem hiding this comment.
done, though you should probably have a look at my implementation, not sure if it's how you want it structured :)
There was a problem hiding this comment.
Hey Viinz, your implementation is fine, the only problem we need to solve is that now the heading isn't included as part of the classlist on the docsite. So users can't see this is applied. I'll try to think on how to solve this, but I like what you did here.
| }); | ||
| </script> | ||
|
|
||
| {#snippet treeNode(nodeProps: TreeViewNodeProps)} |
There was a problem hiding this comment.
I know it's due to the recursive nature of the component, but having to define a snippet for every instance of the Tree View nodes is very confusing at first glance. This is likely going to trip up casual users.
Let's make sure we explain this when we implement the documentation. Something like this:
Since Tree View nodes are used recursively, we implement a {descriptor} to enable this.
- Svelte descriptor: Snippet
- React descriptor: a component
|
@dev-viinz Just wanted to let you know we've introduced an So: You may replace all the relative calls with that. Find a replace makes this a breeze as all relative paths have the same depth thanks to our consistent folder structure. |
Linked Issue
Closes #3468
Description
Adds the TreeView component in the new v4 flavour.
Checklist
Please read and apply all contribution requirements.
docs/,feature/,chore/,bugfix/mainbranchpnpm checkin the root of the monorepopnpm formatin the root of the monorepopnpm lintin the root of the monorepopnpm testin the root of the monorepo/packageprojects, please supply a ChangesetChangesets
View our documentation to learn more about Changesets. To create a Changeset:
pnpm changesetand follow the prompts