Skip to content

Vendor catalog search: Search button, native selection highlight#6270

Closed
heffneil wants to merge 5 commits into
xLightsSequencer:masterfrom
heffneil:fix/vendor-catalog-search-visibility
Closed

Vendor catalog search: Search button, native selection highlight#6270
heffneil wants to merge 5 commits into
xLightsSequencer:masterfrom
heffneil:fix/vendor-catalog-search-visibility

Conversation

@heffneil

@heffneil heffneil commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Vendor catalog dialog: add a visible Search button to the right of the search field; the in-field magnifier icon was easy to miss, especially on macOS. Bind wxEVT_TEXT_ENTER so pressing Enter also advances to the next match.
  • Vendor catalog dialog: when Search finds a match, make that item the active selection (instead of just bold text) and call SetFocus() on the tree so it renders with the system focused-selection colours (blue+white) rather than the muted unfocused colours. Successive Search presses remove the prior find from the selection so matches don't pile up; manual Ctrl+click selections survive.
  • Vendor catalog dialog: reconcile VendorModelDialog.wxs with the cpp guards — the .wxs had been left out of sync after Fix vendor model search on Windows (#6252) #6267 swapped TextCtrl_Search to a wxSearchCtrl.

(The layout-panel filter auto-clear is split out into #6271.)

Test plan

  • Open Import → Download Models, type a search term — Search button visible to right of the field
  • Press Search (or Enter, or the in-field magnifier) — found item is the active selection, scrolled into view, blue+white styling
  • Press Search again — previous highlight cleared, next match becomes the selection
  • Ctrl+click extra items in tree — manual selections preserved across Search presses
  • Cancel (X) — clears highlight

🤖 Generated with Claude Code

The vendor catalog had only a wxSearchCtrl with the small in-field
magnifier icon to advance through matches, which was easy to miss.
Add a visible Next button to the right of the search field and
highlight the found item with a yellow background (in addition to
the existing bold) so users can clearly see which result is current.
Also reconciles VendorModelDialog.wxs with the cpp guards (the .wxs
had been left out of sync after xLightsSequencer#6267 swapped TextCtrl_Search to a
wxSearchCtrl).
Copilot AI review requested due to automatic review settings April 30, 2026 13:58

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

Updates the vendor catalog “Import → Download Models” search UI/behavior by making the search action more discoverable and adding clearer visual indication of the current match, while bringing the wxSmith .wxs back in sync with the generated C++ dialog code.

Changes:

  • Replaces the search field with a wxSearchCtrl and wires its search/cancel buttons to existing handlers.
  • Adds a visible “Next” button next to the search field to advance through matches.
  • Highlights the current match with a yellow background (and clears it on text change/cancel/manual selection).

Reviewed changes

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

File Description
src-ui-wx/wxsmith/VendorModelDialog.wxs Updates wxSmith layout: wxSearchCtrl + event handlers, and changes search button label to “Next”.
src-ui-wx/import_export/VendorModelDialog.h Adds missing Button_Search member and ID_BUTTON4 identifier to match the dialog layout.
src-ui-wx/import_export/VendorModelDialog.cpp Implements the new button/sizer layout, binds search/cancel events, and applies/clears background highlight on matched items.

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

Comment thread src-ui-wx/wxsmith/VendorModelDialog.wxs
Comment thread src-ui-wx/import_export/VendorModelDialog.cpp
- Search/Next replaces label, removes prior search find from selection
  before adding the new one (preserves manual Ctrl+click selections)
- Drops the wxLIGHT_GREY background; relies on the tree's native
  selection rendering instead
- Calls SetFocus on the tree after the search so the selection
  renders in the focused blue+white style rather than the muted
  unfocused grey
- Snapshots _lastSearchItem before reassignment so the prior find
  isn't accidentally re-added to the multi-selection across
  successive Search presses
@heffneil

Copy link
Copy Markdown
Contributor Author

@derwin12 can you double check the windows. I ran it in a VM and it works as I expect but this one has been finicky along with the other one I just can't get that other one operational in windows

Copilot AI review requested due to automatic review settings April 30, 2026 19:51
@heffneil heffneil changed the title Add Next button and yellow highlight for vendor catalog search Vendor catalog search: Search button, native selection highlight, layout filter auto-clear Apr 30, 2026

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 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.

Comment thread src-ui-wx/import_export/VendorModelDialog.cpp
Comment thread src-ui-wx/import_export/VendorModelDialog.cpp
Comment thread src-ui-wx/wxsmith/VendorModelDialog.wxs
Comment thread src-ui-wx/import_export/VendorModelDialog.cpp
Add wxEVT_TEXT_ENTER binding on TextCtrl_Search so pressing Enter in
the search field advances to the next match. Mirror the binding in
VendorModelDialog.wxs. (Copilot review.)
@heffneil heffneil force-pushed the fix/vendor-catalog-search-visibility branch from 59e5c42 to f1f598b Compare April 30, 2026 20:01
@heffneil heffneil changed the title Vendor catalog search: Search button, native selection highlight, layout filter auto-clear Vendor catalog search: Search button, native selection highlight Apr 30, 2026
…g during search

- Update wxs tree style from wxTR_SINGLE to wxTR_MULTIPLE so it matches
  the cpp construction (mismatch was introduced by xLightsSequencer#6073 / xLightsSequencer#6267 not
  updating the .wxs)
- Add _searchInProgress flag and gate the SEL_CHANGED handler's
  _lastSearchItem clearing on it. Without the flag, the re-add loop
  in OnButton_SearchClick fired SEL_CHANGED for each restored prior
  selection and the handler then wiped out the bold and _lastSearchItem
  we just set.
Copilot AI review requested due to automatic review settings May 1, 2026 00:23

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 1 comment.

Comments suppressed due to low confidence (1)

src-ui-wx/import_export/VendorModelDialog.cpp:1482

  • During the selection-restore loop in OnButton_SearchClick, multiple SelectItem/UnselectItem calls fire wxEVT_TREE_SEL_CHANGED events. Even with the _searchInProgress guard, this handler still calls UpdatePanelForItem() for each intermediate selection change, which can cause unnecessary UI work/flicker and potentially leave the details panel briefly showing the wrong item. Since OnButton_SearchClick explicitly calls UpdatePanelForItem(current) at the end, consider skipping UpdatePanelForItem entirely while _searchInProgress is true.
void VendorModelDialog::OnTreeCtrl_NavigatorSelectionChanged(wxTreeEvent& event)
{
    wxTreeItemId startid = event.GetItem();
    if (!startid.IsOk()) {
        startid = GetFocusedItem();
    }
    // User manually selected a different item — clear the search bold/cursor.
    // The _searchInProgress guard prevents the re-add loop in OnButton_SearchClick
    // (which fires SEL_CHANGED for each prior selection it restores) from clearing
    // _lastSearchItem we just set.
    if (!_searchInProgress && _lastSearchItem.IsOk() && startid != _lastSearchItem) {
        TreeCtrl_Navigator->SetItemBold(_lastSearchItem, false);
        _lastSearchItem = wxTreeItemId();
    }
    UpdatePanelForItem(startid);
}

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

Comment thread src-ui-wx/import_export/VendorModelDialog.cpp
The Search highlight was a combination of bold + native tree selection
(SelectItem). The three clear paths (search-text changed, cancel button,
manual selection of a different item) were only undoing the bold half
which left the row visually selected even after the search was cleared.

Add UnselectItem alongside SetItemBold(false) in all three paths so
clearing the search fully removes the visual highlight. UnselectItem is
a no-op if the item is not selected, so the only behavioral difference
is the contrived case where the user explicitly Ctrl-clicked to add the
search-highlighted row to a manual multi-select — clearing the search
will now drop that one row from the selection. Acceptable trade for
fixing the visible "selection is stuck" regression.

Addresses Copilot review on PR xLightsSequencer#6270.

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

Copy link
Copy Markdown
Contributor Author

Addressed the Copilot thread in 5a4a119. Went with the "keep the native-selection highlight, just clean up properly" approach (Option A) rather than reverting to bold-only:

  • Added TreeCtrl_Navigator->UnselectItem(_lastSearchItem) alongside the existing SetItemBold(false) in all three clear paths: OnTextCtrl_SearchText, OnSearchCancelClick, and the selection-changed handler in OnTreeCtrl_NavigatorSelectionChanged.
  • UnselectItem is a no-op if the row isn't selected, so this is harmless in the common case (plain click-elsewhere → tree natively deselected the old row already).
  • The only behavioral edge: if the user explicitly Ctrl-clicked the search-highlighted row to keep it in a manual multi-select, clearing the search will now drop that one row. Recoverable in one click and avoids the much more visible "selection is stuck after clearing search" bug.

@heffneil

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #6256.

#6270 added a find-next search (jump to the next matching model in the catalog tree and highlight it). #6256 takes the better approach for browsing a large catalog: a hierarchy-aware live filter that narrows the tree to only matching vendors/categories/models as you type. With the filter, there's nothing to jump-to/highlight, so the selection-focus machinery here isn't needed.

Going with the filter (#6256) instead. Thanks!

@heffneil heffneil closed this Jun 18, 2026
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.

2 participants