baumths / flutter_tree_view

A Flutter collection of widgets and slivers that helps bringing your hierarchical data to life.
https://baumths.github.io/flutter_tree_view
MIT License
181 stars 69 forks source link

Scrollbar does not work #19

Closed chi-w-ng closed 2 years ago

chi-w-ng commented 2 years ago

First, thank you for the tree view widget. I threw trees with millions of nodes at the widget and it still handles them well. But scrollbar does not work.

Step to show the problem:

  1. load the demo example: https://mbaumgartenbr.github.io/flutter_tree_view/#/
  2. minimize windows so that if you scroll with mouse, scrollbar shows up
  3. if you drag the scrollbar, the pane on the right get scrolled instead of the tree pane (which is on the left).

This happens on web, macos and windows; haven't tried on other platforms.

baumths commented 2 years ago

Hi @chi-w-ng. Thank you for using the package!

I tested it and indeed the scroll bar is scrolling the wrong widget. It seems to be an issue only in the example app right?

I'll be updating the demo ASAP.

Thank you for filling in this issue!

baumths commented 2 years ago

I think the issue was solved by 9f5fa0c68afcdf9a45064d9f473551e914f3de58

Remember to clear the cache of your browser before trying the new version of the demo app. Feel free to reopen the issue if needed!

chi-w-ng commented 2 years ago

Yep, it worked. Thanks.

One other thing, how do you implement single selection? How to mark an item for update when updating state using bloc? I call TreeViewControl.refreshNode(node) and node does not rebuild.

Thanks,

--Chi


From: Matheus Baumgarten @.> Sent: Wednesday, January 12, 2022 5:24 AM To: mbaumgartenbr/flutter_tree_view @.> Cc: Chi Ng @.>; Mention @.> Subject: Re: [mbaumgartenbr/flutter_tree_view] Scrollbar does not work (Issue #19)

Closed #19https://github.com/mbaumgartenbr/flutter_tree_view/issues/19.

— Reply to this email directly, view it on GitHubhttps://github.com/mbaumgartenbr/flutter_tree_view/issues/19#event-5880279792, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AR6XY4CSSKICG5HRPJ7JUFTUVV6JLANCNFSM5LXBMPEA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.***>

baumths commented 2 years ago

Hey @chi-w-ng!

I would need a minimal reproducible example to find out if it's a problem in the refreshNode algorithm. 😅

chi-w-ng commented 2 years ago

Hi, I did make a change and make it work for my case:

@override void refreshNode(TreeNode node, {bool keepExpandedNodes = false}) { if (node.hasChildren) { node.descendants .where((descendant) => isVisible(descendant.id)) .forEach((child) => _nodesThatShouldRefresh[child.id] = true); } _nodesThatShouldRefresh[node.id] = true; super.refreshNode(node, keepExpandedNodes: keepExpandedNodes); notifyListeners(); }

I added the line: _nodesThatShouldRefresh[node.id] = true;

The result is:

in TreeView._nodeBuilder final shouldRefresh = controller.shouldRefresh(node.id);

shouldRefresh will be true. And the node rebuilds.

Did I just do something dangerous and break something?

baumths commented 2 years ago

Hey @chi-w-ng!

Nice catch!

If I'm not mistaking, the "refreshing" code was implemented to update the lines of the children of the node that was refreshed so changes to its descendents wouldn't break line hierarchy.

Refreshing the node itself is totally fine and the way you've done it is perfect.

I'm going to add this change to the package!

Would you like to open a PR? Otherwise I'll do it as soon as I have time.

chi-w-ng commented 2 years ago

I made some other "harmless" changes to my fork, would you please do the change?

Thanks,