[Draft] FTree #712
[Draft] FTree #712
Conversation
KrisBraun
had a problem deploying
to
docs-development
GitHub Actions
Failure
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new hierarchical tree widget to the Forui library, adding FTree functionality for displaying nested data with visual connecting lines and expand/collapse support.
Key changes:
- Implements FTree widget with FTreeItem components for creating hierarchical data structures
- Adds visual connecting lines between parent and child nodes using custom painters
- Includes comprehensive styling system with FTreeStyle, FTreeItemStyle, and FTreeLineStyle
Reviewed Changes
Copilot reviewed 13 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/lib/widgets/tree.dart | Sample implementation demonstrating tree usage with folders and files |
| samples/lib/main.dart | Added tree route to navigation |
| forui/test/src/widgets/tree/tree_test.dart | Unit tests for tree functionality |
| forui/test/src/widgets/tree/tree_golden_test.dart | Golden tests for visual regression testing |
| forui/lib/widgets/tree.dart | Public export file for tree components |
| forui/lib/src/widgets/tree/tree_painter.dart | Custom painter for drawing tree connection lines |
| forui/lib/src/widgets/tree/tree_item.dart | Core tree item widget with expand/collapse logic |
| forui/lib/src/widgets/tree/tree_controller.dart | Controller for managing tree state |
| forui/lib/src/widgets/tree/tree.dart | Main tree widget implementation |
| forui/lib/src/theme/theme_data.dart | Integration with theme system |
| forui/lib/forui.dart | Added tree export |
| forui/CHANGELOG.md | Documentation of new features |
| docs/app/docs/data/tree/page.mdx | Complete documentation with examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }) : revealTween = revealTween ?? Tween(begin: 0.0, end: 1.0), | ||
| fadeTween = fadeTween ?? Tween(begin: 0.0, end: 1.0), | ||
| iconTween = iconTween ?? Tween(begin: 0.0, end: 0.25); |
There was a problem hiding this comment.
Consider adding named parameters for the default tween values to improve maintainability and allow easier customization. Magic numbers like 0.25 should be documented or made configurable.
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| // Draw horizontal lines to each child | ||
| const horizontalEndX = itemPadding - 2; // Stop 2px before the icon area |
There was a problem hiding this comment.
The magic number '2' should be documented or made configurable. Consider adding a named constant or parameter to explain why 2px spacing is needed before the icon area.
Copilot uses AI. Check for mistakes.
|
I fully welcome contributions (or replacements if I've gone the wrong direction). Given my limited time to work on this, I'd be very happy for anyone to step in to make improvements directly. Of course, if no one else has interest or capacity, I'll get to feedback as soon as I can. |
KrisBraun
had a problem deploying
to
docs-development
GitHub Actions
Failure
KrisBraun
had a problem deploying
to
docs-development
GitHub Actions
Failure
KrisBraun
had a problem deploying
to
docs-development
GitHub Actions
Failure
KrisBraun
had a problem deploying
to
docs-development
GitHub Actions
Failure
There was a problem hiding this comment.
I'm a bit stumped why this is showing an earlier version; when I put the test in the sample, it renders correctly. I've run flutter test test/src/widgets/tree/tree_golden_test.dart --update-goldens but it generates the same image.
|
Both Joe & I will have limited capacity for most of October and early November.
I think the expand/collapse arrow should on the LHS for LTR locales. We should probably reverse this for RTL locales. IMO it shouldn't replace any user supplied icon. Rather than it should push the user supplied icon to the right.
IMO we should definitely make this configurable via a parameter. Maybe something like an
Ideally the line should remain even when hovered. We probably need to play around with the colors to get the desired effect. |
|
The implementation has a couple of minor nits but looks okay in general! I think one important thing to note is that we need to add support for RTL locales (Basically mirror the current line drawing logic). |
KrisBraun
had a problem deploying
to
docs-development
GitHub Actions
Failure
|
Thanks, I've added RTL support. I'll keep picking away at improvements. |
Describe the changes
An initial attempt at #317. So far, it's simply a tree with line drawing (which was most of the challenge). I'll admit I relied heavily on Claude Code to bootstrap the new widget and haven't reviewed it all in detail yet.
At this point, it would be helpful to get feedback on the widget API and design. I see a few open UI design questions:
onPress? In my use case, I'd like clicking on the mainFTreeItemto always expand and triggeronPress(since it's a selection of the current item), with a click on the icon required to collapse it. Perhaps ifonPressisnullthen clicking anywhere always toggles.My intent is to create a separate PR to add reordering support after an initial FTree is merged.
Checklist