Add filter to Edit Display Elements Available list#6241
Merged
computergeek1507 merged 5 commits intoApr 29, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an inline search filter to the Available list in Edit Display Elements to make large shows easier to navigate, using an existing wxSearchCtrl UX pattern already present elsewhere in the app.
Changes:
- Adds a
wxSearchCtrlabove the Available (non-models) list and binds live filtering + cancel-to-clear. - Applies filtering inside the list-population gateways (
AddModelToNotList/AddTimingToNotList) and triggersPopulateModels()on filter changes. - Auto-clears the filter after adding items to the view, restoring the full Available list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src-ui-wx/layout/ViewsModelsPanel.h | Declares the filter control/state and helper/event handlers. |
| src-ui-wx/layout/ViewsModelsPanel.cpp | Inserts the search control into the grid, wires events, enforces filtering, and clears filter after adds. |
| README.txt | Release note entry for the new filter UX. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
I have worked on a lot of large shows over the last 10 years and I have never needed any of these search boxes.... |
Contributor
Author
|
i work on my large show everyday and its a useful feature. Not sure why its a problem to incorporate a useful feature. Takes up very little real estate and provides the end user a way to reduce the list to a manageable level to select the prop they are looking for. I don't know if your shows are many props but when the list is greater than 25 it starts to become difficult and when its 50 its hard and by 100 its impossible. |
Contributor
Author
|
Examples below: 
|
heffneil
pushed a commit
to heffneil/xLights
that referenced
this pull request
Apr 27, 2026
Three issues raised, all valid: 1. Memory leak in PopulateModels. Filter check was inside AddModelToNotList, but the model-groups / regular-models loops allocate a fresh ModelElement* up front and pass it in. When the item was filtered out, AddModelToNotList returned without taking ownership, leaking per keystroke on a filtered list. Move the filter check up to the call site so we never allocate when we would just drop the result. 2. Encoding consistency. _nonModelFilter was stored via wxString::ToStdString() but later compared with wxString::FromUTF8(_nonModelFilter), which only matches when the wxString build is UTF-8. Switch _nonModelFilter to wxString throughout (already lower-cased) so non-ASCII names work on every wx build. Also flip the few .empty()/.clear() callsites to .IsEmpty()/.Clear(). 3. SetEffectSequenceMode AddGrowableRow(1) confusion. Row 1 is now shared with the [filter + ListCtrlNonModels] box sizer on col 0. Making row 1 growable still does the right thing because the filter sits at proportion 0 inside the box sizer and only the listctrl below it (proportion 1) absorbs the row growth. Add a comment so future readers don't have to derive that.
heffneil
force-pushed
the
feat/display-elements-filter
branch
from
152f1f0 to
62093c7
Compare
heffneil
pushed a commit
to heffneil/xLights
that referenced
this pull request
Apr 27, 2026
Three issues raised, all valid: 1. Memory leak in PopulateModels. Filter check was inside AddModelToNotList, but the model-groups / regular-models loops allocate a fresh ModelElement* up front and pass it in. When the item was filtered out, AddModelToNotList returned without taking ownership, leaking per keystroke on a filtered list. Move the filter check up to the call site so we never allocate when we would just drop the result. 2. Encoding consistency. _nonModelFilter was stored via wxString::ToStdString() but later compared with wxString::FromUTF8(_nonModelFilter), which only matches when the wxString build is UTF-8. Switch _nonModelFilter to wxString throughout (already lower-cased) so non-ASCII names work on every wx build. Also flip the few .empty()/.clear() callsites to .IsEmpty()/.Clear(). 3. SetEffectSequenceMode AddGrowableRow(1) confusion. Row 1 is now shared with the [filter + ListCtrlNonModels] box sizer on col 0. Making row 1 growable still does the right thing because the filter sits at proportion 0 inside the box sizer and only the listctrl below it (proportion 1) absorbs the row growth. Add a comment so future readers don't have to derive that.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a live filter control to the Available list in Edit Display Elements to make large model/timing/group sets easier to navigate, following the existing wxSearchCtrl UX pattern used elsewhere in the app.
Changes:
- Add a
wxSearchCtrlabove the Available (non-models) list and wire it to live-filter as the user types. - Enforce filtering through the “add-to-Available-list” pathways and clear the filter automatically after adding items.
- Update release notes to mention the new filter behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src-ui-wx/layout/ViewsModelsPanel.h | Adds member state and helpers for the Available-list filter. |
| src-ui-wx/layout/ViewsModelsPanel.cpp | Inserts the search control into the existing GridBagSizer layout and applies filter logic during list population/add. |
| README.txt | Adds a release note entry describing the feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
I also like the idea of a filter (without the need of clicking a button) anywhere that can have a long list of objects (display elements, groups in layout, import wizard, etc). Even better if it supports regex. |
heffneil
pushed a commit
to heffneil/xLights
that referenced
this pull request
Apr 27, 2026
Address Copilot follow-up on xLightsSequencer#6241: PopulateModels rebuilds both list controls and re-allocates ModelElement objects for every model not already in the sequence. Running it on every keystroke is fine on small shows but visibly stutters past ~500 models. Add a 150 ms one-shot wxTimer: - wxEVT_TEXT stores the new filter text immediately, then restarts the timer - The rebuild fires only when the user pauses, collapsing fast typing into a single rebuild - The cancel button (X) still rebuilds immediately - explicit user action shouldn't wait - AddSelectedModels also stops any pending timer so the clear-on-add rebuild isn't shadowed by a stale debounce firing right after Freeze/Thaw on both list controls is already in PopulateModels, so paint batching is already covered. Caches the filter text on every keystroke so callers that read _nonModelFilter (e.g. PopulateModels via IsFilteredOutOfNonModels) always see the current value, even before the debounce fires.
|
i like it and think its useful |
|
I wanted to chime in to support this. Filtering on long lists is one of those features that feels unnecessary right up until you have it, and then you can't imagine going back. Even folks who've gotten fast at scrolling and scanning end up reaching for it once it's there. Doesn't change anyone's existing workflow, just gives people another way in. Appreciate the effort on this! |
|
This would be nice to have, any list that can be bigger than the screen should have a filter option for good user experience |
Member
|
Can you fix the merge conflict and I will merge it |
added 4 commits
April 28, 2026 18:07
Large shows can have hundreds of models/groups/timings; finding one to add to a view via scroll-and-eye is tedious. Add a wxSearchCtrl above the Available list (left side of Edit Display Elements) using the same pattern as the Controllers tab visualizer filter and the ControllerModelDialog model filter — descriptive 'Filter models...' text, cancel button, case-insensitive substring match against the model/timing/group name. The filter clears automatically when items are moved into the view (Add Selected, Add All, drag-to-right, key-right). After moving, the user almost always wants to see the full list again to pick the next thing to add; if they need the same filter they can retype. Implementation notes: - All UI/state lives outside the wxSmith block, so the .wxs file does not need to change. - ListCtrlNonModels was at GBPosition(1,0) span(3,1); we detach it, drop the search ctrl at (1,0), and re-attach the list at (2,0) span(2,1). The growable row stays row 3, so the list still fills. - The filter is enforced in AddModelToNotList / AddTimingToNotList (both gateways into the visible list), so PopulateModels rebuilds with the filter applied without any other code path needing to know about the filter.
Two follow-ups after first round of testing: 1. Layout: row 1 of the GridBagSizer is stretched by ListCtrlViews on the right (the tall view picker), so putting the search ctrl in its own GB row left a tall empty gap between filter and list. Wrap the search ctrl + ListCtrlNonModels in a vertical wxBoxSizer and put that single sizer back at the original GBPosition(1, 0) span(3, 1). Filter now sits flush above the list. 2. Asserting in EnsureVisible. PopulateModels saves the previous top index and tries to restore it after rebuilding. With a filter applied the list shrinks, so the saved topN may be >= the new GetItemCount, tripping wx's listctrl bounds assert. Bounds-check topM/topN before calling EnsureVisible — the existing code already bounds-checked topM+visible-1 / topN+visible-1, but the second plain call was unguarded.
Three issues raised, all valid: 1. Memory leak in PopulateModels. Filter check was inside AddModelToNotList, but the model-groups / regular-models loops allocate a fresh ModelElement* up front and pass it in. When the item was filtered out, AddModelToNotList returned without taking ownership, leaking per keystroke on a filtered list. Move the filter check up to the call site so we never allocate when we would just drop the result. 2. Encoding consistency. _nonModelFilter was stored via wxString::ToStdString() but later compared with wxString::FromUTF8(_nonModelFilter), which only matches when the wxString build is UTF-8. Switch _nonModelFilter to wxString throughout (already lower-cased) so non-ASCII names work on every wx build. Also flip the few .empty()/.clear() callsites to .IsEmpty()/.Clear(). 3. SetEffectSequenceMode AddGrowableRow(1) confusion. Row 1 is now shared with the [filter + ListCtrlNonModels] box sizer on col 0. Making row 1 growable still does the right thing because the filter sits at proportion 0 inside the box sizer and only the listctrl below it (proportion 1) absorbs the row growth. Add a comment so future readers don't have to derive that.
Address Copilot follow-up on xLightsSequencer#6241: PopulateModels rebuilds both list controls and re-allocates ModelElement objects for every model not already in the sequence. Running it on every keystroke is fine on small shows but visibly stutters past ~500 models. Add a 150 ms one-shot wxTimer: - wxEVT_TEXT stores the new filter text immediately, then restarts the timer - The rebuild fires only when the user pauses, collapsing fast typing into a single rebuild - The cancel button (X) still rebuilds immediately - explicit user action shouldn't wait - AddSelectedModels also stops any pending timer so the clear-on-add rebuild isn't shadowed by a stale debounce firing right after Freeze/Thaw on both list controls is already in PopulateModels, so paint batching is already covered. Caches the filter text on every keystroke so callers that read _nonModelFilter (e.g. PopulateModels via IsFilteredOutOfNonModels) always see the current value, even before the debounce fires.
heffneil
force-pushed
the
feat/display-elements-filter
branch
from
46a019b to
a320273
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an “Available list” filter control to the Edit Display Elements dialog (Views/Models panel) to make it easier to find models/timings/groups in large shows, using an existing wxSearchCtrl pattern.
Changes:
- Inserted a
wxSearchCtrlabove the “Available” (ListCtrlNonModels) list without modifying the wxSmith.wxs-generated block. - Implemented case-insensitive substring filtering for the Available list, with a debounced
PopulateModels()rebuild. - Updated release notes to mention the new filter behavior and auto-clear-on-add behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src-ui-wx/layout/ViewsModelsPanel.h |
Adds filter UI/state members and handlers for the Available list filter. |
src-ui-wx/layout/ViewsModelsPanel.cpp |
Builds and wires the wxSearchCtrl, implements debounced filtering, and clears filter state when adding items. |
README.txt |
Adds a release note entry describing the new filter box and auto-clear behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AddSelectedModels was only stopping the filter debounce timer when _nonModelFilter was non-empty. If the user typed into the box, backspaced to empty, and then immediately added items, the one-shot timer was still pending and would fire shortly after our PopulateModels(selectModels) call, triggering a second PopulateModels() that clobbered the selection/highlight just set. Stop the timer unconditionally before the rebuild.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a filter box above the Available list in Edit Display Elements (sequencer right-click → Edit Display Elements). Large shows can have hundreds of models/groups/timings, and scrolling to find the one you want to add to a view is tedious.
Same
wxSearchCtrlpattern we already use in:so users get consistent UX across the app.
Behaviour
Implementation notes
.wxsfile does not change.ListCtrlNonModelswas atGBPosition(1, 0) span(3, 1). We detach it, drop the search ctrl at(1, 0), and re-attach the list at(2, 0) span(2, 1). The growable row stays at row 3 so the list still fills the column.AddModelToNotList/AddTimingToNotList(both gateways into the visible list), so any code path that callsPopulateModels()automatically respects the current filter without needing to know about it.IsFilteredOutOfNonModels(name)does the comparison.Test plan
wxSearchCtrlis cross-platform.PopulateModelsis called on view switch, so the filter persists across views as long as the dialog is open.