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.

Sign up for GitHub

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

Jump to bottom

[two_dimensional_scrollables] TreeView #6592

Merged
merged 32 commits into from
May 16, 2024
Merged

[two_dimensional_scrollables] TreeView #6592

merged 32 commits into from
May 16, 2024

Conversation

Piinks
Copy link
Contributor

@Piinks Piinks commented Apr 22, 2024 •
edited
Loading

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

📜 Design Document

Adds classes and associated callbacks and controllers for TreeView. Core components:

  • TreeView
  • TreeViewport
  • RenderTreeViewport
  • TreeViewController
  • TreeViewNode
  • TreeViewStateMixin
  • TreeViewIndentationType
  • TreeRowDelegateMixin
  • TreeRowBuilderDelegate

Along with Span subclasses for trees:

  • TreeRowPadding
  • TreeRowExtentDelegate
  • TreeRowExtent (and subclasses)
  • TreeRowBorder

Fixes flutter/flutter#42332
Fixes flutter/flutter#126298

treeView_0.mov
treeView_1.mov

Pre-launch Checklist

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

@Piinks Piinks added the p: two_dimensional_scrollables Issues pertaining to the two_dimensional_scrollables package label Apr 22, 2024
@Piinks Piinks mentioned this pull request Apr 22, 2024
Merged
9 tasks
@Piinks Piinks marked this pull request as ready for review April 24, 2024 22:36
goderbauer
goderbauer reviewed May 7, 2024
View reviewed changes
packages/two_dimensional_scrollables/example/lib/table_view/infinite_table.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/example/lib/tree_view/custom_tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/example/lib/tree_view/custom_tree.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/example/lib/tree_view/custom_tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/example/lib/tree_view/simple_tree.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree_delegate.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree_delegate.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart Outdated Show resolved Hide resolved
@Piinks
Copy link
Contributor Author

Piinks commented May 9, 2024

No rush on the re-review, it's a big one. Best taken in breaks. :)

Review feedback diff is contained in 5a0194c

goderbauer
goderbauer reviewed May 10, 2024
View reviewed changes
packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart
final T _content;

/// Other [TreeViewNode]s this this node will be [parent] to.
List<TreeViewNode<T>> get children => _children;
Copy link
Member

Choose a reason for hiding this comment

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

Is this true? didUpdateWidget wouldn't be called if I pass a TreeViewNode to TreeView and then later on call .children.append(...) on that TreeViewNode? Or can you show in a test that that actually works?

packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree_core.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart Show resolved Hide resolved
packages/two_dimensional_scrollables/example/lib/tree_view/custom_tree.dart Show resolved Hide resolved
@Piinks Piinks mentioned this pull request May 14, 2024
Open
@Piinks Piinks mentioned this pull request May 14, 2024
Open
@Piinks
Copy link
Contributor Author

Piinks commented May 14, 2024

Filed follow up issues:

@Piinks Piinks requested a review from goderbauer May 14, 2024 22:34
goderbauer
goderbauer approved these changes May 15, 2024
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart Outdated Show resolved Hide resolved
packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart Outdated Show resolved Hide resolved
@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label May 15, 2024
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented May 15, 2024

auto label is removed for flutter/packages/6592, due to This PR has not met approval requirements for merging. Changes were requested by {TahaTesser}, please make the needed changes and resubmit this PR.
The PR author is a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 15, 2024
@Piinks
Copy link
Contributor Author

Piinks commented May 15, 2024

Oh right, the autosubmit bot has a bug where if anyone submits a "changes requested" review, even if it is dismissed, the bot can't tell. I'll wait for CI to pass and merge it manually.

