tree: add selected and reveal to TreeCfg - #80
jaysonsantos wants to merge 1 commit into
Conversation
The tree owned its expansion state and exposed no way to drive it. An application that changes the current node elsewhere in its interface, for example from a map or a breadcrumb, could not highlight that node or open the branch that holds it. `selected` marks one node. Its row draws with `color_select`, and hover leaves the row colour alone. `reveal` lists the ids to expand. It applies when `selected` changes, so a branch the user collapses afterwards stays collapsed. Both default to empty, so existing trees keep their behaviour.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6132ee05d
ℹ️ 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".
| if cfg.reveal.len == 0 { | ||
| return tree_map |
There was a problem hiding this comment.
When the newly selected node has no ancestors, callers naturally pass an empty reveal, but this return leaves the stored selection unchanged. For example, selecting root/child with ['root'], then selecting root with [], collapsing root, and selecting root/child again causes the stored value to still equal root/child, so the final reveal is skipped and the selected child remains hidden. Update the stored selection before returning so every selection transition is observed.
Useful? React with 👍 / 👎.
| for id in cfg.reveal { | ||
| tree_map[id] = true |
There was a problem hiding this comment.
When a revealed ID identifies an unloaded lazy node, setting only tree_map[id] marks it expanded without setting its loading state or invoking on_lazy_load. tree_collect_flat_rows consequently renders neither children nor a loading row, leaving the externally selected descendant inaccessible until the user collapses and manually expands the node. Programmatic expansion should trigger the same guarded lazy-load path as click or right-arrow expansion.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| is_selected := selected.len > 0 && selected == id |
There was a problem hiding this comment.
When selected matches a row, is_selected affects only its visual styling and hover behavior; the row's a11y_state remains solely expanded or none. Screen-reader users therefore receive no indication of which tree item is selected. Combine AccessState.selected with the existing expanded bit for matching rows.
Useful? React with 👍 / 👎.
1 participant
Problem
treeowns its expansion state inview_state.tree_stateand exposes no way for an application to drive it. If the current node changes somewhere else in the interface — a map, a breadcrumb, a search result — the tree cannot highlight that node or open the branch that holds it. The two views then disagree about where the user is.I hit this while building a disk-usage app: clicking a rectangle in a treemap navigated the app, but the tree beside it stayed on the old folder with no indication of the change.
Change
Two optional fields on
TreeCfg, both defaulting to empty, so existing trees behave exactly as before.selected string— marks one node. Its row draws withgui_theme.color_select, and hover no longer overwrites that colour.reveal []string— ids to expand. Applied whenselectedchanges, tracked in a newgui.tree.revealstate map.The reveal deliberately runs only on a
selectedchange rather than every frame. That way a branch the user collapses afterwards stays collapsed, instead of springing open again on the next render.This follows the controlled-component pattern already documented on
BreadcrumbCfg.Tests
Added
test_tree_reveal_expands_once_per_selection, covering the initial reveal, the collapse that must survive, and the re-reveal whenselectedmoves.All existing tests in
_view_tree_test.vpass, including the drag-reorder ones.Note on scrolling
My application also wants
scroll_to_viewon the selected row. That needs every row to carry a layout id, but today thetr_<cfg>_<node>id is set only whenreorderableis true, andtest_tree_nil_on_reorder_disables_reorder_idsasserts that. I left it alone: always setting an id would change behaviour for every existing tree, and it would be unreliable anyway, since a virtualized tree does not put off-range rows in the layout at all.Happy to follow up with a separate
Window.scroll_tree_to(cfg_id, node_id)that cooperates with virtualization, if you think that is worth having.