Add built-in TreeView checkbox mode#5429
Conversation
|
Does it work with treetable source though? The source for tableview that makes column 0 expandable tree? |
|
@tznind this is not really ready for review.
Can you explain this more? I may be missing something that's already there. thanks! |
public class TreeTableSource : IEnumerableTableSource, IDisposable where T : class It combines treeview with tableview. Giving you columns. Also you can wrap the source with the checkbox wrapper source that gaven you checkbox column at start. But i believe that wrapper got replaced with direct checkbox support in table view at some point (if my memory serves) |
55bb60a to
cdbc3d3
Compare
Good question. No, CheckboxMode is purely for standalone TreeView rendering; it adds the checkbox glyph in Branch.Draw(). When TreeView is used as a data model behind TreeTableSource, the checkbox glyphs aren't rendered in the table column (since GetColumnZeroRepresentationFromTree only pulls prefix + expand symbol + AspectGetter text). For tree-table checkboxes, the existing CheckBoxTableSourceWrapper pattern still applies — wrap the TreeTableSource with it. The two mechanisms are independent. I've documented this distinction in both treeview.md and tableview.md. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1401d1fd72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds first-class checkbox support to TreeView<T>, including check-state APIs, checkbox rendering/toggling, UICatalog editor exposure, documentation, and tests.
Changes:
- Adds
CheckboxMode, checked-state storage/API,CheckedChanged, checkbox glyph rendering, and keyboard/mouse toggling. - Adds tests for checkbox rendering, propagation, tri-state behavior, and mouse interactions.
- Updates TreeView/TableView docs and UICatalog editor controls for the new mode.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Terminal.Gui/Views/TreeView/TreeViewT.cs |
Adds checkbox mode state, event, cleanup, and activation handling. |
Terminal.Gui/Views/TreeView/TreeView.Navigation.cs |
Adds check-state APIs and recursive check-state helpers. |
Terminal.Gui/Views/TreeView/Branch.cs |
Renders checkbox glyphs and updates sizing/hit testing. |
Terminal.Gui/Views/TreeView/ITreeView.cs |
Adds checkbox mode to the interface. |
Terminal.Gui/Views/TreeView/CheckedChangedEventArgs.cs |
Adds event args for check-state changes. |
Examples/UICatalog/Scenarios/EditorsAndHelpers/TreeViewEditor.cs |
Exposes checkbox mode in the TreeView editor scenario. |
Tests/UnitTestsParallelizable/Views/TreeViewTests.cs |
Adds checkbox mode behavioral and rendering coverage. |
docfx/docs/treeview.md |
Documents checkbox mode usage and semantics. |
docfx/docs/tableview.md |
Clarifies TreeTableSource checkbox behavior. |
- Added CheckboxMode to ITreeView interface for editor access - Added CheckboxMode toggle checkbox to TreeViewEditor settings panel Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restructure OnActivated to handle mouse clicks before checking SelectedObject. Previously, if SelectedObject was null, the method returned early without processing mouse events (checkbox toggle, expand/collapse). Now mouse clicks use HitTest directly and only the keyboard activation path checks SelectedObject. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove redundant MouseBindings.Add(LeftButtonClicked, Command.Activate) from TreeView constructor. The base View.DefaultMouseBindings already maps LeftButtonReleased to Command.Activate. Having both caused OnActivated to fire twice per click (once for Released, once for synthesized Clicked), toggling state on then immediately off. Also rewrote tests to use the full mouse pipeline (app.InjectSequence with InputInjectionExtensions.LeftButtonClick) instead of calling NewMouseEvent directly, ensuring tests exercise the same code path as the real application. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restore GetEffectiveCheckState to derive parent state from children - Add SetCheckedRecursive to propagate check state to all descendants - Parent shows indeterminate (None) when children have mixed states - Toggling parent checks/unchecks all descendants - Update tests to verify tri-state behavior and propagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Checkbox Mode section to treeview.md covering tri-state behavior, interaction, programmatic API, and events - Update keyboard/mouse tables to reflect checkbox interactions - Add note to tableview.md Tree Tables section clarifying that TreeTableSource does not render CheckboxMode glyphs — users should use CheckBoxTableSourceWrapper for tree-table checkboxes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- GetEffectiveCheckState: only derive from expanded/known children, not from lazy TreeBuilder.GetChildren (avoids materializing expensive subtrees during paint) - SetCheckedRecursive: add visited HashSet to prevent infinite recursion on cyclic tree builders - SetCheckedCore: report effective (derived) OldValue in CheckedChanged event, not raw dictionary value - Branch.Draw: account for checkbox cells in scrolled indexOfModelText calculation - Fix doc example: IEnumerable<T> not IReadOnlyCollection<T> - Add tests for cyclic tree builder protection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ToggleChecked now correctly handles all cases:
- If effective state is not UnChecked, toggle to UnChecked (propagates to children)
- If effective state is UnChecked but descendants have explicit checked state
(collapsed parent scenario), also toggle to UnChecked
- Only toggle to Checked when fully unchecked with no checked descendants
- Add HasCheckedDescendantsInState helper for collapsed-parent detection
- Add comprehensive mouse-click test that clicks children individually then
unchecks parent (mimics exact UICatalog user scenario)
- Revert Glyphs.cs emoji changes (PR #5439 handles glyph updates separately)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetEffectiveCheckState previously required IsExpanded:true to derive state from children. But ChildBranches are preserved in memory after collapse, so the parent can still derive its tri-state correctly without calling TreeBuilder.GetChildren (which was the original perf concern). Fix: Remove the IsExpanded guard from GetEffectiveCheckState so collapsed parents correctly show indeterminate/checked based on their known children. Tests added (all verified to FAIL without fix, PASS with fix): - Collapsed_Parent_Shows_Indeterminate_When_One_Child_Checked - Collapsed_Parent_Shows_Checked_When_All_Children_Checked - Collapsed_Parent_Shows_UnChecked_When_No_Children_Checked Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1e2a901 to
add55fa
Compare
Fixes
TreeView multi-selection only rendered highlighted rows, requiring custom rendering/input code for checkbox-style trees. This adds native checkbox rendering and toggling for node-level checked state.
Proposed Changes/Todos
TreeView<T>.CheckboxModefor built-in checkbox glyph rendering.GetCheckedObjects()GetCheckState(T?)SetChecked(T?, CheckState)CheckedChangedPull Request checklist:
CTRL-K-Dto automatically reformat your files before committing.dotnet testbefore commit///style comments)