Skip to content

Filter UX: Layout filter realtime, click-out clears filter, Sequencer filter exposed#6190

Open
heffneil wants to merge 2 commits into
xLightsSequencer:masterfrom
heffneil:fix/filter-ux-improvements
Open

Filter UX: Layout filter realtime, click-out clears filter, Sequencer filter exposed#6190
heffneil wants to merge 2 commits into
xLightsSequencer:masterfrom
heffneil:fix/filter-ux-improvements

Conversation

@heffneil

@heffneil heffneil commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Two UX improvements to the existing filter boxes in the Layout and Sequencer tabs:

  1. Layout model filter — realtime, debounced
  2. Sequencer prop filter — always visible, Ctrl+F focuses, view-switch reapplies

(The earlier "clicking a filtered-out model in the preview clears the filter" change was split out and shipped separately via #6271 → master commit 0ba2cf70e. This PR no longer touches SelectModelInTree.)

1. Layout model filter narrows live as you type, debounced

ModelFilterCtrl already bound wxEVT_TEXT and was compiling the regex on every keystroke, but it only refreshed the tree when the filter became empty. Now it kicks a 150ms one-shot _filterDebounceTimer so the tree narrows once typing pauses — no full UpdateModelList(true) per keystroke, no perceived lag. Enter / search-button still refresh immediately and stop the pending timer to avoid a redundant second refresh.

2. Sequencer prop filter exposed + view-switch reapplies

The _seqFilterCtrl already filtered rows live on wxEVT_TEXT, but it was Hide()-ed by default and only reachable through Ctrl+F. Most users never found it. Pressing Enter inside it called ShowSeqFilterPanel(false) which cleared the text — the act of "committing" destroyed the filter.

Changes:

  • Removed the Hide() at construction — filter is always visible above the time display
  • OnSeqFilterEnter now just moves focus to the effect grid; the live-applied filter stays in place
  • OnSeqFilterCancel (X button, mouse) clears the text and re-shows all rows but keeps the filter visible — user can immediately type a new filter
  • Escape (keyboard) clears AND moves focus to the grid (different from X by design — keyboard user is signaling "done with filter")
  • FILTER_SEQUENCER Ctrl+F now focuses the always-visible filter and SelectAll()s any existing text
  • New public MainSequencer::ReApplyCurrentSeqFilter() — called from ViewsModelsPanel::SelectView() after DoForceSequencerRefresh() so the active filter is re-applied against the new view's rows when the user switches views
  • Removed dead ShowSeqFilterPanel(bool) method

Files changed

  • src-ui-wx/layout/LayoutPanel.cpp / .h — debounced realtime filter
  • src-ui-wx/sequencer/MainSequencer.cpp / .h — exposed sequencer filter + ReApplyCurrentSeqFilter
  • src-ui-wx/layout/ViewsModelsPanel.cpp — call ReApplyCurrentSeqFilter from SelectView

Test plan

  • Build green on macOS Debug (universal arm64 + x86_64)
  • Layout filter narrows tree on every keystroke without Enter
  • Layout filter doesn't fire UpdateModelList on every keystroke (debounce verified by typing fast and watching for one refresh)
  • Sequencer filter visible above time display by default
  • Enter inside sequencer filter keeps the filter and focuses the effect grid
  • Escape clears the text and focuses the effect grid
  • X button clears the text but leaves focus in the filter
  • Ctrl+F focuses the sequencer filter and selects existing text
  • Switching views with an active filter re-applies it against the new view's rows
  • Windows / Linux smoke test — code is pure wx + std, no platform-specific APIs

Copilot AI review requested due to automatic review settings April 21, 2026 03:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves filter UX in the Layout and Sequencer tabs by making filtering more discoverable and responsive, and by ensuring selection actions behave sensibly when items are filtered out.

Changes:

  • Layout: refresh the model tree live on every keystroke and clear the filter when clicking/selecting a model that’s currently filtered out.
  • Sequencer: keep the prop filter always visible, make Ctrl+F focus/select it, and prevent Enter/Escape/X from hiding the filter UI.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src-ui-wx/layout/LayoutPanel.cpp Makes the Layout model filter update on each keystroke; clears active filter when selecting a model not currently present due to filtering.
src-ui-wx/sequencer/MainSequencer.cpp Makes Sequencer filter always visible; updates key handling so Ctrl+F focuses it and Enter/Escape/X preserve visibility while applying/clearing filter text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src-ui-wx/sequencer/MainSequencer.cpp Outdated
Comment thread src-ui-wx/sequencer/MainSequencer.cpp Outdated
Comment thread src-ui-wx/layout/LayoutPanel.cpp Outdated
heffneil pushed a commit to heffneil/xLights that referenced this pull request Apr 21, 2026
1. Escape and X-button handlers in the sequencer filter used SetValue("")
   which fires wxEVT_TEXT, re-invoking OnSeqFilterText -> ApplySeqFilter
   a second time on top of the explicit ApplySeqFilter("") call below.
   Double work + potential visual flicker. Switched to ChangeValue("")
   so the empty filter applies exactly once via the explicit path.

2. Debounce the Layout tab's realtime filter refresh. The old
   implementation called UpdateModelList(true) on every keystroke, which
   is heavy on big layouts (preview recompute + full tree rebuild) and
   could feel laggy during fast typing. Now the keystroke handler only
   compiles the regex and restarts a 150ms one-shot wxTimer; the timer
   fires UpdateModelList once when the user pauses. Fast-typed bursts
   collapse into a single refresh. The Enter / search-button / cancel
   paths stop the timer and refresh synchronously so they feel immediate
   and don't trigger a redundant debounced refresh right after.

   150ms is below the human-perceptible lag threshold (~200ms) so it
   still feels "as you type" while eliminating the per-char work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@heffneil

Copy link
Copy Markdown
Contributor Author

Addressed all three Copilot review findings in commit 89bc9cc:

  1. Sequencer filter Escape handler (MainSequencer.cpp:331): switched SetValue("") to ChangeValue("") so wxEVT_TEXT is not fired; the explicit ApplySeqFilter("") just below now runs exactly once instead of twice.
  2. Sequencer filter Cancel handler (MainSequencer.cpp:2301): same SetValueChangeValue fix, same reasoning.
  3. Layout filter per-keystroke rebuild (LayoutPanel.cpp:714): added a 150ms one-shot wxTimer debounce. The keystroke handler now only compiles the regex and restarts the timer; UpdateModelList(true) runs once when the user pauses. The Enter / search-button / cancel paths stop the timer and refresh synchronously so they feel immediate. 150ms is below the perceptible-lag threshold so typing still feels "as you type", while fast bursts collapse into a single tree rebuild on large layouts.

Build green on macOS Debug.

@derwin12

Copy link
Copy Markdown
Contributor

I need to test this -- i didnt put in the realtime filter since the rebuild process wasnt so quick on my lowly pc.

@heffneil

Copy link
Copy Markdown
Contributor Author

I know - copilot complained about how taxing it is but it didn't seem slow. Give it a shot. Its mostly cached so I don't think its hard on a pc with modest hardware

@heffneil

Copy link
Copy Markdown
Contributor Author

@heffneil heffneil force-pushed the fix/filter-ux-improvements branch from 89bc9cc to b3b2545 Compare May 4, 2026 03:11
Copilot AI review requested due to automatic review settings May 4, 2026 03:11
@heffneil

heffneil commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

@derwin12 stripped down per your feedback. Final diff: +25 −10 across 3 files (down from +97 −22).

Rebased on current master so #6271's click-out-clears-filter fix (now upstream) dropped out automatically — no more duplicate work. Comments removed, cosmetic rebase noise reverted.

What's left is the actual behavior change:

Layout debounced realtime filtersrc-ui-wx/layout/LayoutPanel.cpp

  • L727–L735: _filterDebounceTimer setup + EVT_TEXT lambda starts a 150 ms one-shot
  • L10339, L10347: _filterDebounceTimer.Stop() in the cancel and Enter/search-button paths

Layout timer membersrc-ui-wx/layout/LayoutPanel.h

  • L636: wxTimer _filterDebounceTimer;

Sequencer always-visible filtersrc-ui-wx/sequencer/MainSequencer.cpp

  • L327–L333: Escape clears the text and returns focus to the grid (filter stays visible)
  • L335 (deleted): removed _seqFilterCtrl->Hide(); so the control is visible by default
  • L853–L856: FILTER_SEQUENCER (Ctrl+F) focuses the filter + SelectAll instead of toggling visibility
  • L2277–L2284: OnSeqFilterEnter just focuses the grid; OnSeqFilterCancel clears text + re-applies empty filter

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src-ui-wx/layout/LayoutPanel.cpp:10343

  • Using SetValue here re-emits wxEVT_TEXT, which restarts the new debounce timer even though this handler already does an immediate UpdateModelList(true). That means canceling the filter (and the SelectModelInTree path that calls this helper) schedules a second full rebuild about 150 ms later; because UpdateModelList(true) clears the tree selection, the delayed rebuild can wipe out the selection that was just restored.
void LayoutPanel::OnModelFilterCancelBtn(wxCommandEvent& event) {
    _filterDebounceTimer.Stop();
    ModelFilterCtrl->SetValue("");
    _filterString = "";
    _filterRegexValid = false;
    UpdateModelList(true);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src-ui-wx/sequencer/MainSequencer.cpp
Comment thread src-ui-wx/layout/LayoutPanel.cpp
Comment thread src-ui-wx/sequencer/MainSequencer.cpp
Comment thread src-ui-wx/sequencer/MainSequencer.cpp
@heffneil heffneil force-pushed the fix/filter-ux-improvements branch from b3b2545 to 9b92a26 Compare May 4, 2026 03:33
@heffneil

Copy link
Copy Markdown
Contributor Author

@derwin12 — friendly bump. The stripped-down version per your feedback (now +25/-10 across 3 files, down from +97/-22) has been sitting since May 4. Ready for another look whenever you have a sec.

- Layout model filter: refresh as the user types, debounced through a
  150ms one-shot timer so fast typing doesn't fire UpdateModelList on
  every keystroke. Enter / search-button still refresh immediately.
- Sequencer prop filter: leave the control visible by default instead
  of hidden behind Ctrl+F. Ctrl+F focuses the filter and selects any
  existing text. Enter just moves focus to the effect grid (the
  filter is already applied live). Escape and the X button clear the
  filter and re-show all rows but keep the control visible.
Copilot AI review requested due to automatic review settings June 21, 2026 16:05
@heffneil heffneil force-pushed the fix/filter-ux-improvements branch from 9b92a26 to a0dc0a1 Compare June 21, 2026 16:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src-ui-wx/layout/LayoutPanel.cpp:12261

  • OnModelFilterCancelBtn uses ModelFilterCtrl->SetValue(""), which fires wxEVT_TEXT. That handler restarts _filterDebounceTimer, so after the immediate UpdateModelList(true) here you can still get a second redundant refresh ~150ms later (extra work + selection/focus churn). Use ChangeValue("") to avoid generating wxEVT_TEXT when cancelling explicitly (same pattern as ViewsModelsPanel::OnNonModelsFilterCancel).
    _filterDebounceTimer.Stop();
    ModelFilterCtrl->SetValue("");
    _filterString = "";
    _filterRegexValid = false;
    UpdateModelList(true);

Comment thread src-ui-wx/sequencer/MainSequencer.cpp
@heffneil

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (it had drifted ~368 commits behind) and force-pushed. Resolves cleanly, macOS Release build passes, and all earlier review threads are resolved. CI should re-run green.

@keithsw1111 @dkulp @computergeek1507 — this small UX fix has been sitting since May. Could one of you take a look / let me know if anything's blocking it?

Recap of what it does:

  • Layout model filter now applies in real time (debounced 150ms) instead of requiring Enter; the cancel/clear button stops the pending refresh.
  • Sequencer filter box is always visible; Esc clears it and returns focus to the grid; the FILTER_SEQUENCER keybinding focuses + selects the box.

Diff is small (+36 / −24 across 5 files) and touches only the filter UI paths.

- OnModelFilterCancelBtn uses ChangeValue (not SetValue) so clearing the
  filter doesn't re-fire wxEVT_TEXT and reschedule a debounced rebuild
  that wipes the restored selection (also covers the SelectModelInTree
  clear path)
- Drop stale 'hidden by default' comment; the sequencer filter is now
  always visible

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants