From be2c16eca523b9517db860628f7a894e0680945f Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:45:42 +0200 Subject: [PATCH] [UI] Fix auto-completion acceptance on Space/Tab/Enter (#7466) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [UI] Fix auto-completion acceptance on Space/Tab/Enter * [UI] Accept current completion when no item selected in popup * [UI] Also accept completion on focus out when none selected * [UI] Keep focus in chat on Tab completion acceptance * [UI] Prevent Tab from moving focus when no completion popup * [UI] Refocus on Tab when completing * [UI] Let focusOut handle Tab completion acceptance * [UI] Restore Tab behavior to match original (handle in focusOut) * [UI] Defer refocus on Tab to avoid focus chain issues * [UI] Auto-select first completion match in popup * [UI] Auto-select first match when completer popup resets * Revert "[UI] Auto-select first match when completer popup resets" This reverts commit c7a3de3595a565cbcb35610ebe3e4b36322e0919. * Revert "[UI] Auto-select first completion match in popup" This reverts commit ae0dfb862263e8e39339103b1fd87535d2437c9a. * [UI] Ensure completion selection updates on model changes * Revert "[UI] Ensure completion selection updates on model changes" This reverts commit 686bbef5b2a7d87bd1ecd04c4b7e2a23143188cd. * [UI] Highlight the selected completion in chat popups The card completer's above-variant moved the current row with NoUpdate, so the delegate never painted the selection border, and it used Select which accumulated highlights while navigating. Use ClearAndSelect so only the current row is highlighted and the old one is cleared. The mention popup opened with no current row at all, so nothing was highlighted and there was no visual hint of what Enter would insert. Select the row currentCompletion() uses, and install a delegate that mirrors the active highlight into the inactive group so the selection stays visible when the transient popup is not the active window. --------- Co-authored-by: Lukas BrĂ¼bach --- .../widgets/utility/card_completer_styler.cpp | 5 +- .../widgets/utility/completer_utils.cpp | 52 +++++++++++ .../widgets/utility/line_edit_completer.cpp | 90 +++++++++++++++---- 3 files changed, 128 insertions(+), 19 deletions(-) diff --git a/cockatrice/src/interface/widgets/utility/card_completer_styler.cpp b/cockatrice/src/interface/widgets/utility/card_completer_styler.cpp index fef1dcbf7..1ddcc4200 100644 --- a/cockatrice/src/interface/widgets/utility/card_completer_styler.cpp +++ b/cockatrice/src/interface/widgets/utility/card_completer_styler.cpp @@ -150,7 +150,8 @@ bool CardCompleterStyler::handlePopupKeyPress(QKeyEvent *event) if (newRow != currentRow) { const QModelIndex newIndex = completer->completionModel()->index(newRow, completer->completionColumn()); - popup->selectionModel()->setCurrentIndex(newIndex, QItemSelectionModel::NoUpdate); + popup->selectionModel()->setCurrentIndex(newIndex, + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); } return true; @@ -180,7 +181,7 @@ void CardCompleterStyler::ensureClosestSelected() if (currentRow != closestRow) { popup->selectionModel()->setCurrentIndex(completionModel->index(closestRow, completer->completionColumn()), - QItemSelectionModel::NoUpdate); + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); } } diff --git a/cockatrice/src/interface/widgets/utility/completer_utils.cpp b/cockatrice/src/interface/widgets/utility/completer_utils.cpp index 53db58b70..f1cd9c6a7 100644 --- a/cockatrice/src/interface/widgets/utility/completer_utils.cpp +++ b/cockatrice/src/interface/widgets/utility/completer_utils.cpp @@ -3,12 +3,19 @@ #include "../../../client/settings/cache_settings.h" #include "card_completer_styler.h" +#include #include #include +#include #include +#include +#include #include #include #include +#include +#include +#include #include #include #include @@ -17,6 +24,45 @@ namespace { +/** + * @brief Paints the mention popup's current row with a visible highlight. + * + * A completer popup is a transient window that is usually not the application's + * active window, so the style paints a selected row using the palette's + * Inactive highlight group. Most themes set that colour close to the popup + * background, which hides the current completion entirely. Mirror the active + * highlight into the inactive group so the selection stays visible. + */ +class MentionCompleterDelegate : public QStyledItemDelegate +{ +public: + explicit MentionCompleterDelegate(QAbstractItemView *view) : QStyledItemDelegate(view), view(view) + { + } + + void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override + { + QStyleOptionViewItem opt = option; + opt.showDecorationSelected = true; + + if (view->currentIndex() == index) { + opt.state |= QStyle::State_HasFocus; + } + + if (opt.state & QStyle::State_Selected) { + opt.palette.setColor(QPalette::Inactive, QPalette::Highlight, + opt.palette.color(QPalette::Active, QPalette::Highlight)); + opt.palette.setColor(QPalette::Inactive, QPalette::HighlightedText, + opt.palette.color(QPalette::Active, QPalette::HighlightedText)); + } + + QStyledItemDelegate::paint(painter, opt, index); + } + +private: + QAbstractItemView *view; +}; + void applyCardSearchLanguage(CardSearchModel *searchModel) { const CardsDisplaySettings &cardsDisplay = SettingsCache::instance().cardsDisplay(); @@ -69,5 +115,11 @@ QCompleter *createMentionCompleter(QStringListModel *model, QObject *parent) completer->setCaseSensitivity(Qt::CaseInsensitive); completer->setMaxVisibleItems(5); completer->setFilterMode(Qt::MatchStartsWith); + + // Force the popup to be created so the highlight delegate can be installed; + // otherwise QCompleter's own delegate hides the selection on most themes. + QAbstractItemView *popup = completer->popup(); + popup->setItemDelegate(new MentionCompleterDelegate(popup)); + return completer; } diff --git a/cockatrice/src/interface/widgets/utility/line_edit_completer.cpp b/cockatrice/src/interface/widgets/utility/line_edit_completer.cpp index 38e3e4350..056887f1a 100644 --- a/cockatrice/src/interface/widgets/utility/line_edit_completer.cpp +++ b/cockatrice/src/interface/widgets/utility/line_edit_completer.cpp @@ -1,9 +1,11 @@ #include "line_edit_completer.h" +#include #include #include #include #include +#include #include #include #include @@ -48,37 +50,80 @@ void LineEditCompleter::focusOutEvent(QFocusEvent *e) { LineEditUnfocusable::focusOutEvent(e); - // Only commit the highlighted completion when focus moves away via Tab. + // Commit completion when focus moves away via Tab if popup is visible. // Other focus losses (e.g. the unfocus shortcut / Escape) must simply close // the popup without inserting anything. - if (e->reason() != Qt::TabFocusReason) { - hideCompleterPopups(); + bool popupWasVisible = false; + for (auto &info : completers) { + if (info.completer->popup()->isVisible()) { + popupWasVisible = true; + const QModelIndex currentIndex = info.completer->popup()->currentIndex(); + QString completionText; + if (currentIndex.isValid()) { + completionText = currentIndex.data().toString(); + } else { + completionText = info.completer->currentCompletion(); + } + if (!completionText.isEmpty()) { + insertCompletion(info.completer, completionText); + } + } + } + hideCompleterPopups(); + + if (popupWasVisible && e->reason() == Qt::TabFocusReason) { + // Refocus the edit so Tab doesn't move focus away + // Use a queued connection to ensure focus is restored after the focus change completes + QMetaObject::invokeMethod(this, "setFocus", Qt::QueuedConnection); + e->accept(); return; } - for (auto &info : completers) { - if (!info.completer->popup()->isVisible()) { - continue; - } - - const QModelIndex currentIndex = info.completer->popup()->currentIndex(); - if (currentIndex.isValid()) { - insertCompletion(info.completer, currentIndex.data().toString()); - } + if (popupWasVisible) { + return; } - - hideCompleterPopups(); } void LineEditCompleter::keyPressEvent(QKeyEvent *event) { - LineEditUnfocusable::keyPressEvent(event); - if (event->key() == Qt::Key_Escape) { - hideCompleterPopups(); + if (hasVisibleCompleterPopup()) { + event->ignore(); + hideCompleterPopups(); + return; + } + LineEditUnfocusable::keyPressEvent(event); return; } + if (event->key() == Qt::Key_Return || event->key() == Qt::Key_Enter || event->key() == Qt::Key_Space) { + for (auto &info : completers) { + if (info.completer->popup()->isVisible()) { + event->ignore(); + const QModelIndex currentIndex = info.completer->popup()->currentIndex(); + QString completionText; + if (currentIndex.isValid()) { + completionText = currentIndex.data().toString(); + } else { + completionText = info.completer->currentCompletion(); + } + if (!completionText.isEmpty()) { + insertCompletion(info.completer, completionText); + } + hideCompleterPopups(); + return; + } + } + } + + if (event->key() == Qt::Key_Tab) { + // Let Tab proceed normally; focusOutEvent will handle completion if popup is visible + LineEditUnfocusable::keyPressEvent(event); + return; + } + + LineEditUnfocusable::keyPressEvent(event); + QString textValue = text(); int cursorPos = cursorPosition(); @@ -132,6 +177,17 @@ void LineEditCompleter::keyPressEvent(QKeyEvent *event) } active->completer->complete(); + + // The popup opens without a current row, so nothing is highlighted and there + // is no visual indication of what Enter will insert. Select the row that + // currentCompletion() would use so the current mention stays visible. + auto *completionModel = active->completer->completionModel(); + const int currentRow = active->completer->currentRow(); + if (currentRow >= 0 && currentRow < completionModel->rowCount()) { + active->completer->popup()->selectionModel()->setCurrentIndex( + completionModel->index(currentRow, active->completer->completionColumn()), + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + } } void LineEditCompleter::insertCompletion(const QString &completion)