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

How to implement a TreeView with node changes? #230

Merged
merged 19 commits into from
Apr 15, 2024

Conversation

h0lg
Copy link
Contributor

@h0lg h0lg commented Mar 22, 2024

Hey Fabulous Avalonia team!

On this branch I'm trying to extend the existing TreeViewPage example with some interactivity and as an example chose to count clicks per node and display the count while also preserving the node expansion of the tree. I'm failing with one or the other in different implementations.

Could you please have a look at it and share your thoughts or suggest changes? You'll find my different attempts in my commit history, what works and doesn't with each in the respective message.

Thanks a bunch!

@h0lg h0lg marked this pull request as draft March 22, 2024 23:09
@h0lg h0lg force-pushed the treeview-with-node-changes branch 2 times, most recently from 43e97a4 to 5d8f500 Compare March 25, 2024 21:32
@h0lg h0lg force-pushed the treeview-with-node-changes branch from 5d8f500 to d144d86 Compare April 1, 2024 19:56
member this.Name = name
member this.Children = children

module NodeView =
Copy link
Member

@edgarfgp edgarfgp Apr 2, 2024

Choose a reason for hiding this comment

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

@h0lg I have not used INotifyPropertyChanged with Fabulous in the past(might be possible, but not sure if fits with the Fabulous state runners.). I normally create a simple MVU component. See my last commit and improve the way you need :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edgarfgp You may be right. I have seen things like the often used AvaloniaPropertyChangedEventArgs in the Avalonia source that suggest there may be a custom property change implementation in play, but haven't looked into it.

Thank you - I was able to understand and turn your approach into a working solution within minutes!
I'll try it out in a slightly more complex scenario with a filtered tree.

If I understand correctly, the trick is to keep the messages encapsulated within the NodeView Component so that the host program doesn't have to re-render the TreeView.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, having separate component is needed here to track and update the node state and also to prevent the re-rendering of the widget tree.

@h0lg h0lg force-pushed the treeview-with-node-changes branch from cf4d072 to 2b66c11 Compare April 3, 2024 00:20
@edgarfgp
Copy link
Member

@h0lg This is already a good improvement for the samples. I am happy to accept it as it is. You can follow up in a separate PR if that works for you ?

@h0lg
Copy link
Contributor Author

h0lg commented Apr 14, 2024

@h0lg This is already a good improvement for the samples. I am happy to accept it as it is. You can follow up in a separate PR if that works for you ?

@edgarfgp My intent was to convert this into a separate example and keep the existing one untouched - to have at least one TreeView example remaining that's really basic. I find that helpful as an introduction. There's also some unused code that begs the question. Maybe it just needs some comments like //TODO do something to the selected model node here

@edgarfgp
Copy link
Member

My intent was to convert this into a separate example and keep the existing one untouched

Thanks, that would be useful.

There's also some unused code that begs the question. Maybe it just needs some comments like //TODO do something to the selected model node here

Yeah. Let's add a // TODO there so we will at least show how to use the selected value

@h0lg h0lg force-pushed the treeview-with-node-changes branch from 2b66c11 to 7c1273a Compare April 14, 2024 21:20
@h0lg h0lg force-pushed the treeview-with-node-changes branch from 909b00c to 7826232 Compare April 15, 2024 17:58
@h0lg
Copy link
Contributor Author

h0lg commented Apr 15, 2024

Yeah. Let's add a // TODO there so we will at least show how to use the selected value

@edgarfgp I've instead found a use for the selected model node by adding a TextBlock showing its name.

My intent was to convert this into a separate example and keep the existing one untouched

done - ready for a review and merge. Thanks a bunch!

@h0lg h0lg marked this pull request as ready for review April 15, 2024 18:04
Copy link
Member

@edgarfgp edgarfgp left a comment

Choose a reason for hiding this comment

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

Thanks for all this improvements. Love your contributions 👍

@edgarfgp edgarfgp merged commit de7ed61 into fabulous-dev:main Apr 15, 2024
1 check passed
@h0lg h0lg deleted the treeview-with-node-changes branch April 15, 2024 18:22
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.

None yet

2 participants