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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: tree bubble #639

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Conversation

dlvhdr
Copy link

@dlvhdr dlvhdr commented Oct 15, 2024

Introduces a tree bubble for TUIs that require a tree like model with expand/collapse/select functionality.

Screen.Recording.2024-10-25.at.17.31.24.mov

Intended Interactions

  • All the viewport scrolling methods
  • expand / collapse
  • select
  • later on - filter which will show the tree of matching items

API

  • Use lipgloss.Tree for the rendering
  • Expose SelectedNode()
  • Expose SetSelectedNode()
  • Tree items
    • Have a YOffset that represents their vertical position in the tree
    • Will expose their YOffset in a getter so users can scroll to a node
    • Will be useful if put inside a viewport
  • Expose a node's depth in a getter
    • so users can style a node based on its depth
    • should be done in lipgloss?

Example Usage

See charmbracelet/bubbletea#1190

Closes #233.

@dlvhdr dlvhdr force-pushed the dlvhdr/tree-bubble branch from e32c664 to 0addf7c Compare October 17, 2024 14:39
tree/tree.go Outdated Show resolved Hide resolved
tree/tree.go Outdated
// Node implements lipgloss's tree.Node
type Node struct {
// tree is used as the renderer layer
tree *ltree.Tree
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking whether I should implement everything here, and not use lipgloss.Tree because:

  1. I'm using lipgloss.Tree as a renderer, but I found myself implementing a lot of its logic.
  2. I've also found that the table bubble doesn't actually use lipgloss.Table
  3. I needed more granular styling like the notion of a selected node, which lipgloss.Tree doesn't "natively" support, so I wasn't sure if what I'm doing is an abuse.
  4. I would also need to expose everything lipgloss.Tree exports like all the different Enumerator styles and the rest.

Let me know what you think. Should the view function also be implemented here and not use lipgloss.Tree.View()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we use lipgloss/tree for rendering. If it doesn't support what we need we can get it there.

So for some background, the table/bubble predates lipgloss.Table, but the plan was always to migrate to Lip Gloss for rendering as lipgloss/table, which is much, much more robust than the Bubbles implementation.

tree/tree.go Outdated
Comment on lines 153 to 172
// IsSelected returns whether this item is selected.
func (t *Node) IsSelected() bool {
return t.yOffset == t.opts.treeYOffset
}

// Depth returns the depth of the node in the tree.
func (t *Node) Depth() int {
return t.depth
}

// Size returns the number of nodes in the tree.
// Note that if a child isn't open, its size is 1
func (t *Node) Size() int {
return t.size
}

// YOffset returns the vertical offset of the Node
func (t *Node) YOffset() int {
return t.yOffset
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be used to allow for more flexible styling.
I found the StyleFunc of lipgloss.Tree doesn't support some stuff I want, like appending a symbol to the end of each item at the same indentation to the right of it. Maybe there are more use cases.

tree/tree.go Outdated
}

// Used to print the Node's tree
// TODO: Value is not called on the root node, so we need to repeat the open/closed character
Copy link
Author

@dlvhdr dlvhdr Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I found I needed to implement the String() func to repeat the open/close character logic because the Value() func isn't called on the root. Maybe this is an easy fix in lipgloss.

tree/tree.go Outdated
Comment on lines 204 to 225
func (t *Node) GivenValue() any {
return t.value
}
Copy link
Author

@dlvhdr dlvhdr Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was helpful in the TOC example I think. I found that the tree "swallows" my values and I can't read them again. This forces the user to save them before passing to the tree, which is just duplication and not the most ergonomic API. The name is confusing though given the Value() func also exists due to the Node interface from lipgloss.
A ValueFunc, similar to the StyleFunc could also solve this, as I believe that was my usecase.

tree/tree.go Outdated
Comment on lines 209 to 239
func (t *Node) Children() ltree.Children {
return t.tree.Children()
}
Copy link
Author

@dlvhdr dlvhdr Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exposes lipgloss as it has to return lipgloss.Children since that's the interface

tree/tree.go Outdated Show resolved Hide resolved
@dlvhdr
Copy link
Author

dlvhdr commented Nov 16, 2024

update: starting to take shape when used inside https://github.com/dlvhdr/diffnav

image

Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great! would be awesome to add an example as well

tree/node.go Outdated Show resolved Hide resolved
tree/node.go Outdated Show resolved Hide resolved
@dlvhdr dlvhdr force-pushed the dlvhdr/tree-bubble branch from 021d11d to dbf0223 Compare December 5, 2024 21:41
@dlvhdr dlvhdr marked this pull request as ready for review December 5, 2024 21:42
@dlvhdr dlvhdr requested a review from bashbunni as a code owner December 5, 2024 21:42
@wjam
Copy link

wjam commented Dec 13, 2024

I've just been playing with this PR locally and wondered whether it'd be possible to configure the open/closed character per node? The idea I had was a file tree where the closed character for a directory would be a closed folder and the open character would be an open folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Tree model
4 participants