@Piinks Piinks merged commit b83b63c into flutter:main May 16, 2024
74 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 16, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 16, 2024
@engine-flutter-autoroll
b83b63c55 [two_dimensional_scrollables] TreeView (flutter/packages#6592)
008497e
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 17, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 17, 2024
@engine-flutter-autoroll
b83b63c55 [two_dimensional_scrollables] TreeView (flutter/packages#6592)
9f1fef1
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 17, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 17, 2024
@engine-flutter-autoroll
b83b63c55 [two_dimensional_scrollables] TreeView (flutter/packages#6592)
9916fcf
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 17, 2024
@engine-flutter-autoroll
Roll Packages from 87a02e393be0 to ae4dd32f4a66 (9 revisions) (#148555)
3695802 
flutter/packages@87a02e3...ae4dd32

2024-05-17 engine-flutter-autoroll@skia.org Roll Flutter from 0d22d91 to 00425ef (14 revisions) (flutter/packages#6753)
2024-05-16 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Add test for `onExit` (flutter/packages#6614)
2024-05-16 32666446+hamdikahloun@users.noreply.github.com [camera_android_camerax] update to latest stable camerax 1.3.3 (flutter/packages#6737)
2024-05-16 magder@google.com [camera_avfoundation] Revert camera example PRODUCT_BUNDLE_IDENTIFIER (flutter/packages#6735)
2024-05-16 15619084+vashworth@users.noreply.github.com [file_selector_ios, image_picker_ios] Remove Swift Package Support (flutter/packages#6740)
2024-05-16 katelovett@google.com [two_dimensional_scrollables] TreeView (flutter/packages#6592)
2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter from 39651e8 to 0d22d91 (23 revisions) (flutter/packages#6748)
2024-05-16 byoungchan.lee@gmx.com [pigeon][swift] Removes FlutterError in favor of PigeonError (flutter/packages#6611)
2024-05-16 15619084+vashworth@users.noreply.github.com [webview_flutter] Skip "Video playback policy" drive tests (flutter/packages#6747)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
@Piinks @TecHaxter
[two_dimensional_scrollables] TreeView (flutter#6592)
f55d614
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 4, 2024
@Piinks
TreeSliver & associated classes (#147171)
b50eb97 
**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](https://docs.google.com/document/d/1-aFI7VjkF9yMkWpP94J8T_JREDS-M3bOak26PVehUYg/edit?usp=sharing)

This adds classes and associated callbacks and controllers for TreeSliver. Core components:
- TreeSliver
- RenderTreeSliver
- TreeSliverNode
- TreeSliverController
- TreeSliverStateMixin
- TreeSliverIndentationType

Fixes #114299

https://github.com/flutter/flutter/assets/16964204/3facd095-7262-4068-aa33-d713e2deca99

https://github.com/flutter/flutter/assets/16964204/f851ae30-8e71-45c7-82a4-9606986a5872
@auto-submit auto-submit bot mentioned this pull request Jun 5, 2024
Merged
9 tasks
auto-submit bot added a commit to flutter/flutter that referenced this pull request Jun 5, 2024
@auto-submit
Reverts "TreeSliver & associated classes (#147171)" (#149754)
27e0656 
Reverts: #147171
Initiated by: QuncCccccc
Reason for reverting: tree is red due to a test in this PR
Original PR Author: Piinks

Reviewed By: {QuncCccccc, TahaTesser, bleroux}

This change reverts the following previous change:
**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](https://docs.google.com/document/d/1-aFI7VjkF9yMkWpP94J8T_JREDS-M3bOak26PVehUYg/edit?usp=sharing)

This adds classes and associated callbacks and controllers for TreeSliver. Core components:
- TreeSliver
- RenderTreeSliver
- TreeSliverNode
- TreeSliverController
- TreeSliverStateMixin
- TreeSliverIndentationType

Fixes #114299

https://github.com/flutter/flutter/assets/16964204/3facd095-7262-4068-aa33-d713e2deca99

https://github.com/flutter/flutter/assets/16964204/f851ae30-8e71-45c7-82a4-9606986a5872
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@Piinks @arc-yong
[two_dimensional_scrollables] TreeView (flutter#6592)
917b173
@reidbaker reidbaker mentioned this pull request Jul 2, 2024
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goderbauer goderbauer goderbauer approved these changes

@HansMuller HansMuller Awaiting requested review from HansMuller

@QuncCccccc QuncCccccc Awaiting requested review from QuncCccccc

@bleroux bleroux Awaiting requested review from bleroux

@TahaTesser TahaTesser Awaiting requested review from TahaTesser

Assignees
No one assigned
Labels
p: two_dimensional_scrollables Issues pertaining to the two_dimensional_scrollables package platform-macos
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2D Follow up: More sample code 2D TreeView widget
4 participants
@Piinks @bleroux @goderbauer @TahaTesser