Add hierarchy-aware live filter to vendor model catalog#6256
Conversation
The vendor model catalog tree (Layout > Import models from xlights.org) can have hundreds of nodes spread across a dozen vendors. The existing Search button steps through matches one at a time but doesn't narrow the tree; users still scroll past every non-matching vendor and category to find what they want. Add two live wxSearchCtrl filter inputs above the tree: - AND-narrowing across both boxes (typing 'halloween' in box 1 then 'pumpkin' in box 2 shows only Halloween's pumpkins). - Hierarchy-aware match: each model's haystack is its full breadcrumb path 'Vendor / Category / Sub-category / ModelName' lower-cased, so typing a vendor or category name surfaces the whole sub-tree. - Empty branches pruned bottom-up by a new PruneEmptyBranches pass — the original DeleteEmptyCategories only caught leaf categories that were already empty when first visited and never deleted vendors, so filtered results would be cluttered with empty parent rows. - ExpandAll while a filter is active so the user sees every survivor without re-expanding each vendor on every keystroke. When both filters clear, the tree returns to its original collapsed-vendor default. - 200 ms debounce — full tree rebuild is O(N+M) and re-allocates a ModelElement per visible node, so collapsing fast keystrokes into one rebuild matters on big catalogs. - All UI lives outside the wxSmith block; the .wxs file does not need to know about it. The original step-through Search button row at the bottom is kept unchanged — power users who want next-match navigation still have it.
There was a problem hiding this comment.
Pull request overview
Adds a hierarchy-aware, debounced live filtering UI to the vendor model catalog tree in VendorModelDialog, allowing users to narrow visible models as they type (AND-ing two filter terms) while pruning empty branches.
Changes:
- Added two
wxSearchCtrlinputs above the catalog tree with a 200ms debounce and immediate rebuild on cancel. - Refactored tree construction into
RebuildTreeUI()and added bottom-upPruneEmptyBranches()to remove empty categories/vendors after filtering. - Updated release notes entry describing the new filtering behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src-ui-wx/import_export/VendorModelDialog.h | Adds filter UI members/handlers and new prune/rebuild helpers. |
| src-ui-wx/import_export/VendorModelDialog.cpp | Implements filter controls, debounced rebuild, path-aware match, and bottom-up pruning. |
| README.txt | Adds a 2026.08 release note entry for the new live filtering feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I would add this as search modifiers on the current search box. i.e "pumpkin && boscoyo" This would allow infinite length not just two |
|
I understand, but && isn't very user-friendly or intuitive. The second filter option lets you filter the results list. In testing, it seems to do an admirable job of whittling the list down to a very very manageable level |
|
I would also argue that two unlabeled search boxes is not very intuitive, windows explorer, finder, chrome, don't use double search boxes |
|
They are like mushrooms... now we'll have 3? |
|
think of them as anchors eventually you'll have to come up for air |
Four points raised, all valid: 1. PruneEmptyBranches deleted suppressed vendors. Suppressed vendors intentionally have zero children (we skip AddHierachy for them in the tree build) so the bottom-up prune was treating them as orphans and dropping them — meaning users couldn't see them in the tree and therefore couldn't un-suppress via the 'Don't download this vendors list of models' checkbox panel. Skip pruning when the empty Vendor node is suppressed; empty non-suppressed vendors (only happens with a filter active) still get pruned as intended. 2. VendorModelDialog.h was using wxTimer* and wxTimerEvent in member declarations and handler signatures without including <wx/timer.h>. Made the header self-contained. 3. The header-comment block above the catalog filter members still referred to the old 'pruned by DeleteEmptyCategories' path and 'matches model names', neither of which is true after the PruneEmptyBranches + path-aware-match work. Updated the comment. 4. CatalogFilterMatches() became dead code once AddModels was refactored to call CatalogFilterMatchesPath directly. Removed the wrapper and updated the path-aware function's doc comment to stand alone.
Replaces the dual TextCtrl_Filter1/Filter2 inputs with a single TextCtrl_Filter where the user types space-separated terms and each is AND-narrowed against the breadcrumb path. e.g. typing 'tree EFL' is now equivalent to what 'tree' in box 1 + 'EFL' in box 2 used to do, but in one input — much closer to Spotlight / VS Code search behaviour. Mechanically: - Single wxSearchCtrl with placeholder 'Filter catalog (space-separated terms)...' - Filter state moves from two wxString members to a std::vector<wxString> _filterTokens lower-cased on capture - CatalogFilterMatchesPath loops the tokens and requires every token to appear somewhere in the path haystack - OnCatalogFilterText tokenizes via wxStringTokenizer on space/tab - OnCatalogFilterCancel just clears the input and the token vector, no per-control event-source disambiguation needed - ExpandAll-while-filtering check now keys off !_filterTokens.empty() rather than two IsEmpty() checks Net: 91 lines of cpp + 32 lines of header churn for a feature that feels measurably nicer to use.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Round 2 of the suppressed-vendor handling in PruneEmptyBranches. The previous round protected suppressed vendors from being pruned period — but that meant 'DMX Fixture Library' (suppressed by default) stayed visible even when the user typed a filter that matched nothing under it, which is confusing. Refine: only protect suppressed vendors from pruning when there's NO active filter. Under an active filter, suppressed vendors with no surviving descendants get pruned along with everything else that doesn't match. Without a filter, they still show so the user can un-suppress via the 'Don't download this vendors list of models' checkbox panel.
Copilot caught the inconsistency: code now has ONE filter input that AND-narrows on whitespace-separated tokens (typing 'tree EFL' matches both terms in the path), but README still described 'two live-filter inputs' and a 'second box'. Realign the release note text with what shipped.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
EnsureVisible(first) ran on every filter rebuild, scrolling the tree back to the top on every keystroke. Gate it on a _initialBuild flag so it only runs the first time.
Windows tester reported a hang/crash typing 'efl' into the catalog filter. Most likely cause: DeleteAllItems on a tree with an active selection fires selection-changed events that race with the rebuild. UnselectAll first to silence them. Also guard against re-entrant rebuilds.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move rebuild guard from static-local to class member so OnTreeCtrl_NavigatorSelectionChanged can also short-circuit while a rebuild is in flight (the handler does network IO via PopulateVendorPanel for the vendor logo, which was blocking the UI thread when triggered by the rebuild's UnselectAll). - Replace ExpandAll with a targeted 'expand surviving vendor nodes only' loop. Categories are already auto-expanded by AddHierachy during the build, so ExpandAll was redundantly walking the entire tree on Windows native wxTreeCtrl which is expensive at scale.
User reported on Windows that typing 'EF' froze the UI, clicking the tree, then typing 'L' crashed. Hypothesis: the click queued an event that fired after rebuild deleted the clicked item, leaving a stale wxTreeItemId in SelectionChanged. - Disable/Enable the tree across the rebuild so clicks/keys can't reach it during the (potentially slow) rebuild. - RAII guard so Thaw + Enable + flag reset always run, even if a later step throws or returns early.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per-step spdlog tracing on Windows confirmed the rebuild hung in the post-prune per-vendor Expand call: log stopped at 'expand[0] children=8' and never wrote the matching done line. Each vendor that survived the filter has many already-expanded categories underneath. When we Expand the vendor itself, the visible-item count explodes and the Win32 TreeView_Expand call during Freeze never returns. Leave matched vendors collapsed — users click to drill in, same UX as the unfiltered tree.
Restores the auto-expand of matched vendors under the filter so users see results without clicking. Avoids the Windows hang by skipping the per-category Expand inside AddHierachy when a filter is active — expanding the vendor then only reveals collapsed first-level categories instead of every nested grandchild, keeping the visible- item count small enough that Win32 TreeView_Expand returns.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
v18 log proved the rebuild produced the right internal state (root_children=1 for an efl filter that should leave only EFL Designs) but the screenshot still showed all 10 vendors. Windows native wxTreeCtrl can leave the pre-Freeze frame on screen after Thaw if many items were added/removed during the freeze. Explicitly invalidate and update so the user sees the filtered tree.
…lback Queued SEL_CHANGED events fired AFTER RebuildTreeUI returns: the events item id was deleted by the rebuild so GetItemData returned null, my previous validation then fell back to GetFocusedItem which picked up a freshly-built item from the NEW tree. PopulateModelPanel then ran on that item and called synchronous model->DownloadImages(), freezing the UI for many seconds when GitHub rate-limited us. Drop the fallback. If the events item is stale, treat the event as having no selection — the panel hides until the user actually clicks something.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Bug in v19: Refresh+Update ran inside the function body, but the RebuildScopes destructor that calls Thaw only fires when the function returns. Calling Refresh while still frozen left the tree visible area painted with an empty frame. Use CallAfter so the post-rebuild paint happens after the scope guard has thawed and the timer event has unwound.
Per-vendor and multi-wiring Expand calls under filter consistently hang Windows on certain vendor data shapes (EFL Designs). User confirmed boscoyo, tree, arches all work but EFL locks every time. Removing both auto-expands keeps the rebuild simple and stable — matched vendors render collapsed, user clicks to drill in, same UX as the unfiltered tree.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Closing this PR. The hierarchy-aware filter works smoothly on macOS but Windows native wxTreeCtrl hangs during the rebuild for certain vendors (EFL Designs, Gilbert Engineering — possibly others with deep nesting or many multi-wiring models). After 20+ iterations and detailed per-step spdlog tracing the Windows hang point kept shifting — first the vendor Expand call, then post-rebuild paint sync, then queued selection events. We kept making progress on each one but always uncovered another platform-specific edge case. Without an interactive Windows debugger / repro on this side it's not solvable in this iteration. Filter runs beautifully on macOS. Reopening in the future when better Windows tooling is available. |
|
Reopening — tested on macOS Debug and the Windows CI build (
Full per-step |
|
Bulk-resolved the prior Copilot review threads (27 of them). Most pointed at intermediate code paths from the debugging iteration on this branch — e.g. "both filter inputs" wording (only one input now), debug |
|
Worth flagging: this PR took ~45 iterations to land cleanly. The hierarchy-aware filter logic itself was straightforward, but reaching a build that's stable on both macOS and Windows required a long debugging cycle — primarily chasing a Windows-only EFL hang in |
|
@dkulp / @derwin12 — gentle bump on this one. Tested cleanly on macOS Debug + the Windows CI build. The 45-iteration debugging cycle turned up the EFL hang on Windows which is fully fixed at the current head. All Copilot threads resolved, CI green, mergeable. If anyone has bandwidth to give it another pass I'd really appreciate it — just trying to avoid the diff going stale against master. |
…er-6256 # Conflicts: # README.txt # macOS
…encer#6256) Replace the multi-level category tree with a two-level tree: Vendor -> Model. Each vendor node now holds a flat list of its models built by iterating the vendor's owning _models list, so a model listed under several categories appears exactly once (de-duplicated by identity) instead of repeating per category. Filtering stays hierarchy-aware: a model matches if the vendor name or any of its category paths (resolved via Category::GetPath) plus the model name satisfies every filter token, so typing a vendor or category name still surfaces that node's models even though categories are no longer shown as rows. Removed AddHierachy / AddModels / DeleteEmptyCategories (category-node machinery no longer needed); PruneEmptyBranches still drops vendors with no matching models. Selection, detail pane and download are unchanged - model and wiring leaves keep their existing item data.
| // Master's #6267 added a wxSearchCtrl at the bottom of Panel3 | ||
| // (TextCtrl_Search). Our top filter supersedes it; hide so | ||
| // the user only sees one search box. | ||
| if (TextCtrl_Search != nullptr) { | ||
| TextCtrl_Search->Hide(); | ||
| } |
| // The tree is now two levels (Vendor -> Model), so there are no | ||
| // category nodes to prune; PruneEmptyBranches just drops Vendor nodes | ||
| // that ended up with no matching models under the active filter. | ||
| spdlog::info("VMD::RTUI step 6: PruneEmptyBranches start"); | ||
| PruneEmptyBranches(root); |
| spdlog::info("VMD::RebuildTreeUI ENTER tokens={} rebuilding={} vendors={}", | ||
| _filterTokens.size(), _treeRebuilding, _vendors.size()); | ||
| if (_treeRebuilding) { | ||
| spdlog::warn("VMD::RebuildTreeUI BAILED at entry guard"); | ||
| return; | ||
| } |
| CallAfter([this]() { | ||
| if (TreeCtrl_Navigator != nullptr) { | ||
| TreeCtrl_Navigator->Refresh(); | ||
| TreeCtrl_Navigator->Update(); | ||
| } | ||
| }); |
- Multi-wiring models no longer create a nested wiring sub-level. Each wiring becomes its own flat 'Model - Wiring' leaf under the vendor, so every result row is one level deep and directly downloadable (the download path needs a wiring node; a bare multi-wiring model node resolves to no wiring). - Auto-expand surviving vendor nodes when a filter is active so matches show without a click. Safe now that the tree is two levels (Vendor -> Model) - no deep category subtree to explode the visible-item count. Vendors stay collapsed with no filter.
|
Summary
Adds a live
wxSearchCtrlfilter input above the vendor model catalog tree (Layout → Import models from xlights.org) that narrows the tree as you type, instead of stepping through matches one at a time the way the existing Search button does.Why
The catalog has hundreds of nodes across a dozen vendors. Even after the multi-select work in #6073, finding what you want still meant scrolling past every non-matching vendor and category. Step-through Search is fine for "I know there's a pumpkin somewhere" but bad for "let me see all of Boscoyo's Christmas options."
Behaviour
tree EFLis two terms — both must appear somewhere in the path. Replaces an earlier two-input prototype after feedback that one box with tokens is cleaner.Vendor / Category / Sub-category / ModelNamelower-cased, so typing a vendor or category name surfaces the entire sub-tree, not just leaves whose own names happen to contain the term.PruneEmptyBranchespass drops categories and vendors that have no surviving descendants. Suppressed vendors (e.g. "DMX Fixture Library", suppressed by default) stay visible when no filter is active so users can un-suppress them via the checkbox panel; under an active filter they get pruned along with everything else that doesn't match.ExpandAllso users see every survivor without re-expanding each vendor on every keystroke. Cleared filter returns to the default collapsed-vendor state.ModelElementper visible node; collapsing fast keystrokes into one rebuild matters on big catalogs. Cancel button (×) rebuilds immediately — explicit user action shouldn't wait.Implementation notes
.wxsfile is unchanged.RebuildTreeUI()so it's callable on every filter change.AddHierachyandAddModelsthread the lowercased ancestor path string down for the haystack match.AddModelsearly-returns on filter mismatch — every code path into the visible tree goes through it.wxStringTokenizeron space/tab; stored in astd::vector<wxString>(lower-cased).Test plan