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

TreeSliver & associated classes #147171

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

Conversation

Piinks
Copy link
Contributor

@Piinks Piinks commented Apr 22, 2024

FYI for Reviewers: Much of the API surface matches that of the 2D TreeView in flutter/packages#6592. If it changes here, it should change there, and vice versa.

📜 Design Document

This adds classes and associated callbacks and controllers for TreeSliver. Core components:

  • TreeSliver
  • RenderTreeSliver
  • TreeSliverNode
  • TreeSliverController
  • TreeSliverStateMixin
  • TreeSliverIndentationType

Fixes #114299

sliver_0.mov
sliver_1.mov

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@Piinks Piinks added c: new feature Nothing broken; request for a new capability framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Apr 22, 2024
@github-actions github-actions bot added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Apr 22, 2024
@Piinks Piinks changed the title Sliver tree 2 SliverTree Apr 22, 2024
@github-actions github-actions bot added the f: material design flutter/packages/flutter/material repository. label Apr 22, 2024
@Piinks
Copy link
Contributor Author

Piinks commented Apr 22, 2024

For tomorrow:

@github-actions github-actions bot removed the f: material design flutter/packages/flutter/material repository. label Apr 23, 2024
@Piinks Piinks changed the title SliverTree SliverTreeList Apr 23, 2024
@Piinks Piinks marked this pull request as ready for review April 24, 2024 22:36
Copy link
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

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

LGTM for the examples!

Spotted minor typos in the non-examples files.

packages/flutter/lib/src/widgets/sliver_tree.dart Outdated Show resolved Hide resolved
examples/api/lib/widgets/sliver/sliver_tree.1.dart Outdated Show resolved Hide resolved
examples/api/test/widgets/sliver/sliver_tree.0_test.dart Outdated Show resolved Hide resolved
examples/api/test/widgets/sliver/sliver_tree.1_test.dart Outdated Show resolved Hide resolved
examples/api/test/widgets/sliver/sliver_tree.1_test.dart Outdated Show resolved Hide resolved
examples/api/test/widgets/sliver/sliver_tree.0_test.dart Outdated Show resolved Hide resolved
@Piinks Piinks marked this pull request as ready for review May 16, 2024 23:27
Copy link
Member

@TahaTesser TahaTesser left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

@baumths baumths left a comment

Choose a reason for hiding this comment

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

Hello team, please consider adding generics to the callbacks and builders of the TreeSliver and TreeView (from two_dimensional_scrollables) so users can avoid casting the TreeSliverNode.content in each callback. Example:

TreeSliver<MyItem>(
  tree: myTree,
  treeNodeBuilder: (BuildContext context, TreeSliverNode<Object?> node, _) {
    return MyTreeSliverNodeTile(
      // Casting as I need to access some state of MyItem deeper down in the widget tree
      item: node.content as MyItem,
  },
  // In the `TreeView` widget, I'm using this callback to add focus/hover/active decorations to the whole width of a node
  treeRowExtentBuilder: (TreeSliverNode<Object?> node, _) {
    final item = node.content as MyItem;
    return item.preferredHeight;
  },
  onNodeToggle: (TreeSliverNode<Object?> node) {
    final item = node.content as MyItem;
    myStorage.persistMyItemExpansionState(item.id, node.isExpanded);
  },
);

If I just change the type to TreeSliverNode<MyItem> I get the following error message:
The argument type 'MyTreeSliverNodeTile Function(BuildContext, TreeSliverNode<MyItem>, AnimationStyle)' can't be assigned to the parameter type 'Widget Function(BuildContext, TreeSliverNode<Object?>, AnimationStyle)'.

///
/// Used by [TreeSliver.treeNodeBuilder] to build rows on demand for the
/// tree.
typedef TreeSliverNodeBuilder = Widget Function(
Copy link

Choose a reason for hiding this comment

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

Suggested change
typedef TreeSliverNodeBuilder = Widget Function(
typedef TreeSliverNodeBuilder<T> = Widget Function(

/// tree.
typedef TreeSliverNodeBuilder = Widget Function(
BuildContext context,
TreeSliverNode<Object?> node,
Copy link

Choose a reason for hiding this comment

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

Suggested change
TreeSliverNode<Object?> node,
TreeSliverNode<T> node,

///
/// By default, if this is unset, the [TreeSliver.defaultTreeNodeBuilder]
/// is used.
final TreeSliverNodeBuilder treeNodeBuilder;
Copy link

Choose a reason for hiding this comment

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

Suggested change
final TreeSliverNodeBuilder treeNodeBuilder;
final TreeSliverNodeBuilder<T> treeNodeBuilder;

///
/// * [SliverVariedExtentList], which uses a similar item extent builder for
/// dynamic child sizing in the list.
typedef TreeSliverRowExtentBuilder = double Function(
Copy link

Choose a reason for hiding this comment

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

Suggested change
typedef TreeSliverRowExtentBuilder = double Function(
typedef TreeSliverRowExtentBuilder<T> = double Function(

/// * [SliverVariedExtentList], which uses a similar item extent builder for
/// dynamic child sizing in the list.
typedef TreeSliverRowExtentBuilder = double Function(
TreeSliverNode<Object?> node,
Copy link

Choose a reason for hiding this comment

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

Suggested change
TreeSliverNode<Object?> node,
TreeSliverNode<T> node,

///
/// * [TreeSliver.onNodeToggle], for controlling node expansion
/// programmatically.
typedef TreeSliverNodeCallback = void Function(TreeSliverNode<Object?> node);
Copy link

Choose a reason for hiding this comment

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

Suggested change
typedef TreeSliverNodeCallback = void Function(TreeSliverNode<Object?> node);
typedef TreeSliverNodeCallback<T> = void Function(TreeSliverNode<T> node);

///
/// * [SliverVariedExtentList.itemExtentBuilder], a very similar method that
/// allows users to dynamically compute extents on demand.
final TreeSliverRowExtentBuilder treeRowExtentBuilder;
Copy link

Choose a reason for hiding this comment

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

Suggested change
final TreeSliverRowExtentBuilder treeRowExtentBuilder;
final TreeSliverRowExtentBuilder<T> treeRowExtentBuilder;

/// Called when a [TreeSliverNode] expands or collapses.
///
/// This will not be called if a [TreeSliverNode] does not have any children.
final TreeSliverNodeCallback? onNodeToggle;
Copy link

Choose a reason for hiding this comment

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

Suggested change
final TreeSliverNodeCallback? onNodeToggle;
final TreeSliverNodeCallback<T>? onNodeToggle;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: new feature Nothing broken; request for a new capability d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a lazy SliverTree
5 participants