feat(gui): clearer search-page buttons, and confirm before clearing history - #754
Merged
Merged
Conversation
…istory Three small changes to the Search page, all about making the two destructive actions distinguishable at a glance. "Clear" becomes "Clear Search Results". On a page that also offers to clear the search *history*, a bare "Clear" does not say which of the two it means. "Clear search history" becomes "Clear Search History" and moves out of the search-parameter row into the action row, directly after "Reset Fields". It is a one-shot command like the buttons it now sits with, not a parameter, and putting it beside "Clear Search Results" lets the two read as a pair. The divider that precedes it is inserted with it and tracked as a member, so the remember-history preference hides both together rather than leaving a stray separator and a gap in the row. Clearing the history now asks first. It deletes searchhistory.dat outright with no undo, and the button sits one position away from "Clear Search Results", which is trivially repeatable -- a misclick should not be able to silently destroy the stored terms. The prompt defaults to No. Both routes into clearing go through the confirmation: the button and the search box's right-click menu, whose item is relabelled to match. The menu route defers via CallAfter, because its handler runs while PopupMenu() is still unwinding and a modal dialog raised there misbehaves on wxOSX -- the same deferral this file already documents for the EC-reply path, for a different reason. ClearSearchHistory() itself is left unprompted so it stays usable if anything ever needs to clear without asking. Catalogs regenerated for the three changed msgids. The bare "Clear" msgid survives: it is still used elsewhere in the tree.
got3nks
added a commit
that referenced
this pull request
Aug 6, 2026
The Search tab's Name-field history held 30 entries, eMule's CCustomAutoComplete default, adopted in the #643 review rather than chosen for aMule. A history is only useful as far back as it reaches, and 30 queries is a short reach for anyone who searches often; clearing it has been a deliberate, confirmed action since #754, so there is less cost to keeping more of it around. Raised to 100 on request (#755). Eviction is LRU and stays that way: ApplySearchHistoryEntry moves a searched term to the front and drops any earlier case-insensitive copy, so what falls off the tail is the least recently searched rather than the least recently added. Terms that get reused stay near the front whatever the cap is, which is what makes the extra slots worth having -- they go to the long tail instead of hoarding stale one-offs. The constant moves from an anonymous namespace in SearchDlg.cpp to SearchHistory.h, beside the function that consumes it. That closes a gap in the tests: the one that claimed to lock "the constant CSearchDlg actually passes in" was hardcoding its own copy of the number, so it would have gone on passing while the GUI used something else entirely. It now reads the same symbol the GUI does. A longer history could have worsened the other half of #755 -- the dropdown overlapping other UI -- so the list was checked with a full 100 entries on Windows 11, Ubuntu and macOS first. None of the three runs it off the screen. Bounding the visible entries is a separate matter and not addressed here: wxWidgets exposes no dropdown-height control on plain wxComboBox (SetPopupMaxHeight is wxComboCtrl/wxOwnerDrawnComboBox only), so it needs a different widget rather than a setting.
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
1 participant
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.
Three small changes to the Search page, all about making the two destructive actions distinguishable at a glance.
Clear→Clear Search Results. On a page that also offers to clear the search history, a bare "Clear" does not say which of the two it means.Clear search history→Clear Search History, moved into the action row directly after "Reset Fields". It is a one-shot command like the buttons it now sits with, not a search parameter, and placing it beside "Clear Search Results" lets the two read as a pair. The row becomes:The divider preceding it is inserted with it and tracked as a member, so the remember-history preference hides both together instead of leaving a stray separator and a gap.
Clearing the history now asks first. It deletes
searchhistory.datoutright with no undo, and the button now sits one position from "Clear Search Results", which is trivially repeatable — a misclick should not silently destroy the stored terms. The prompt defaults to No.Both routes go through the confirmation: the button, and the search box's right-click menu, whose item is relabelled to match. The menu route defers via
CallAfterbecause its handler runs whilePopupMenu()is still unwinding, and a modal dialog raised there misbehaves on wxOSX — the same deferral this file already documents for the EC-reply path, for a different reason.ClearSearchHistory()itself stays unprompted so it remains usable if anything ever needs to clear without asking.Catalogs regenerated for the three changed msgids; the bare
"Clear"msgid survives, still used elsewhere in the tree.Verified: monolithic amule and aMuleGUI build clean on macOS; the resulting button order was checked against the sizer order, and the confirmation on both routes, the enable/disable state and the preference-driven hide of button + divider were exercised in the running GUI. clang-format 18 and both clang-tidy tiers clean on the diff, 0 compiler errors.