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

SliverTreeList #147171

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

SliverTreeList #147171

wants to merge 30 commits into from

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 SliverTree. Core components:

  • SliverTree
  • RenderSliverTree
  • SliverTreeNode
  • SliverTreeController
  • TreeStateMixin
  • SliverTreeIndentationType
  • SliverTreeTraversalOrder

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
///
/// ** See code in examples/api/lib/widgets/sliver/sliver_tree.1.dart **
/// {@end-tool}
class SliverTreeList<T> extends StatefulWidget {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not love this name, but SliverTree and SliverAnimatedTree are taken. Open to other ideas. I think this works because the tree itself is flattened to a list of active nodes.
And it subclasses SliverVariedExtentList. So.. it's list-y.

/// the [SliverTreeList.defaultAnimationCurve], which is [Curves.linear].
///
/// To disable the tree animation, use [AnimationStyle.noAnimation].
final AnimationStyle? animationStyle;
Copy link
Member

Choose a reason for hiding this comment

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

animationStyle could be named toggleAnimationStyle, which makes it easier to understand what type of animation this property customizes. In this case it used for toggle animation based on the docs. (Noticed this when reviewing examples)

E.g. expansionAnimationStyle is used to customize ExpansionTile expand/collapse animation.

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.

// unpack.
return true;
}
// If we are not animating, respect node.isExpanded;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// If we are not animating, respect node.isExpanded;
// If we are not animating, respect node.isExpanded.

Comment on lines +996 to +1001
_currentLayoutDimensions = SliverLayoutDimensions(
scrollOffset: constraints.scrollOffset,
precedingScrollExtent: constraints.precedingScrollExtent,
viewportMainAxisExtent: constraints.viewportMainAxisExtent,
crossAxisExtent: constraints.crossAxisExtent
);
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation + missing comma at the end

///
/// It can be useful to expand or collapse nodes of the tree
/// programmatically, for example to reconfigure an existing node
/// based on a system event. To do so, create an [SliverTreeList]
Copy link
Contributor

Choose a reason for hiding this comment

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

an -> a (same on lines 158, 178, 231, 266 and 293)

Comment on lines +69 to +72
SliverTreeList<String>(
tree: _tree,
controller: controller,
treeNodeBuilder: (
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to customize the padding between the nodes and the labels?
It looks like gap between the nodes and labels is bit too high when you see something like GitHub or VS Code tree list.

Preview 1 Preview 2 Preview 3

Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: const Text('SliverTree demo'),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
title: const Text('SliverTree demo'),
title: const Text('SliverTree Demo'),

? Icons.file_open_outlined
: Icons.folder_outlined,
),
const SizedBox(height: 25.0),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const SizedBox(height: 25.0),
const SizedBox(height: 16.0),

Reduced the padding between selection icon and label.

import 'package:flutter_test/flutter_test.dart';

void main() {
testWidgets('SliverTree example', (WidgetTester tester) async {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
testWidgets('SliverTree example', (WidgetTester tester) async {
testWidgets('Can toggle nodes in SliverTree', (WidgetTester tester) async {

import 'package:flutter_test/flutter_test.dart';

void main() {
testWidgets('SliverTree example', (WidgetTester tester) async {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
testWidgets('SliverTree example', (WidgetTester tester) async {
testWidgets('Can toggle nodes in SliverTree', (WidgetTester tester) async {

);
expect(find.text('lib'), findsOneWidget);
expect(find.text('src'), findsNothing);
// Toggle tree node
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Toggle tree node
// Toggle tree node.

);
expect(find.text('Second'), findsOneWidget);
expect(find.text('alpha'), findsNothing);
// Toggle tree node
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Toggle tree node
// Toggle tree node.

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
4 participants