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

expandAllChildren causes duplicate tree nodes #15

Closed
christianrowlands opened this issue May 16, 2023 · 8 comments · Fixed by #16
Closed

expandAllChildren causes duplicate tree nodes #15

christianrowlands opened this issue May 16, 2023 · 8 comments · Fixed by #16
Assignees
Labels
bug Something isn't working

Comments

@christianrowlands
Copy link

Good evening!

Thank you for creating this project. It has been awesome to use!

I have run into a bug using the new expandAllChildren feature released in version 2.0.0. When calling that method on the TreeViewController, it creates duplicates of all the items in the tree. I was able to reproduce the issue in your example Flutter app.

Here is a screenshot of the problem. Notice that there are two instances of Item 1-0E. All I did to get into this state is click the Expand all button exactly once. If you click it a few times then the duplicates are cleared out.

image

@jawwad-hassan89
Copy link
Contributor

hi @christianrowlands, thanks for the appreciation and your feedback.
I will look into this issue, and fix it.

@jawwad-hassan89 jawwad-hassan89 self-assigned this May 24, 2023
@jawwad-hassan89 jawwad-hassan89 added the bug Something isn't working label May 24, 2023
@berslen
Copy link

berslen commented May 26, 2023

hi @christianrowlands, thanks for the appreciation and your feedback. I will look into this issue, and fix it.

Same issue with expand as well, And please add example for best practice to get tree controller.

@christianrowlands
Copy link
Author

Yeah, that is another issue I am running into. I can get the controller, but it is not initialized until after the initState() and build() methods are called. As a result, I am not sure when to expand all the children. I am using your library to display comments on a post, and I want the comments to always be expanded. I have tried a variety of options but am unable to figure out how to expand all without adding some sort of delayed timer that is called so that the lazy initialized controller is ready.

@jawwad-hassan89
Copy link
Contributor

@christianrowlands I didn't such a use case in mind when I designed the controller, and with the current implementation I don't see any way of getting to know when the controller will get initialized, so the a delayed seems like the best option.
Just an FYI, thee controller is initialized in the initState method of the TreeView, so it takes some time to initialize.

What I can do, is to add a onTreeReady callback to the TreeView which will provide the controller. This way the controller will also be more straight-forward to access rather than using the GlobalKey to get the controller.

Will this work for you @christianrowlands ?

@jawwad-hassan89
Copy link
Contributor

@berslen I will look into the Expand method as well, will hopefully have a fix ready by tomorrow.

@christianrowlands
Copy link
Author

Yeah, an onTreeReady callback would be great.

Or maybe there is an even more straightforward approach for my specific use case. I am looking to mimic Reddit's behavior for comments on a post where all of the comments and child comments are expanded by default. Given the new expandAllChildren method that was added, I thought that was the best option. But if there were a way to set the default expansion state of a node, then I could override the TreeNode class and set the expansion state to true. Something like this?

/// Represents a Comment that is submitted to/received from the server backend.
class Comment extends TreeNode {
  final String commentId;
  final String? parentId; // Null indicates a top level comment
  final String postId;
  final String userId;
  final String comment;
  DateTime? createdAt;

  Comment({
    required this.commentId,
    this.parentId,
    required this.postId,
    required this.userId,
    required this.comment,
    this.createdAt,
  }) : super(key: commentId, initialExpansionState: true);
}

@jawwad-hassan89
Copy link
Contributor

jawwad-hassan89 commented May 26, 2023

@christianrowlands you can already use the TreeNode..expansionNotifier.value to set the initial expansion state, but right now it won't have any effect as the initial state is always collapsed for the TreeNode.
I will have to do some refactoring so that the initial expansion state is respected by the TreeNode.

onTreeReady callback will be easy to implement, this might take a little longer.

jawwad-hassan89 added a commit that referenced this issue May 27, 2023
jawwad-hassan89 added a commit that referenced this issue May 27, 2023
jawwad-hassan89 added a commit that referenced this issue May 27, 2023
@jawwad-hassan89
Copy link
Contributor

the fix for TreeViewController .expand and .expandAllChildren has been pushed to the fix/treeview-controller branch.
The onTreeReady callback has also been added.

You can use the updated code from this branch, it is working but not properly tested yet. The branch will be merged into main after testing and documentation changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants