From 5af68f7081ad4508f56c53ac80d475f9b20373de Mon Sep 17 00:00:00 2001 From: Hakim Date: Wed, 22 Jul 2026 09:55:19 +0200 Subject: [PATCH 1/3] feat(ui): paste the selected clip with Enter and hide on Escape Enter now pastes the clip the arrow keys landed on: the list's row-activated signal routes the focused row through the same copy path as a click. Single-click activation is turned off so it doesn't double up with ClipCard's own click gesture (copy / Ctrl-pin). Escape hides the window, via a capture-phase key controller on the content so it takes precedence over the search entry's clear-on-Escape. --- src/ui/MainWindow.cpp | 37 +++++++++++++++++++++++++++++++++++++ src/ui/MainWindow.hpp | 10 ++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/ui/MainWindow.cpp b/src/ui/MainWindow.cpp index ba0ca58..b326dba 100644 --- a/src/ui/MainWindow.cpp +++ b/src/ui/MainWindow.cpp @@ -9,9 +9,12 @@ #include #include +#include #include #include +#include + #include #include @@ -174,6 +177,15 @@ void MainWindow::build_ui(GtkApplication* application) { auto* content = Gtk::make_managed(Gtk::Orientation::VERTICAL, kContentMargin); content->set_margin(kContentMargin); + // Escape hides the window from anywhere inside it. Capture phase so it is read + // before a focused child (e.g. the search entry, which would otherwise just clear + // its text) consumes it. + auto key_controller = Gtk::EventControllerKey::create(); + key_controller->set_propagation_phase(Gtk::PropagationPhase::CAPTURE); + key_controller->signal_key_pressed().connect(sigc::mem_fun(*this, &MainWindow::on_key_pressed), + false); + content->add_controller(key_controller); + search_ = Gtk::make_managed(); search_->set_placeholder_text("Search clipboard history…"); search_->signal_search_changed().connect([this] { @@ -189,6 +201,11 @@ void MainWindow::build_ui(GtkApplication* application) { scrolled->set_policy(Gtk::PolicyType::NEVER, Gtk::PolicyType::AUTOMATIC); list_ = Gtk::make_managed(); list_->set_selection_mode(Gtk::SelectionMode::NONE); + // Don't activate on a single click — that is ClipCard's own gesture (copy / + // Ctrl-pin). Keyboard activation (Enter on the focused row) still fires + // row-activated, which pastes the clip the arrow keys landed on. + list_->set_activate_on_single_click(false); + list_->signal_row_activated().connect(sigc::mem_fun(*this, &MainWindow::on_row_activated)); list_->add_css_class("background"); list_->set_valign(Gtk::Align::START); // Keep rows ordered so incrementally-added cards land in place (see rebuild_cards). @@ -315,6 +332,26 @@ void MainWindow::apply_filter() { stack_->set_visible_child(kPageEmpty); } +bool MainWindow::on_key_pressed(guint keyval, guint /*keycode*/, Gdk::ModifierType /*state*/) { + if (keyval == GDK_KEY_Escape) { + gtk_widget_set_visible(GTK_WIDGET(window_), FALSE); + return true; + } + return false; // everything else falls through (typing, arrow navigation, Enter, …) +} + +void MainWindow::on_row_activated(Gtk::ListBoxRow* row) { + auto* card = dynamic_cast(row); + if (card == nullptr) { + return; + } + // Defer to an idle for the same reason ClipCard does on click: copy() rebuilds the + // list, and tearing this row down from inside the activation would corrupt GTK's + // state accounting. Capture the entry by value so it outlives the card. + const core::ClipboardEntry entry = card->entry(); + Glib::signal_idle().connect_once([this, entry] { copy(entry); }); +} + void MainWindow::copy(const core::ClipboardEntry& entry) { // Reconstruct the clipboard payload for the entry's kind. Image bytes are // fetched lazily by hash; rich text carries its HTML alongside the plain text. diff --git a/src/ui/MainWindow.hpp b/src/ui/MainWindow.hpp index 7c19526..6a74f8b 100644 --- a/src/ui/MainWindow.hpp +++ b/src/ui/MainWindow.hpp @@ -20,11 +20,16 @@ #include +#include + #include #include +#include #include #include +#include + #include #include #include @@ -62,6 +67,11 @@ class MainWindow { void schedule_refresh(); void rebuild_cards(); void apply_filter(); + // Paste the row the keyboard navigation landed on — Enter (or double-click) + // activates it, taking the same path as a plain click on the card. + void on_row_activated(Gtk::ListBoxRow* row); + // Escape hides the window; every other key falls through. Returns true when handled. + bool on_key_pressed(guint keyval, guint keycode, Gdk::ModifierType state); void copy(const core::ClipboardEntry& entry); void pin(const std::string& content); void clear_history(); From bd55ac1bb80efa9a93c0bf67486f421dd0401260 Mon Sep 17 00:00:00 2001 From: Walkercito Date: Wed, 22 Jul 2026 10:41:34 -0400 Subject: [PATCH 2/3] fix(ui): make Enter paste while the search entry holds text Enter typed mid-search hit the entry's own Return binding instead of pasting, so the reported repro never worked. Handle the keys on a capture-phase controller at the window, ahead of the focused child, and make the list selection the cursor Enter acts on. The key mapping moves to a pure key_action() policy that unit-tests headless. Dialogs and popovers keep their own keys, a focused button keeps Enter, copy() guards against the double activation ClipCard could already emit, and hide() clears the filter on every exit path. Closes #7 --- src/ui/CMakeLists.txt | 1 + src/ui/KeyAction.cpp | 30 ++++++ src/ui/KeyAction.hpp | 33 +++++++ src/ui/MainWindow.cpp | 194 +++++++++++++++++++++++++++++-------- src/ui/MainWindow.hpp | 23 ++++- tests/ui/CMakeLists.txt | 1 + tests/ui/KeyActionTest.cpp | 78 +++++++++++++++ 7 files changed, 317 insertions(+), 43 deletions(-) create mode 100644 src/ui/KeyAction.cpp create mode 100644 src/ui/KeyAction.hpp create mode 100644 tests/ui/KeyActionTest.cpp diff --git a/src/ui/CMakeLists.txt b/src/ui/CMakeLists.txt index 3516922..3dd9375 100644 --- a/src/ui/CMakeLists.txt +++ b/src/ui/CMakeLists.txt @@ -30,6 +30,7 @@ add_library( StatusNotifierItem.cpp KeystrokePaster.cpp CopyAction.cpp + KeyAction.cpp widgets/ClipCard.cpp widgets/ShortcutChooser.cpp dialogs/SettingsDialog.cpp diff --git a/src/ui/KeyAction.cpp b/src/ui/KeyAction.cpp new file mode 100644 index 0000000..e1ee161 --- /dev/null +++ b/src/ui/KeyAction.cpp @@ -0,0 +1,30 @@ +#include "ui/KeyAction.hpp" + +#include + +namespace copyclip::ui { + +KeyAction key_action(unsigned int keyval, const KeyContext& context) { + switch (keyval) { + case GDK_KEY_Escape: + // First Escape drops an active search, a second dismisses — so narrowing a + // search is undoable without losing the window. + return context.search_active ? KeyAction::ClearSearch : KeyAction::Dismiss; + case GDK_KEY_Up: + case GDK_KEY_KP_Up: + return KeyAction::SelectPrevious; + case GDK_KEY_Down: + case GDK_KEY_KP_Down: + return KeyAction::SelectNext; + case GDK_KEY_Return: + case GDK_KEY_KP_Enter: + case GDK_KEY_ISO_Enter: + // A focused button owns Enter: the header's Settings/Clear and a card's + // expand toggle must activate what the user tabbed to, not paste. + return context.button_focused ? KeyAction::None : KeyAction::Paste; + default: + return KeyAction::None; // typing, Tab, everything else keeps its stock behavior + } +} + +} // namespace copyclip::ui diff --git a/src/ui/KeyAction.hpp b/src/ui/KeyAction.hpp new file mode 100644 index 0000000..173e3ea --- /dev/null +++ b/src/ui/KeyAction.hpp @@ -0,0 +1,33 @@ +#pragma once + +// The main window's keyboard policy, as a pure decision: which action a key press +// asks for, given what the window currently holds. No widgets are touched here, so +// the policy unit-tests headless while MainWindow keeps only the dispatch. + +#include + +namespace copyclip::ui { + +// What a key press asks the main window to do. +enum class KeyAction : std::uint8_t { + None, // not ours — let the focused widget have it + ClearSearch, // drop the active filter, keeping the window up + Dismiss, // hide the window without pasting + SelectPrevious, // move the highlight to the previous match + SelectNext, // move the highlight to the next match + Paste, // paste the highlighted clip +}; + +// The window state the policy depends on. Grouped in a struct so the call site +// reads as named fields rather than a row of anonymous bools. +struct KeyContext { + bool search_active = false; // the search entry holds text + bool button_focused = false; // a button has focus, so it owns Enter +}; + +// Map a GDK keyval (GDK_KEY_*) to the action it asks for. Modifiers are +// deliberately ignored: Ctrl+Enter and the like read as their plain form, since +// none of them mean anything else in this window. +[[nodiscard]] KeyAction key_action(unsigned int keyval, const KeyContext& context); + +} // namespace copyclip::ui diff --git a/src/ui/MainWindow.cpp b/src/ui/MainWindow.cpp index b326dba..8f03377 100644 --- a/src/ui/MainWindow.cpp +++ b/src/ui/MainWindow.cpp @@ -4,17 +4,17 @@ #include "ui/ClipText.hpp" #include "ui/Constants.hpp" #include "ui/Fuzzy.hpp" +#include "ui/KeyAction.hpp" #include "ui/Theme.hpp" #include "ui/widgets/ClipCard.hpp" +#include #include #include #include #include #include -#include - #include #include @@ -80,6 +80,34 @@ void trim_heap() { return 0; } +// True when a button holds the focus (see KeyAction for why that matters). +[[nodiscard]] bool button_focused(GtkWindow* window) { + GtkWidget* focus = gtk_window_get_focus(window); + return focus != nullptr && GTK_IS_BUTTON(focus) != FALSE; +} + +// True while focus sits inside a popover — the search entry's right-click Cut/Copy/ +// Paste menu, or the emoji chooser. Those are not dialogs, so the dialog guard misses +// them, and they own their own Escape and arrow keys. +[[nodiscard]] bool popover_focused(GtkWindow* window) { + GtkWidget* focus = gtk_window_get_focus(window); + return focus != nullptr && gtk_widget_get_ancestor(focus, GTK_TYPE_POPOVER) != nullptr; +} + +// The nearest row before/after `from` in display order that the search filter left +// visible, or nullptr at the end of the list. +[[nodiscard]] Gtk::ListBoxRow* visible_sibling(Gtk::Widget& from, bool forward) { + for (Gtk::Widget* sibling = forward ? from.get_next_sibling() : from.get_prev_sibling(); + sibling != nullptr; + sibling = forward ? sibling->get_next_sibling() : sibling->get_prev_sibling()) { + auto* row = dynamic_cast(sibling); + if (row != nullptr && row->get_visible()) { + return row; + } + } + return nullptr; +} + } // namespace MainWindow::MainWindow(GtkApplication* application, core::HistoryService& history, @@ -109,11 +137,23 @@ void MainWindow::build_ui(GtkApplication* application) { // Closing hides the window so the app keeps capturing in the background. g_signal_connect(window_, "close-request", - G_CALLBACK(+[](GtkWindow* window, gpointer) -> gboolean { - gtk_widget_set_visible(GTK_WIDGET(window), FALSE); + G_CALLBACK(+[](GtkWindow*, gpointer self) -> gboolean { + static_cast(self)->hide(); return TRUE; }), - nullptr); + this); + + // Escape, Up/Down and Enter are the window's own, wherever focus sits — capture + // phase so they are read before the focused child's stock bindings (the search + // entry would otherwise swallow Enter, which is the whole bug). + auto key_controller = Gtk::EventControllerKey::create(); + key_controller->set_propagation_phase(Gtk::PropagationPhase::CAPTURE); + key_controller->signal_key_pressed().connect(sigc::mem_fun(*this, &MainWindow::on_key_pressed), + false); + // add_controller takes ownership of a ref, so gobj_copy() mints it — unlike the + // plain gobj() handoff used for child widgets below, which would double-unref here. + gtk_widget_add_controller(GTK_WIDGET(window_), + GTK_EVENT_CONTROLLER(key_controller->gobj_copy())); // Trim on every hide — close, copy, and toggle all route through this signal. g_signal_connect(window_, "hide", G_CALLBACK(+[](GtkWidget*, gpointer) { trim_heap(); }), @@ -177,15 +217,6 @@ void MainWindow::build_ui(GtkApplication* application) { auto* content = Gtk::make_managed(Gtk::Orientation::VERTICAL, kContentMargin); content->set_margin(kContentMargin); - // Escape hides the window from anywhere inside it. Capture phase so it is read - // before a focused child (e.g. the search entry, which would otherwise just clear - // its text) consumes it. - auto key_controller = Gtk::EventControllerKey::create(); - key_controller->set_propagation_phase(Gtk::PropagationPhase::CAPTURE); - key_controller->signal_key_pressed().connect(sigc::mem_fun(*this, &MainWindow::on_key_pressed), - false); - content->add_controller(key_controller); - search_ = Gtk::make_managed(); search_->set_placeholder_text("Search clipboard history…"); search_->signal_search_changed().connect([this] { @@ -197,22 +228,20 @@ void MainWindow::build_ui(GtkApplication* application) { stack_ = Gtk::make_managed(); stack_->set_vexpand(true); - auto* scrolled = Gtk::make_managed(); - scrolled->set_policy(Gtk::PolicyType::NEVER, Gtk::PolicyType::AUTOMATIC); + scrolled_ = Gtk::make_managed(); + scrolled_->set_policy(Gtk::PolicyType::NEVER, Gtk::PolicyType::AUTOMATIC); list_ = Gtk::make_managed(); - list_->set_selection_mode(Gtk::SelectionMode::NONE); - // Don't activate on a single click — that is ClipCard's own gesture (copy / - // Ctrl-pin). Keyboard activation (Enter on the focused row) still fires - // row-activated, which pastes the clip the arrow keys landed on. - list_->set_activate_on_single_click(false); - list_->signal_row_activated().connect(sigc::mem_fun(*this, &MainWindow::on_row_activated)); + // Single selection is the keyboard cursor: apply_filter keeps exactly one match + // highlighted, the arrow keys move it, and Enter pastes it. Row activation is + // left alone — the mouse is ClipCard's own gesture (copy / Ctrl-pin). + list_->set_selection_mode(Gtk::SelectionMode::SINGLE); list_->add_css_class("background"); list_->set_valign(Gtk::Align::START); // Keep rows ordered so incrementally-added cards land in place (see rebuild_cards). list_->set_sort_func( [](Gtk::ListBoxRow* a, Gtk::ListBoxRow* b) { return clip_card_sort(a, b); }); - scrolled->set_child(*list_); - stack_->add(*scrolled, kPageList); + scrolled_->set_child(*list_); + stack_->add(*scrolled_, kPageList); auto* empty = Gtk::make_managed(Gtk::Orientation::VERTICAL, kContentMargin); empty->set_valign(Gtk::Align::CENTER); @@ -304,6 +333,9 @@ void MainWindow::apply_filter() { // The search bar is only useful once there is something to search. search_->set_visible(card_count_ > 0); + const Gtk::ListBoxRow* selected = list_->get_selected_row(); + ClipCard* first_shown = nullptr; + bool selection_shown = false; std::size_t visible = 0; for (Gtk::Widget* child = list_->get_first_child(); child != nullptr; child = child->get_next_sibling()) { @@ -313,8 +345,26 @@ void MainWindow::apply_filter() { } const bool shown = matches(card->content()); card->set_visible(shown); - if (shown) { - ++visible; + if (!shown) { + continue; + } + ++visible; + if (first_shown == nullptr) { + first_shown = card; + } + selection_shown = selection_shown || card == selected; + } + + // Always leave a row highlighted for Enter to act on. When the filter — or a + // rebuild that dropped the card — took the selection away, fall back to the + // first match and scroll back up to it: hidden rows get no allocation, so that + // row sits at the very top of the list. + if (!selection_shown) { + if (first_shown != nullptr) { + list_->select_row(*first_shown); + scrolled_->get_vadjustment()->set_value(0.0); + } else { + list_->unselect_all(); } } @@ -333,26 +383,83 @@ void MainWindow::apply_filter() { } bool MainWindow::on_key_pressed(guint keyval, guint /*keycode*/, Gdk::ModifierType /*state*/) { - if (keyval == GDK_KEY_Escape) { - gtk_widget_set_visible(GTK_WIDGET(window_), FALSE); + // Settings and first-run are AdwDialogs presented inside this very window, and + // menus are popovers in it — while either is up, its own keys must win. + if (adw_application_window_get_visible_dialog(window_) != nullptr || + popover_focused(GTK_WINDOW(window_))) { + return false; + } + // Read the entry, not search_text_: GtkSearchEntry delays search-changed by + // ~150 ms, so the cached copy lags behind text the user just typed — Escape + // right after the first keystroke must still read as an active search. + // ponytail: an in-progress input-method preedit is not detected, so Enter and + // the arrows preempt CJK candidate selection. Filter through Gtk::IMContext if + // that ever matters. + const KeyContext context{.search_active = !search_->get_text().empty(), + .button_focused = button_focused(GTK_WINDOW(window_))}; + switch (key_action(keyval, context)) { + case KeyAction::ClearSearch: + search_->set_text(""); return true; + case KeyAction::Dismiss: + hide(); + return true; + case KeyAction::SelectPrevious: + move_selection(false); + return true; + case KeyAction::SelectNext: + move_selection(true); + return true; + case KeyAction::Paste: + return activate_selection(); + case KeyAction::None: + return false; } - return false; // everything else falls through (typing, arrow navigation, Enter, …) + // No default case, so a new KeyAction trips -Wswitch rather than being ignored. + return false; } -void MainWindow::on_row_activated(Gtk::ListBoxRow* row) { - auto* card = dynamic_cast(row); +void MainWindow::move_selection(bool forward) { + Gtk::ListBoxRow* const current = list_->get_selected_row(); + if (current == nullptr) { + return; // nothing matches the search + } + // Stop at the ends rather than wrapping: with the list also acting as the + // paste target, wrapping past the last row invites pasting the wrong clip. + if (Gtk::ListBoxRow* const next = visible_sibling(*current, forward); next != nullptr) { + list_->select_row(*next); + reveal(*next); + } +} + +bool MainWindow::activate_selection() { + auto* card = dynamic_cast(list_->get_selected_row()); if (card == nullptr) { - return; + return false; + } + // Straight through, unlike ClipCard's click: the rebuild copy() sets off is + // itself deferred (see schedule_refresh), so no widget dies under this dispatch. + copy(card->entry()); + return true; +} + +void MainWindow::reveal(Gtk::ListBoxRow& row) { + double row_x = 0.0; + double row_y = 0.0; + if (row.translate_coordinates(*list_, 0.0, 0.0, row_x, row_y)) { + // The list is what the viewport scrolls, so list coordinates are the + // adjustment's own. + scrolled_->get_vadjustment()->clamp_page(row_y, row_y + row.get_height()); } - // Defer to an idle for the same reason ClipCard does on click: copy() rebuilds the - // list, and tearing this row down from inside the activation would corrupt GTK's - // state accounting. Capture the entry by value so it outlives the card. - const core::ClipboardEntry entry = card->entry(); - Glib::signal_idle().connect_once([this, entry] { copy(entry); }); } void MainWindow::copy(const core::ClipboardEntry& entry) { + // ClipCard defers its click to an idle, so a fast double click can queue two + // copies of the same clip — which with auto-paste on injects Ctrl+V twice into + // the target app. The first one hid the window, so that is the signal to stop. + if (gtk_widget_get_visible(GTK_WIDGET(window_)) == FALSE) { + return; + } // Reconstruct the clipboard payload for the entry's kind. Image bytes are // fetched lazily by hash; rich text carries its HTML alongside the plain text. core::ClipContent content; @@ -367,10 +474,19 @@ void MainWindow::copy(const core::ClipboardEntry& entry) { } // CopyAction handles clipboard + history + auto-paste; the window just hides. if (copy_action_.run(content)) { - gtk_widget_set_visible(GTK_WIDGET(window_), FALSE); + hide(); } } +void MainWindow::hide() { + // Drop the filter on the way out, whichever path got here — pasting, clicking, + // closing or toggling. The window is a popup: reopening it on someone's old + // query (which grab_focus leaves unselected, so typing appends to it) is never + // what was meant. + search_->set_text(""); + gtk_widget_set_visible(GTK_WIDGET(window_), FALSE); +} + void MainWindow::pin(const std::string& content) { history_.get().toggle_pin(content); } @@ -407,7 +523,7 @@ GtkWidget* MainWindow::native() const { void MainWindow::toggle() { if (gtk_widget_get_visible(GTK_WIDGET(window_)) != FALSE) { - gtk_widget_set_visible(GTK_WIDGET(window_), FALSE); + hide(); return; } present(); diff --git a/src/ui/MainWindow.hpp b/src/ui/MainWindow.hpp index 6a74f8b..2d42f19 100644 --- a/src/ui/MainWindow.hpp +++ b/src/ui/MainWindow.hpp @@ -9,6 +9,11 @@ // new clip touches one widget, not all N. Search toggles each card's visibility in // place. Refreshes are deferred to an idle so a card may safely trigger one from // inside its own click handler. +// +// The window is keyboard-first: focus stays in the search entry, Up/Down move the +// highlighted row through the filtered list, Enter pastes it, and Escape drops the +// search before dismissing. A window-level key controller owns that policy, so the +// keys work wherever focus happens to sit. #include "core/HistoryService.hpp" #include "core/Interfaces.hpp" @@ -25,6 +30,7 @@ #include #include #include +#include #include #include @@ -67,12 +73,20 @@ class MainWindow { void schedule_refresh(); void rebuild_cards(); void apply_filter(); - // Paste the row the keyboard navigation landed on — Enter (or double-click) - // activates it, taking the same path as a plain click on the card. - void on_row_activated(Gtk::ListBoxRow* row); - // Escape hides the window; every other key falls through. Returns true when handled. + // The window's keyboard policy — Escape, Up/Down and Enter. Returns true when + // the key was consumed; everything else falls through to the focused widget. bool on_key_pressed(guint keyval, guint keycode, Gdk::ModifierType state); + // Move the highlighted row to the next/previous match, and scroll it into view. + void move_selection(bool forward); + // Paste the highlighted clip. False when there is nothing highlighted, so the + // key keeps its stock behavior. + bool activate_selection(); + // Scroll the list so `row` is visible — moving the selection from the search + // entry never moves focus, so GTK won't scroll for us. + void reveal(Gtk::ListBoxRow& row); void copy(const core::ClipboardEntry& entry); + // Hide the window; the app keeps capturing in the background. + void hide(); void pin(const std::string& content); void clear_history(); void open_settings(); @@ -87,6 +101,7 @@ class MainWindow { std::string search_text_; AdwApplicationWindow* window_ = nullptr; Gtk::Stack* stack_ = nullptr; + Gtk::ScrolledWindow* scrolled_ = nullptr; // holds list_; its vadjustment scrolls it Gtk::ListBox* list_ = nullptr; // Live cards keyed by their clip content, for incremental reconciliation. The // cards are owned by `list_`; these are non-owning observers kept in sync with diff --git a/tests/ui/CMakeLists.txt b/tests/ui/CMakeLists.txt index af314a7..a4d7a83 100644 --- a/tests/ui/CMakeLists.txt +++ b/tests/ui/CMakeLists.txt @@ -2,3 +2,4 @@ copyclip_add_test(clip_text_test SOURCES ClipTextTest.cpp LIBS copyclip::ui_text copyclip_add_test(shortcut_text_test SOURCES ShortcutTextTest.cpp LIBS copyclip::ui_text) copyclip_add_test(fuzzy_test SOURCES FuzzyTest.cpp LIBS copyclip::ui_text) copyclip_add_test(copy_action_test SOURCES CopyActionTest.cpp LIBS copyclip::ui) +copyclip_add_test(key_action_test SOURCES KeyActionTest.cpp LIBS copyclip::ui) diff --git a/tests/ui/KeyActionTest.cpp b/tests/ui/KeyActionTest.cpp new file mode 100644 index 0000000..be69b82 --- /dev/null +++ b/tests/ui/KeyActionTest.cpp @@ -0,0 +1,78 @@ +#include "ui/KeyAction.hpp" + +#include + +#include + +namespace { + +using copyclip::ui::key_action; +using copyclip::ui::KeyAction; +using copyclip::ui::KeyContext; + +// The window as it sits when it opens: nothing typed, focus in the search entry. +constexpr KeyContext kFresh{.search_active = false, .button_focused = false}; +// Mid-search: text sits in the filter. +constexpr KeyContext kSearching{.search_active = true, .button_focused = false}; +// Tabbed onto a button (header Settings/Clear, or a card's expand toggle). +constexpr KeyContext kOnButton{.search_active = false, .button_focused = true}; + +// Type to filter, press Enter — it must paste, not fall through to the search +// entry's own Return binding (issue #7). +TEST(KeyActionTest, EnterPastesWhileSearching) { + EXPECT_EQ(key_action(GDK_KEY_Return, kSearching), KeyAction::Paste); +} + +TEST(KeyActionTest, EnterPastesFromEveryReturnKey) { + EXPECT_EQ(key_action(GDK_KEY_Return, kFresh), KeyAction::Paste); + EXPECT_EQ(key_action(GDK_KEY_KP_Enter, kFresh), KeyAction::Paste); + EXPECT_EQ(key_action(GDK_KEY_ISO_Enter, kFresh), KeyAction::Paste); +} + +// A focused button keeps Enter, or tabbing to Settings would paste instead. +TEST(KeyActionTest, EnterOnAButtonIsNotOurs) { + EXPECT_EQ(key_action(GDK_KEY_Return, kOnButton), KeyAction::None); +} + +// First Escape clears the filter, a second dismisses — narrowing is undoable +// without losing the window. (hide() separately clears the filter on every path.) +TEST(KeyActionTest, EscapeClearsSearchBeforeDismissing) { + EXPECT_EQ(key_action(GDK_KEY_Escape, kSearching), KeyAction::ClearSearch); + EXPECT_EQ(key_action(GDK_KEY_Escape, kFresh), KeyAction::Dismiss); +} + +// Escape dismisses from a button too — unlike Enter, no widget owns it. +TEST(KeyActionTest, EscapeIsOursEvenOnAButton) { + EXPECT_EQ(key_action(GDK_KEY_Escape, kOnButton), KeyAction::Dismiss); +} + +// The one state where the two rules could collide: mid-search with a button +// focused. Enter is still the button's, Escape still clears. +TEST(KeyActionTest, SearchingWithAButtonFocusedKeepsBothRules) { + constexpr KeyContext both{.search_active = true, .button_focused = true}; + EXPECT_EQ(key_action(GDK_KEY_Return, both), KeyAction::None); + EXPECT_EQ(key_action(GDK_KEY_Escape, both), KeyAction::ClearSearch); +} + +TEST(KeyActionTest, ArrowsMoveTheSelection) { + EXPECT_EQ(key_action(GDK_KEY_Up, kFresh), KeyAction::SelectPrevious); + EXPECT_EQ(key_action(GDK_KEY_KP_Up, kFresh), KeyAction::SelectPrevious); + EXPECT_EQ(key_action(GDK_KEY_Down, kFresh), KeyAction::SelectNext); + EXPECT_EQ(key_action(GDK_KEY_KP_Down, kFresh), KeyAction::SelectNext); +} + +// The arrows stay ours mid-search: that is the whole point of navigating the +// results without leaving the search entry. +TEST(KeyActionTest, ArrowsMoveTheSelectionWhileSearching) { + EXPECT_EQ(key_action(GDK_KEY_Down, kSearching), KeyAction::SelectNext); +} + +// Typing must reach the search entry untouched, or the filter stops working. +TEST(KeyActionTest, OtherKeysFallThrough) { + EXPECT_EQ(key_action(GDK_KEY_a, kSearching), KeyAction::None); + EXPECT_EQ(key_action(GDK_KEY_space, kSearching), KeyAction::None); + EXPECT_EQ(key_action(GDK_KEY_BackSpace, kSearching), KeyAction::None); + EXPECT_EQ(key_action(GDK_KEY_Tab, kFresh), KeyAction::None); +} + +} // namespace From c79e888d5677c0902f08c12c976226b7fd7efaf1 Mon Sep 17 00:00:00 2001 From: Walkercito Date: Wed, 22 Jul 2026 11:03:45 -0400 Subject: [PATCH 3/3] fix(ui): keep the keyboard cursor put across a pin Pinning recreates the card the cursor sits on, so the selection died with it and apply_filter snapped the cursor to the top of the list. Remember the selected content across the rebuild and put the cursor back on it. Ctrl+click reaches GtkListBox as a selection toggle and can leave nothing selected, which left the arrows dead until the next filter pass; they now start at the top when there is no cursor to move. --- src/ui/MainWindow.cpp | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/ui/MainWindow.cpp b/src/ui/MainWindow.cpp index 8f03377..2a5a395 100644 --- a/src/ui/MainWindow.cpp +++ b/src/ui/MainWindow.cpp @@ -94,13 +94,12 @@ void trim_heap() { return focus != nullptr && gtk_widget_get_ancestor(focus, GTK_TYPE_POPOVER) != nullptr; } -// The nearest row before/after `from` in display order that the search filter left -// visible, or nullptr at the end of the list. -[[nodiscard]] Gtk::ListBoxRow* visible_sibling(Gtk::Widget& from, bool forward) { - for (Gtk::Widget* sibling = forward ? from.get_next_sibling() : from.get_prev_sibling(); - sibling != nullptr; - sibling = forward ? sibling->get_next_sibling() : sibling->get_prev_sibling()) { - auto* row = dynamic_cast(sibling); +// The first row at or after `start`, scanning in display order or against it, that +// the search filter left visible — or nullptr once the scan runs off the list. +[[nodiscard]] Gtk::ListBoxRow* visible_row_from(Gtk::Widget* start, bool forward) { + for (Gtk::Widget* candidate = start; candidate != nullptr; + candidate = forward ? candidate->get_next_sibling() : candidate->get_prev_sibling()) { + auto* row = dynamic_cast(candidate); if (row != nullptr && row->get_visible()) { return row; } @@ -278,6 +277,12 @@ void MainWindow::rebuild_cards() { const std::vector entries = history_.get().entries(); card_count_ = entries.size(); + // Pinning recreates the very card the cursor sits on, taking the selection with + // it. Remember what it held so the cursor can be put back below, instead of + // apply_filter snapping it to the top of the list. + const auto* selected = dynamic_cast(list_->get_selected_row()); + const std::string selected_content = selected != nullptr ? selected->entry().content : ""; + // Index the entries we want shown, by their key, for O(1) lookup below. std::map wanted; for (const core::ClipboardEntry& entry : entries) { @@ -325,6 +330,12 @@ void MainWindow::rebuild_cards() { cards_.emplace(entry.content, card); } + // Put the cursor back on the card it was on, recreated or not. Gone for good — + // cleared, evicted, filtered out — leaves apply_filter to pick the fallback. + if (const auto card = cards_.find(selected_content); card != cards_.end()) { + list_->select_row(*card->second); + } + list_->invalidate_sort(); apply_filter(); } @@ -420,13 +431,16 @@ bool MainWindow::on_key_pressed(guint keyval, guint /*keycode*/, Gdk::ModifierTy } void MainWindow::move_selection(bool forward) { - Gtk::ListBoxRow* const current = list_->get_selected_row(); - if (current == nullptr) { - return; // nothing matches the search + // Ctrl+click reaches GtkListBox as a toggle and leaves the list unselected, so + // the cursor can be missing even with matches on screen. Start the scan at the + // top then, rather than letting the arrows go dead. + Gtk::Widget* start = list_->get_first_child(); + if (Gtk::ListBoxRow* const current = list_->get_selected_row(); current != nullptr) { + start = forward ? current->get_next_sibling() : current->get_prev_sibling(); } // Stop at the ends rather than wrapping: with the list also acting as the // paste target, wrapping past the last row invites pasting the wrong clip. - if (Gtk::ListBoxRow* const next = visible_sibling(*current, forward); next != nullptr) { + if (Gtk::ListBoxRow* const next = visible_row_from(start, forward); next != nullptr) { list_->select_row(*next); reveal(*next); }