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

Feature request: Tree model #233

Open
mariusor opened this issue Sep 9, 2022 · 9 comments · May be fixed by #639
Open

Feature request: Tree model #233

mariusor opened this issue Sep 9, 2022 · 9 comments · May be fixed by #639
Assignees
Labels
enhancement New feature or request

Comments

@mariusor
Copy link

mariusor commented Sep 9, 2022

Hello, I want to ask if the charmbracelet community would be interested in accepting a PR for adding a tree model to the project.

A while ago I needed one for a project of mine and at the time there was none available, so I created one.

Currently I think it reached a level of maturity that other people might be interested in using it.

If anyone can have a look at the code, and the example, let me know if it would be accepted as a PR.

@mariusor mariusor changed the title Feature request: tree Feature request: Tree model Sep 9, 2022
@mariusor
Copy link
Author

@maaslalani @meowgorithm (apologies for pinging you directly, but I saw you are members of Charm that are active in the issues section).

Is there an interest in my proposal, or should I just keep it as an out of tree model?

@muesli muesli added the enhancement New feature or request label Oct 5, 2022
@muesli
Copy link
Contributor

muesli commented Oct 5, 2022

@mariusor Apologies for not having responded sooner, but we'll definitely check out your code! A tree-view bubble is probably inevitable and we definitely see a need for such a component. It's certainly one of the most complex visual components and getting it right is non-trivial, so I'm excited to see what you came up with!

@mariusor
Copy link
Author

mariusor commented Oct 6, 2022

Thank you @muesli. Looking forward to comments.

@mariusor
Copy link
Author

Hello everyone, any hope to review my tree model?

I have made some improvements that extend the use to something like a threaded "conversation" model, which has another example

screenshot-20231120125447_560268653-swaygrab

@simbafs
Copy link

simbafs commented Nov 20, 2023

Look awesome!

@bashbunni
Copy link
Member

Hey, thanks for this! I saw your messages on Mastodon as well. We're definitely interested in your implementation, we just need to give it a very thorough review and try to break it. It may take a while to merge, but please don't take that as a lack of interest. Thank you so much for your patience!

@mariusor
Copy link
Author

Hi, I think that with the addition of the lipgloss/tree model my PR is probably unnecessary to merge.

I have not played with that one to see if there are any performance or any significant API differences but I trust you guys.

@dlvhdr dlvhdr linked a pull request Oct 15, 2024 that will close this issue
@meowgorithm
Copy link
Member

Hey again @mariusor: just a heads up that @dlvdhr is tackling this one in #639, which we intend to merge.

@mariusor
Copy link
Author

mariusor commented Dec 4, 2024

No worries. I have resigned myself that my model won't make it into the official repository.

I like my implementation better with regards to having the Node type be an interface that any other model can implement and be part of a tree, but I'm not married to the idea, it's probably much simpler having it as a stand alone data struct.

Having it as an interface allows that the logic of something like expand/collapse, multi-line, different sizes/heights for the nodes, etc. can be part of the individual node models instead of requiring the tree to have code for each custom behaviour. I haven't hit any performance issues yet due to this (propagating state to the nodes needs to be done with Updates for all nodes), but I'm not rendering very deep/tall trees.

I'm not sure I'm going to change my own applications that make use of trees to this new implementation, but that's mostly due to laziness. :)

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

Successfully merging a pull request may close this issue.

5 participants