From 679762c624f9a5fbdc3c4b8ecb26905cfb27407f Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:48:50 +0200 Subject: [PATCH 1/8] [Client] Settings: extract SettingsView from DlgSettings (#7482) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Settings: extract SettingsView from DlgSettings Move the search bar, tab bar, stacked pages and search results out of DlgSettings into a reusable SettingsView widget. The dialog becomes a thin frame (title, OK button, close validation) delegating to the view. No visual or behavior change; tr() contexts preserved as DlgSettings. * [Client] Settings: match CI clang-format-18 line breaking --------- Co-authored-by: Lukas Brübach --- cockatrice/CMakeLists.txt | 1 + .../widgets/dialogs/dlg_settings.cpp | 462 +--------------- .../interface/widgets/dialogs/dlg_settings.h | 86 +-- .../widgets/dialogs/settings_view.cpp | 497 ++++++++++++++++++ .../interface/widgets/dialogs/settings_view.h | 98 ++++ 5 files changed, 625 insertions(+), 519 deletions(-) create mode 100644 cockatrice/src/interface/widgets/dialogs/settings_view.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/settings_view.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index e59326214..5baf10b1e 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -60,6 +60,7 @@ set(cockatrice_SOURCES src/interface/widgets/dialogs/dlg_view_log.cpp src/interface/widgets/dialogs/override_printing_warning.cpp src/interface/widgets/dialogs/path_safety_warning.cpp + src/interface/widgets/dialogs/settings_view.cpp src/interface/widgets/dialogs/tip_of_the_day.cpp src/interface/widgets/deck_share/deck_share_utils.cpp src/interface/widgets/deck_share/shared_deck_preview_widget.cpp diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp index d41c948eb..9b20372db 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp @@ -1,205 +1,41 @@ /** * @file dlg_settings.cpp - * @brief Implementation of the main settings dialog + * @brief Implementation of the main settings dialog (thin frame around SettingsView) * @ingroup Dialogs */ #include "dlg_settings.h" #include "../../../client/settings/cache_settings.h" -#include "../../pixel_map_generator.h" #include "../main.h" #include "../settings_page/abstract_settings_page.h" -#include "../settings_page/appearance_settings_page.h" -#include "../settings_page/deck_editor_settings_page.h" -#include "../settings_page/general_settings_page.h" -#include "../settings_page/messages_settings_page.h" -#include "../settings_page/settings_search_delegate.h" -#include "../settings_page/settings_search_model.h" -#include "../settings_page/shortcut_settings_page.h" -#include "../settings_page/sound_settings_page.h" -#include "../settings_page/storage_settings_page.h" -#include "../settings_page/user_interface_settings_page.h" #include "libcockatrice/card/database/card_database.h" #include "libcockatrice/card/database/card_database_loader.h" #include "libcockatrice/card/database/card_database_manager.h" -#include -#include #include -#include -#include #include -#include -#include #include -#include -#include -#include -#include -#include #include #include -#include -#include -#include -#include -#include #include #include -#include -#include -#include -#include -#include #include -#include #include -#include -#include -#include -#include #include -#include -#include -#include -#include -#include #include #include #include #include #include -#include -class QModelIndex; - -/** - * @brief Wraps a widget in a scroll area for long settings pages - * @param widget The widget to wrap - * @return The scroll area containing the widget - */ -static QScrollArea *makeScrollable(QWidget *widget) -{ - widget->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Maximum); - - auto *scrollArea = new QScrollArea; - scrollArea->setWidgetResizable(true); - scrollArea->setContentsMargins(0, 0, 0, 0); - scrollArea->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); - scrollArea->horizontalScrollBar()->setEnabled(false); - scrollArea->setWidget(widget); - return scrollArea; -} - -/** - * @brief Returns the theme icon resources for each settings page, indexed by SettingsPage order - */ -static QStringList pageIconResources() -{ - return {QStringLiteral("theme:config/general"), QStringLiteral("theme:config/appearance"), - QStringLiteral("theme:config/interface"), QStringLiteral("theme:config/deckeditor"), - QStringLiteral("theme:config/storage"), QStringLiteral("theme:config/messages"), - QStringLiteral("theme:config/sound"), QStringLiteral("theme:config/shorcuts")}; -} - -DlgSettings::DlgSettings(QWidget *parent) : QDialog(parent), currentTabIndex(0), searchActive(false) +DlgSettings::DlgSettings(QWidget *parent) : QDialog(parent) { auto rec = QGuiApplication::primaryScreen()->availableGeometry(); setMinimumSize(qMin(750, rec.width()), qMin(700, rec.height())); connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &DlgSettings::updateLanguage); - setupUi(); - - connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &DlgSettings::retranslateUi); - retranslateUi(); - - searchEdit->setFocus(); - - adjustSize(); -} - -void DlgSettings::setupUi() -{ - // Search bar - searchEdit = new QLineEdit; - searchEdit->setClearButtonEnabled(true); - searchEdit->addAction(themePixmap(QStringLiteral("icons/search")), QLineEdit::LeadingPosition); - searchEdit->installEventFilter(this); - connect(searchEdit, &QLineEdit::textChanged, this, &DlgSettings::onSearchTextChanged); - - auto *searchLayout = new QHBoxLayout; - searchLayout->addWidget(searchEdit); - - // Tab bar (built in setupTabBar) - setupTabBar(); - - // Pages stacked widget - pagesWidget = new QStackedWidget; - - auto *generalPage = new GeneralSettingsPage; - auto *appearancePage = new AppearanceSettingsPage; - auto *userInterfacePage = new UserInterfaceSettingsPage; - auto *deckEditorPage = new DeckEditorSettingsPage; - auto *storagePage = new StorageSettingsPage; - auto *messagesPage = new MessagesSettingsPage; - auto *soundPage = new SoundSettingsPage; - auto *shortcutsPage = new ShortcutSettingsPage; - - pages.append(generalPage); - pages.append(appearancePage); - pages.append(userInterfacePage); - pages.append(deckEditorPage); - pages.append(storagePage); - pages.append(messagesPage); - pages.append(soundPage); - pages.append(shortcutsPage); - - pagesWidget->addWidget(makeScrollable(generalPage)); - pagesWidget->addWidget(makeScrollable(appearancePage)); - pagesWidget->addWidget(makeScrollable(userInterfacePage)); - pagesWidget->addWidget(makeScrollable(deckEditorPage)); - pagesWidget->addWidget(makeScrollable(storagePage)); - pagesWidget->addWidget(makeScrollable(messagesPage)); - pagesWidget->addWidget(soundPage); - pagesWidget->addWidget(shortcutsPage); - - Q_ASSERT(pages.size() == NumPages); - - // Search results view (hidden by default) - searchResultsView = new QListView; - searchResultsView->setUniformItemSizes(false); - searchResultsView->setSelectionMode(QAbstractItemView::SingleSelection); - searchResultsView->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); - searchResultsView->setVisible(false); - searchResultsView->setStyleSheet( - "QListView::item:selected { background: palette(highlight); color: palette(highlighted-text); }"); - - searchModel = new SettingsSearchModel(this); - searchDelegate = new SettingsSearchDelegate(this); - searchResultsView->setModel(searchModel); - searchResultsView->setItemDelegate(searchDelegate); - connect(searchResultsView, &QListView::clicked, this, &DlgSettings::onSearchResultClicked); - - connect(&SettingsCache::instance(), &SettingsCache::themeChanged, this, [this] { - const QStringList icons = pageIconResources(); - for (int i = 0; i < tabButtons.size() && i < icons.size(); ++i) { - tabButtons[i]->setIcon(QPixmap(icons[i])); - } - searchDelegate->setPageIcons(icons); - searchResultsView->viewport()->update(); - }); - - // Build search index after pages are created - buildSearchIndex(); - - // Pages container (stacked widget + search results overlay) - pagesContainer = new QWidget; - auto *containerLayout = new QStackedLayout; - containerLayout->setStackingMode(QStackedLayout::StackAll); - containerLayout->addWidget(pagesWidget); - containerLayout->addWidget(searchResultsView); - pagesContainer->setLayout(containerLayout); + settingsView = new SettingsView(this); // Bottom buttons auto *buttonBox = new QHBoxLayout; @@ -211,241 +47,25 @@ void DlgSettings::setupUi() // Main layout auto *mainLayout = new QVBoxLayout; - mainLayout->addLayout(searchLayout); - mainLayout->addWidget(tabBarWidget); - auto *separator = new QFrame; - separator->setFrameShape(QFrame::HLine); - separator->setFrameShadow(QFrame::Sunken); - mainLayout->addWidget(separator); - mainLayout->addWidget(pagesContainer); + mainLayout->addWidget(settingsView); mainLayout->addSpacing(4); mainLayout->addLayout(buttonBox); setLayout(mainLayout); - // Keyboard shortcuts - auto *searchShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_F), this); - connect(searchShortcut, &QShortcut::activated, searchEdit, qOverload<>(&QLineEdit::setFocus)); + connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &DlgSettings::retranslateUi); + retranslateUi(); - auto *nextTabShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_Tab), this); - connect(nextTabShortcut, &QShortcut::activated, this, [this] { - int next = (currentTabIndex + 1) % tabButtons.size(); - setActiveTab(next); - }); - - auto *prevTabShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_Tab), this); - connect(prevTabShortcut, &QShortcut::activated, this, [this] { - int prev = (currentTabIndex - 1 + tabButtons.size()) % tabButtons.size(); - setActiveTab(prev); - }); - - // Initialize to first tab - setActiveTab(0); -} - -void DlgSettings::setupTabBar() -{ - tabBarWidget = new QWidget; - auto *tabLayout = new QHBoxLayout; - tabLayout->setContentsMargins(0, 0, 0, 0); - tabLayout->setSpacing(2); - - const QStringList iconResources = pageIconResources(); - - for (int i = 0; i < iconResources.size(); ++i) { - auto *tabButton = new QToolButton; - tabButton->setCheckable(true); - tabButton->setIcon(QPixmap(iconResources[i])); - tabButton->setIconSize(QSize(48, 48)); - tabButton->setToolButtonStyle(Qt::ToolButtonTextUnderIcon); - tabButton->setAutoExclusive(true); - tabButton->setMinimumHeight(85); - tabButton->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Fixed); - - connect(tabButton, &QToolButton::clicked, this, [this, idx = i] { onTabClicked(idx); }); - - tabButtons.append(tabButton); - tabLayout->addWidget(tabButton); - } - - tabBarWidget->setLayout(tabLayout); -} - -void DlgSettings::buildSearchIndex() -{ - QList allEntries; - - const QStringList pageNames = translatedPageNames(); - searchDelegate->setPageNames(pageNames); - searchDelegate->setPageIcons(pageIconResources()); - - for (int i = 0; i < pages.size(); ++i) { - QList pageEntries = pages[i]->getSearchEntries(); - for (auto &entry : pageEntries) { - if (entry.pageIndex == -1) { - entry.pageIndex = i; - } - } - allEntries.append(pageEntries); - } - - searchModel->setSourceEntries(allEntries); -} - -void DlgSettings::onTabClicked(int index) -{ - if (searchActive) { - switchToTabMode(); - } - setActiveTab(index); -} - -void DlgSettings::setActiveTab(int index) -{ - if (index < 0 || index >= tabButtons.size()) { - return; - } - - currentTabIndex = index; - pagesWidget->setCurrentIndex(index); - - for (int i = 0; i < tabButtons.size(); ++i) { - tabButtons[i]->setChecked(i == index); - } - - // Style active tab with a thick accent border + subtle background tint - for (int i = 0; i < tabButtons.size(); ++i) { - if (i == index) { - tabButtons[i]->setStyleSheet("QToolButton { border: none; border-bottom: 3px solid palette(highlight); " - "border-top-left-radius: 4px; border-top-right-radius: 4px; " - "background: palette(window); padding-bottom: 1px; }"); - } else { - tabButtons[i]->setStyleSheet("QToolButton { border: none; border-bottom: 1px solid transparent; " - "border-top-left-radius: 4px; border-top-right-radius: 4px; " - "background: transparent; }"); - } - } -} - -void DlgSettings::flashWidget(QWidget *widget) -{ - auto *overlay = new QWidget(widget); - overlay->setGeometry(widget->rect()); - overlay->setAttribute(Qt::WA_TransparentForMouseEvents, true); - - QPalette pal = overlay->palette(); - QColor flashColor = pal.color(QPalette::Highlight); - flashColor.setAlpha(100); - pal.setBrush(QPalette::Window, flashColor); - overlay->setPalette(pal); - overlay->setAutoFillBackground(true); - - auto *effect = new QGraphicsOpacityEffect(overlay); - effect->setOpacity(0.0); - overlay->setGraphicsEffect(effect); - overlay->show(); - overlay->raise(); - - auto *flashIn = new QPropertyAnimation(effect, "opacity"); - flashIn->setDuration(120); - flashIn->setStartValue(0.0); - flashIn->setEndValue(0.6); - flashIn->setEasingCurve(QEasingCurve::OutCubic); - - auto *fadeOut = new QPropertyAnimation(effect, "opacity"); - fadeOut->setDuration(900); - fadeOut->setStartValue(0.6); - fadeOut->setEndValue(0.0); - fadeOut->setEasingCurve(QEasingCurve::InCubic); - - auto *group = new QSequentialAnimationGroup(overlay); - group->addAnimation(flashIn); - group->addAnimation(fadeOut); - - connect(group, &QSequentialAnimationGroup::finished, overlay, &QWidget::deleteLater); - - group->start(QAbstractAnimation::DeleteWhenStopped); -} - -void DlgSettings::onSearchTextChanged(const QString &text) -{ - searchModel->setFilterString(text); - - if (searchModel->isFilterActive() && !text.trimmed().isEmpty()) { - if (!searchActive) { - switchToSearchMode(); - } - if (searchModel->rowCount(QModelIndex()) > 0) { - searchResultsView->setCurrentIndex(searchModel->index(0)); - } - } else if (searchActive) { - switchToTabMode(); - } -} - -void DlgSettings::switchToSearchMode() -{ - searchActive = true; - tabBarWidget->setVisible(false); - pagesWidget->setVisible(false); - searchResultsView->setVisible(true); - if (searchModel->rowCount(QModelIndex()) > 0) { - searchResultsView->setCurrentIndex(searchModel->index(0)); - } -} - -void DlgSettings::switchToTabMode() -{ - searchActive = false; - tabBarWidget->setVisible(true); - pagesWidget->setVisible(true); - searchResultsView->setVisible(false); - searchEdit->blockSignals(true); - searchEdit->clear(); - searchEdit->blockSignals(false); - setActiveTab(currentTabIndex); -} - -void DlgSettings::onSearchResultClicked(const QModelIndex &index) -{ - navigateToSearchResult(index); -} - -void DlgSettings::navigateToSearchResult(const QModelIndex &index) -{ - SettingsSearchEntry entry = searchModel->entryForIndex(index); - if (entry.pageIndex < 0 || entry.pageIndex >= pages.size()) { - return; - } - - // Switch to the page - switchToTabMode(); - setActiveTab(entry.pageIndex); - - // Scroll to the widget, focus it, and flash to highlight it - if (entry.widget) { - QWidget *widget = entry.widget; - while (widget) { - if (auto *scrollArea = qobject_cast(widget)) { - scrollArea->ensureWidgetVisible(entry.widget); - break; - } - widget = widget->parentWidget(); - } - entry.widget->setFocus(); - flashWidget(entry.widget); - } + adjustSize(); } void DlgSettings::setTab(int index) { - if (index >= 0 && index < tabButtons.size()) { - setActiveTab(index); - } + settingsView->setTab(index); } AbstractSettingsPage *DlgSettings::page(SettingsPage which) const { - return pages.value(static_cast(which)); + return settingsView->page(which); } void DlgSettings::updateLanguage() @@ -454,49 +74,6 @@ void DlgSettings::updateLanguage() installNewTranslator(); } -bool DlgSettings::eventFilter(QObject *watched, QEvent *event) -{ - if (watched == searchEdit && event->type() == QEvent::KeyPress) { - auto *keyEvent = static_cast(event); - if (keyEvent->key() == Qt::Key_Escape) { - if (searchActive) { - switchToTabMode(); - return true; - } - } else if (keyEvent->key() == Qt::Key_Return || keyEvent->key() == Qt::Key_Enter) { - if (searchActive) { - if (searchResultsView->currentIndex().isValid()) { - navigateToSearchResult(searchResultsView->currentIndex()); - } - return true; - } - } else if (keyEvent->key() == Qt::Key_Down) { - if (searchActive) { - int nextRow = searchResultsView->currentIndex().row() + 1; - if (nextRow >= searchModel->rowCount()) { - nextRow = 0; - } - searchResultsView->setCurrentIndex(searchModel->index(nextRow)); - searchResultsView->scrollTo(searchModel->index(nextRow)); - return true; - } - } else if (keyEvent->key() == Qt::Key_Up) { - if (searchActive) { - int prevRow = searchResultsView->currentIndex().row() - 1; - if (prevRow < 0) { - prevRow = searchModel->rowCount() - 1; - } - if (prevRow >= 0) { - searchResultsView->setCurrentIndex(searchModel->index(prevRow)); - searchResultsView->scrollTo(searchModel->index(prevRow)); - } - return true; - } - } - } - return QDialog::eventFilter(watched, event); -} - void DlgSettings::closeEvent(QCloseEvent *event) { bool showLoadError = true; @@ -576,26 +153,5 @@ void DlgSettings::closeEvent(QCloseEvent *event) void DlgSettings::retranslateUi() { setWindowTitle(tr("Settings")); - retranslateTabNames(); - - searchEdit->setPlaceholderText(tr("Search settings...")); okButton->setText(tr("OK")); - - // Rebuild search index for translated text - buildSearchIndex(); -} - -QStringList DlgSettings::translatedPageNames() -{ - return {tr("General"), tr("Appearance"), tr("User Interface"), tr("Card Sources"), - tr("Storage"), tr("Chat"), tr("Sound"), tr("Shortcuts")}; -} - -void DlgSettings::retranslateTabNames() -{ - const QStringList tabLabels = translatedPageNames(); - - for (int i = 0; i < tabButtons.size() && i < tabLabels.size(); ++i) { - tabButtons[i]->setText(tabLabels[i]); - } } diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_settings.h b/cockatrice/src/interface/widgets/dialogs/dlg_settings.h index 1f15e76d5..9942b410c 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_settings.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_settings.h @@ -1,105 +1,59 @@ /** * @file dlg_settings.h - * @brief Main settings dialog for the Cockatrice client + * @brief Main settings dialog for the Cockatrice client (thin frame around SettingsView) * @ingroup Dialogs */ #ifndef DLG_SETTINGS_H #define DLG_SETTINGS_H +#include "settings_view.h" + #include -#include #include -#include -#include #include class QPushButton; -class QModelIndex; -class QWidget; +class AbstractSettingsPage; inline Q_LOGGING_CATEGORY(DlgSettingsLog, "dlg_settings"); -class QStackedWidget; -class QToolButton; -class QListView; -class QLineEdit; -class AbstractSettingsPage; -class SettingsSearchModel; -class SettingsSearchDelegate; - /** - * @brief Main application settings dialog with tabbed navigation and search + * @brief Main application settings dialog hosting the shared SettingsView * - * Provides a modern settings interface organized into tabbed pages. Users can - * either navigate by clicking tabs or search for specific settings using the - * built-in search bar. Search results are filtered and ranked by relevance. + * Thin QDialog frame around SettingsView: only the window title, the OK button + * and the close validation live here. All pages, navigation and search are + * owned by SettingsView so other hosts (e.g. a settings tab) can reuse them. */ class DlgSettings : public QDialog { Q_OBJECT public: - /** - * @brief Page order in the tab bar, matching the order pages are added in setupUi() - * - * Use these values instead of raw indices so reordering pages never silently - * breaks external callers like tab_room.cpp. - */ - enum SettingsPage - { - GeneralPage = 0, - AppearancePage, - UserInterfacePage, - DeckEditorPage, - StoragePage, - MessagesPage, - SoundPage, - ShortcutsPage, - NumPages - }; + using SettingsPage = SettingsView::SettingsPage; + static constexpr SettingsPage GeneralPage = SettingsView::GeneralPage; + static constexpr SettingsPage AppearancePage = SettingsView::AppearancePage; + static constexpr SettingsPage UserInterfacePage = SettingsView::UserInterfacePage; + static constexpr SettingsPage DeckEditorPage = SettingsView::DeckEditorPage; + static constexpr SettingsPage StoragePage = SettingsView::StoragePage; + static constexpr SettingsPage MessagesPage = SettingsView::MessagesPage; + static constexpr SettingsPage SoundPage = SettingsView::SoundPage; + static constexpr SettingsPage ShortcutsPage = SettingsView::ShortcutsPage; + static constexpr SettingsPage NumPages = SettingsView::NumPages; explicit DlgSettings(QWidget *parent = nullptr); void setTab(int index); AbstractSettingsPage *page(SettingsPage which) const; private slots: - void onTabClicked(int index); - void onSearchTextChanged(const QString &text); - void onSearchResultClicked(const QModelIndex &index); void updateLanguage(); private: - // UI elements - QLineEdit *searchEdit; ///< Search bar for filtering settings - QWidget *tabBarWidget; ///< Container widget for the tab buttons - QList tabButtons; ///< Navigation tab buttons - QStackedWidget *pagesWidget; ///< Stacked widget containing settings pages - QListView *searchResultsView; ///< Search results list view - QWidget *pagesContainer; ///< Container stacking pages and search results - QPushButton *okButton; ///< Button to close the dialog - - // Data - QList pages; ///< All settings page instances - SettingsSearchModel *searchModel; ///< Model for search results - SettingsSearchDelegate *searchDelegate; ///< Delegate for search result rendering - int currentTabIndex; ///< Currently active tab index - bool searchActive; ///< Whether search mode is active - - void setupUi(); - void setupTabBar(); - void buildSearchIndex(); - void switchToTabMode(); - void switchToSearchMode(); - void navigateToSearchResult(const QModelIndex &index); - void setActiveTab(int index); - static void flashWidget(QWidget *widget); - static QStringList translatedPageNames(); + SettingsView *settingsView; ///< Shared settings content + QPushButton *okButton; ///< Button to close the dialog void retranslateUi(); - void retranslateTabNames(); protected: void closeEvent(QCloseEvent *event) override; - bool eventFilter(QObject *watched, QEvent *event) override; }; #endif // DLG_SETTINGS_H diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp new file mode 100644 index 000000000..c2d9860c3 --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp @@ -0,0 +1,497 @@ +/** + * @file settings_view.cpp + * @brief Implementation of the settings content widget + * @ingroup Dialogs + */ +#include "settings_view.h" + +#include "../../../client/settings/cache_settings.h" +#include "../../pixel_map_generator.h" +#include "../settings_page/abstract_settings_page.h" +#include "../settings_page/appearance_settings_page.h" +#include "../settings_page/deck_editor_settings_page.h" +#include "../settings_page/general_settings_page.h" +#include "../settings_page/messages_settings_page.h" +#include "../settings_page/settings_search_delegate.h" +#include "../settings_page/settings_search_model.h" +#include "../settings_page/shortcut_settings_page.h" +#include "../settings_page/sound_settings_page.h" +#include "../settings_page/storage_settings_page.h" +#include "../settings_page/user_interface_settings_page.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/** + * @brief Wraps a widget in a scroll area for long settings pages + * @param widget The widget to wrap + * @return The scroll area containing the widget + */ +static QScrollArea *makeScrollable(QWidget *widget) +{ + widget->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Maximum); + + auto *scrollArea = new QScrollArea; + scrollArea->setWidgetResizable(true); + scrollArea->setContentsMargins(0, 0, 0, 0); + scrollArea->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + scrollArea->horizontalScrollBar()->setEnabled(false); + scrollArea->setWidget(widget); + return scrollArea; +} + +/** + * @brief Returns the theme icon resources for each settings page, indexed by SettingsPage order + */ +static QStringList pageIconResources() +{ + return {QStringLiteral("theme:config/general"), QStringLiteral("theme:config/appearance"), + QStringLiteral("theme:config/interface"), QStringLiteral("theme:config/deckeditor"), + QStringLiteral("theme:config/storage"), QStringLiteral("theme:config/messages"), + QStringLiteral("theme:config/sound"), QStringLiteral("theme:config/shorcuts")}; +} + +SettingsView::SettingsView(QWidget *parent) : QWidget(parent), currentTabIndex(0), searchActive(false) +{ + setupUi(); + + connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &SettingsView::retranslateUi); + retranslateUi(); + + searchEdit->setFocus(); +} + +void SettingsView::setupUi() +{ + // Search bar + searchEdit = new QLineEdit; + searchEdit->setClearButtonEnabled(true); + searchEdit->addAction(themePixmap(QStringLiteral("icons/search")), QLineEdit::LeadingPosition); + searchEdit->installEventFilter(this); + connect(searchEdit, &QLineEdit::textChanged, this, &SettingsView::onSearchTextChanged); + + auto *searchLayout = new QHBoxLayout; + searchLayout->addWidget(searchEdit); + + // Tab bar (built in setupTabBar) + setupTabBar(); + + // Pages stacked widget + pagesWidget = new QStackedWidget; + + auto *generalPage = new GeneralSettingsPage; + auto *appearancePage = new AppearanceSettingsPage; + auto *userInterfacePage = new UserInterfaceSettingsPage; + auto *deckEditorPage = new DeckEditorSettingsPage; + auto *storagePage = new StorageSettingsPage; + auto *messagesPage = new MessagesSettingsPage; + auto *soundPage = new SoundSettingsPage; + auto *shortcutsPage = new ShortcutSettingsPage; + + pages.append(generalPage); + pages.append(appearancePage); + pages.append(userInterfacePage); + pages.append(deckEditorPage); + pages.append(storagePage); + pages.append(messagesPage); + pages.append(soundPage); + pages.append(shortcutsPage); + + pagesWidget->addWidget(makeScrollable(generalPage)); + pagesWidget->addWidget(makeScrollable(appearancePage)); + pagesWidget->addWidget(makeScrollable(userInterfacePage)); + pagesWidget->addWidget(makeScrollable(deckEditorPage)); + pagesWidget->addWidget(makeScrollable(storagePage)); + pagesWidget->addWidget(makeScrollable(messagesPage)); + pagesWidget->addWidget(soundPage); + pagesWidget->addWidget(shortcutsPage); + + Q_ASSERT(pages.size() == NumPages); + + // Search results view (hidden by default) + searchResultsView = new QListView; + searchResultsView->setUniformItemSizes(false); + searchResultsView->setSelectionMode(QAbstractItemView::SingleSelection); + searchResultsView->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + searchResultsView->setVisible(false); + searchResultsView->setStyleSheet( + "QListView::item:selected { background: palette(highlight); color: palette(highlighted-text); }"); + + searchModel = new SettingsSearchModel(this); + searchDelegate = new SettingsSearchDelegate(this); + searchResultsView->setModel(searchModel); + searchResultsView->setItemDelegate(searchDelegate); + connect(searchResultsView, &QListView::clicked, this, &SettingsView::onSearchResultClicked); + + connect(&SettingsCache::instance(), &SettingsCache::themeChanged, this, [this] { + const QStringList icons = pageIconResources(); + for (int i = 0; i < tabButtons.size() && i < icons.size(); ++i) { + tabButtons[i]->setIcon(QPixmap(icons[i])); + } + searchDelegate->setPageIcons(icons); + searchResultsView->viewport()->update(); + }); + + // Build search index after pages are created + buildSearchIndex(); + + // Pages container (stacked widget + search results overlay) + pagesContainer = new QWidget; + auto *containerLayout = new QStackedLayout; + containerLayout->setStackingMode(QStackedLayout::StackAll); + containerLayout->addWidget(pagesWidget); + containerLayout->addWidget(searchResultsView); + pagesContainer->setLayout(containerLayout); + + // Main layout + auto *mainLayout = new QVBoxLayout; + mainLayout->addLayout(searchLayout); + mainLayout->addWidget(tabBarWidget); + auto *separator = new QFrame; + separator->setFrameShape(QFrame::HLine); + separator->setFrameShadow(QFrame::Sunken); + mainLayout->addWidget(separator); + mainLayout->addWidget(pagesContainer); + mainLayout->addSpacing(4); + setLayout(mainLayout); + + // Keyboard shortcuts + auto *searchShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_F), this); + connect(searchShortcut, &QShortcut::activated, searchEdit, qOverload<>(&QLineEdit::setFocus)); + + auto *nextTabShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_Tab), this); + connect(nextTabShortcut, &QShortcut::activated, this, [this] { + int next = (currentTabIndex + 1) % tabButtons.size(); + setActiveTab(next); + }); + + auto *prevTabShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_Tab), this); + connect(prevTabShortcut, &QShortcut::activated, this, [this] { + int prev = (currentTabIndex - 1 + tabButtons.size()) % tabButtons.size(); + setActiveTab(prev); + }); + + // Initialize to first tab + setActiveTab(0); +} + +void SettingsView::setupTabBar() +{ + tabBarWidget = new QWidget; + auto *tabLayout = new QHBoxLayout; + tabLayout->setContentsMargins(0, 0, 0, 0); + tabLayout->setSpacing(2); + + const QStringList iconResources = pageIconResources(); + + for (int i = 0; i < iconResources.size(); ++i) { + auto *tabButton = new QToolButton; + tabButton->setCheckable(true); + tabButton->setIcon(QPixmap(iconResources[i])); + tabButton->setIconSize(QSize(48, 48)); + tabButton->setToolButtonStyle(Qt::ToolButtonTextUnderIcon); + tabButton->setAutoExclusive(true); + tabButton->setMinimumHeight(85); + tabButton->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Fixed); + + connect(tabButton, &QToolButton::clicked, this, [this, idx = i] { onTabClicked(idx); }); + + tabButtons.append(tabButton); + tabLayout->addWidget(tabButton); + } + + tabBarWidget->setLayout(tabLayout); +} + +void SettingsView::buildSearchIndex() +{ + QList allEntries; + + const QStringList pageNames = translatedPageNames(); + searchDelegate->setPageNames(pageNames); + searchDelegate->setPageIcons(pageIconResources()); + + for (int i = 0; i < pages.size(); ++i) { + QList pageEntries = pages[i]->getSearchEntries(); + for (auto &entry : pageEntries) { + if (entry.pageIndex == -1) { + entry.pageIndex = i; + } + } + allEntries.append(pageEntries); + } + + searchModel->setSourceEntries(allEntries); +} + +void SettingsView::onTabClicked(int index) +{ + if (searchActive) { + switchToTabMode(); + } + setActiveTab(index); +} + +void SettingsView::setActiveTab(int index) +{ + if (index < 0 || index >= tabButtons.size()) { + return; + } + + currentTabIndex = index; + pagesWidget->setCurrentIndex(index); + + for (int i = 0; i < tabButtons.size(); ++i) { + tabButtons[i]->setChecked(i == index); + } + + // Style active tab with a thick accent border + subtle background tint + for (int i = 0; i < tabButtons.size(); ++i) { + if (i == index) { + tabButtons[i]->setStyleSheet("QToolButton { border: none; border-bottom: 3px solid palette(highlight); " + "border-top-left-radius: 4px; border-top-right-radius: 4px; " + "background: palette(window); padding-bottom: 1px; }"); + } else { + tabButtons[i]->setStyleSheet("QToolButton { border: none; border-bottom: 1px solid transparent; " + "border-top-left-radius: 4px; border-top-right-radius: 4px; " + "background: transparent; }"); + } + } +} + +void SettingsView::flashWidget(QWidget *widget) +{ + auto *overlay = new QWidget(widget); + overlay->setGeometry(widget->rect()); + overlay->setAttribute(Qt::WA_TransparentForMouseEvents, true); + + QPalette pal = overlay->palette(); + QColor flashColor = pal.color(QPalette::Highlight); + flashColor.setAlpha(100); + pal.setBrush(QPalette::Window, flashColor); + overlay->setPalette(pal); + overlay->setAutoFillBackground(true); + + auto *effect = new QGraphicsOpacityEffect(overlay); + effect->setOpacity(0.0); + overlay->setGraphicsEffect(effect); + overlay->show(); + overlay->raise(); + + auto *flashIn = new QPropertyAnimation(effect, "opacity"); + flashIn->setDuration(120); + flashIn->setStartValue(0.0); + flashIn->setEndValue(0.6); + flashIn->setEasingCurve(QEasingCurve::OutCubic); + + auto *fadeOut = new QPropertyAnimation(effect, "opacity"); + fadeOut->setDuration(900); + fadeOut->setStartValue(0.6); + fadeOut->setEndValue(0.0); + fadeOut->setEasingCurve(QEasingCurve::InCubic); + + auto *group = new QSequentialAnimationGroup(overlay); + group->addAnimation(flashIn); + group->addAnimation(fadeOut); + + connect(group, &QSequentialAnimationGroup::finished, overlay, &QWidget::deleteLater); + + group->start(QAbstractAnimation::DeleteWhenStopped); +} + +void SettingsView::onSearchTextChanged(const QString &text) +{ + searchModel->setFilterString(text); + + if (searchModel->isFilterActive() && !text.trimmed().isEmpty()) { + if (!searchActive) { + switchToSearchMode(); + } + if (searchModel->rowCount(QModelIndex()) > 0) { + searchResultsView->setCurrentIndex(searchModel->index(0)); + } + } else if (searchActive) { + switchToTabMode(); + } +} + +void SettingsView::switchToSearchMode() +{ + searchActive = true; + tabBarWidget->setVisible(false); + pagesWidget->setVisible(false); + searchResultsView->setVisible(true); + if (searchModel->rowCount(QModelIndex()) > 0) { + searchResultsView->setCurrentIndex(searchModel->index(0)); + } +} + +void SettingsView::switchToTabMode() +{ + searchActive = false; + tabBarWidget->setVisible(true); + pagesWidget->setVisible(true); + searchResultsView->setVisible(false); + searchEdit->blockSignals(true); + searchEdit->clear(); + searchEdit->blockSignals(false); + setActiveTab(currentTabIndex); +} + +void SettingsView::onSearchResultClicked(const QModelIndex &index) +{ + navigateToSearchResult(index); +} + +void SettingsView::navigateToSearchResult(const QModelIndex &index) +{ + SettingsSearchEntry entry = searchModel->entryForIndex(index); + if (entry.pageIndex < 0 || entry.pageIndex >= pages.size()) { + return; + } + + // Switch to the page + switchToTabMode(); + setActiveTab(entry.pageIndex); + + // Scroll to the widget, focus it, and flash to highlight it + if (entry.widget) { + QWidget *widget = entry.widget; + while (widget) { + if (auto *scrollArea = qobject_cast(widget)) { + scrollArea->ensureWidgetVisible(entry.widget); + break; + } + widget = widget->parentWidget(); + } + entry.widget->setFocus(); + flashWidget(entry.widget); + } +} + +void SettingsView::setTab(int index) +{ + if (index >= 0 && index < tabButtons.size()) { + setActiveTab(index); + } +} + +AbstractSettingsPage *SettingsView::page(SettingsPage which) const +{ + return pages.value(static_cast(which)); +} + +bool SettingsView::eventFilter(QObject *watched, QEvent *event) +{ + if (watched == searchEdit && event->type() == QEvent::KeyPress) { + auto *keyEvent = static_cast(event); + if (keyEvent->key() == Qt::Key_Escape) { + if (searchActive) { + switchToTabMode(); + return true; + } + } else if (keyEvent->key() == Qt::Key_Return || keyEvent->key() == Qt::Key_Enter) { + if (searchActive) { + if (searchResultsView->currentIndex().isValid()) { + navigateToSearchResult(searchResultsView->currentIndex()); + } + return true; + } + } else if (keyEvent->key() == Qt::Key_Down) { + if (searchActive) { + int nextRow = searchResultsView->currentIndex().row() + 1; + if (nextRow >= searchModel->rowCount()) { + nextRow = 0; + } + searchResultsView->setCurrentIndex(searchModel->index(nextRow)); + searchResultsView->scrollTo(searchModel->index(nextRow)); + return true; + } + } else if (keyEvent->key() == Qt::Key_Up) { + if (searchActive) { + int prevRow = searchResultsView->currentIndex().row() - 1; + if (prevRow < 0) { + prevRow = searchModel->rowCount() - 1; + } + if (prevRow >= 0) { + searchResultsView->setCurrentIndex(searchModel->index(prevRow)); + searchResultsView->scrollTo(searchModel->index(prevRow)); + } + return true; + } + } + } + return QWidget::eventFilter(watched, event); +} + +void SettingsView::retranslateUi() +{ + retranslateTabNames(); + + // NOTE: the "DlgSettings" context is deliberate — these strings lived in DlgSettings + // before the view was extracted, and keeping the context preserves existing translations. + searchEdit->setPlaceholderText(QCoreApplication::translate("DlgSettings", "Search settings...")); + + // Rebuild search index for translated text + buildSearchIndex(); +} + +QStringList SettingsView::translatedPageNames() +{ + // NOTE: same "DlgSettings" context as above (translation continuity, see retranslateUi). + return {QCoreApplication::translate("DlgSettings", "General"), + QCoreApplication::translate("DlgSettings", "Appearance"), + QCoreApplication::translate("DlgSettings", "User Interface"), + QCoreApplication::translate("DlgSettings", "Card Sources"), + QCoreApplication::translate("DlgSettings", "Storage"), + QCoreApplication::translate("DlgSettings", "Chat"), + QCoreApplication::translate("DlgSettings", "Sound"), + QCoreApplication::translate("DlgSettings", "Shortcuts")}; +} + +void SettingsView::retranslateTabNames() +{ + const QStringList tabLabels = translatedPageNames(); + + for (int i = 0; i < tabButtons.size() && i < tabLabels.size(); ++i) { + tabButtons[i]->setText(tabLabels[i]); + } +} diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.h b/cockatrice/src/interface/widgets/dialogs/settings_view.h new file mode 100644 index 000000000..fe1b925d8 --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.h @@ -0,0 +1,98 @@ +/** + * @file settings_view.h + * @brief Settings content widget hosted by the settings dialog (and, later, the settings tab) + * @ingroup Dialogs + */ +#ifndef SETTINGS_VIEW_H +#define SETTINGS_VIEW_H + +#include +#include +#include +#include +#include + +class QModelIndex; +class QObject; +class QEvent; + +class QStackedWidget; +class QToolButton; +class QListView; +class QLineEdit; +class AbstractSettingsPage; +class SettingsSearchModel; +class SettingsSearchDelegate; + +/** + * @brief Settings pages with tabbed navigation and search, without any dialog chrome + * + * Owns the search bar, the tab bar, the stacked settings pages and the search + * results view. Dialog/tab hosts only add their own frame (buttons, titles). + */ +class SettingsView : public QWidget +{ + Q_OBJECT +public: + /** + * @brief Page order in the tab bar, matching the order pages are added in setupUi() + * + * Use these values instead of raw indices so reordering pages never silently + * breaks external callers like tab_room.cpp. + */ + enum SettingsPage + { + GeneralPage = 0, + AppearancePage, + UserInterfacePage, + DeckEditorPage, + StoragePage, + MessagesPage, + SoundPage, + ShortcutsPage, + NumPages + }; + + explicit SettingsView(QWidget *parent = nullptr); + void setTab(int index); + AbstractSettingsPage *page(SettingsPage which) const; + +private slots: + void onTabClicked(int index); + void onSearchTextChanged(const QString &text); + void onSearchResultClicked(const QModelIndex &index); + +private: + // UI elements + QLineEdit *searchEdit; ///< Search bar for filtering settings + QWidget *tabBarWidget; ///< Container widget for the tab buttons + QList tabButtons; ///< Navigation tab buttons + QStackedWidget *pagesWidget; ///< Stacked widget containing settings pages + QListView *searchResultsView; ///< Search results list view + QWidget *pagesContainer; ///< Container stacking pages and search results + + // Data + QList pages; ///< All settings page instances + SettingsSearchModel *searchModel; ///< Model for search results + SettingsSearchDelegate *searchDelegate; ///< Delegate for search result rendering + int currentTabIndex; ///< Currently active tab index + bool searchActive; ///< Whether search mode is active + + void setupUi(); + void setupTabBar(); + void buildSearchIndex(); + void switchToTabMode(); + void switchToSearchMode(); + void navigateToSearchResult(const QModelIndex &index); + void setActiveTab(int index); + static void flashWidget(QWidget *widget); + static QStringList translatedPageNames(); + + void retranslateUi(); + void retranslateTabNames(); + +protected: + bool eventFilter(QObject *watched, QEvent *event) override; +}; + +#endif // SETTINGS_VIEW_H From 665f7267e4df61f80b13d5b38153a56278d77411 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:48:50 +0200 Subject: [PATCH 2/8] [Client] Settings: enlarge the settings dialog (#7483) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Settings: enlarge the settings dialog Keep settings modal per review: size to ~90% of the main window (5% margins each side) with screen fallback, size grip and 750x700 minimum bounded to the screen. Drops the tab approach while giving dense pages room without the tab-switch friction. * [Client] Settings: clamp dialog width to 1280 per review * [Client] Settings: extract dialog size calculation per review Move the host/screen target math out of the DlgSettings constructor into a calculateDialogSize helper. Drop the leftover 'per review' wording from the clamp comment. No behavior change. --------- Co-authored-by: Lukas Brübach --- .../widgets/dialogs/dlg_settings.cpp | 46 +++++++++++++++++-- .../interface/widgets/dialogs/dlg_settings.h | 3 +- .../interface/widgets/dialogs/settings_view.h | 4 +- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp index 9b20372db..ab870a2c8 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -28,10 +29,49 @@ #include #include +static constexpr int MIN_DIALOG_WIDTH = 750; +static constexpr int MIN_DIALOG_HEIGHT = 700; +static constexpr int MAX_DIALOG_WIDTH = 1280; +static constexpr double DIALOG_SIZE_RATIO = 0.9; + +/** + * @brief Calculates the initial dialog size from the host window or screen + * @param hostSize Size of the hosting main window, or empty when parentless + * @param availableSize Available screen area the dialog must fit into + * @param minimumSize Minimum dialog size already applied via setMinimumSize + * @return Target size at ~90% of the host with small margins, clamped to fit + */ +static QSize calculateDialogSize(const QSize &hostSize, const QSize &availableSize, const QSize &minimumSize) +{ + QSize target = !hostSize.isEmpty() ? QSize(qRound(hostSize.width() * DIALOG_SIZE_RATIO), + qRound(hostSize.height() * DIALOG_SIZE_RATIO)) + : QSize(qRound(availableSize.width() * DIALOG_SIZE_RATIO), + qRound(availableSize.height() * DIALOG_SIZE_RATIO)); + // Evenly-spaced columns leave unnatural dead space on very wide windows, so clamp the width. + target.setWidth(qMin(target.width(), MAX_DIALOG_WIDTH)); + return target.expandedTo(minimumSize).boundedTo(availableSize); +} + DlgSettings::DlgSettings(QWidget *parent) : QDialog(parent) { - auto rec = QGuiApplication::primaryScreen()->availableGeometry(); - setMinimumSize(qMin(750, rec.width()), qMin(700, rec.height())); + // Fill the main window with small margins so multi-group pages (e.g. Deck Editor) are not buried in a scroll. + // 90% of the host window leaves ~5% margins on each side; falls back to 90% of the screen when parentless. + QWidget *hostWindow = parent ? parent->window() : nullptr; + QScreen *screen = nullptr; + QSize hostSize; + if (hostWindow) { + hostSize = hostWindow->size(); + screen = QGuiApplication::screenAt(hostWindow->geometry().center()); + } + if (!screen) { + screen = QGuiApplication::primaryScreen(); + } + const QRect available = screen ? screen->availableGeometry() : QRect(0, 0, 1280, 800); + const QSize minimum(qMin(MIN_DIALOG_WIDTH, available.width()), qMin(MIN_DIALOG_HEIGHT, available.height())); + setMinimumSize(minimum); + + resize(calculateDialogSize(hostSize, available.size(), minimum)); + setSizeGripEnabled(true); connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &DlgSettings::updateLanguage); @@ -54,8 +94,6 @@ DlgSettings::DlgSettings(QWidget *parent) : QDialog(parent) connect(&SettingsCache::instance().personal(), &PersonalSettings::langChanged, this, &DlgSettings::retranslateUi); retranslateUi(); - - adjustSize(); } void DlgSettings::setTab(int index) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_settings.h b/cockatrice/src/interface/widgets/dialogs/dlg_settings.h index 9942b410c..03ccd6b7e 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_settings.h +++ b/cockatrice/src/interface/widgets/dialogs/dlg_settings.h @@ -22,7 +22,8 @@ inline Q_LOGGING_CATEGORY(DlgSettingsLog, "dlg_settings"); * * Thin QDialog frame around SettingsView: only the window title, the OK button * and the close validation live here. All pages, navigation and search are - * owned by SettingsView so other hosts (e.g. a settings tab) can reuse them. + * owned by SettingsView. The dialog fills ~90% of the main window so dense + * pages stay visible without losing the modal adjust-and-return flow. */ class DlgSettings : public QDialog { diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.h b/cockatrice/src/interface/widgets/dialogs/settings_view.h index fe1b925d8..f1d248679 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.h +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.h @@ -1,6 +1,6 @@ /** * @file settings_view.h - * @brief Settings content widget hosted by the settings dialog (and, later, the settings tab) + * @brief Settings content widget hosted by the settings dialog * @ingroup Dialogs */ #ifndef SETTINGS_VIEW_H @@ -28,7 +28,7 @@ class SettingsSearchDelegate; * @brief Settings pages with tabbed navigation and search, without any dialog chrome * * Owns the search bar, the tab bar, the stacked settings pages and the search - * results view. Dialog/tab hosts only add their own frame (buttons, titles). + * results view. The settings dialog only adds its own frame (buttons, titles). */ class SettingsView : public QWidget { From 860625be4df3a9fef8cdcd559c010653a5e26c49 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:48:50 +0200 Subject: [PATCH 3/8] [Client] Settings: grouped left navigation for settings pages (#7484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Settings: grouped left navigation for settings pages Replace the top icon-button bar in SettingsView with a grouped left QTreeWidget: Application, Cards and Data, Chat and Sound, Advanced. The active page is the selected row (palette highlight); search mode hides the tree; group headers expand but never select. Page tr() contexts stay DlgSettings; new group names are SettingsView strings. Reworked to stay in the enlarged modal instead of the settings tab. * [Settings] Make groupedPages static const per review --------- Co-authored-by: Lukas Brübach --- .../widgets/dialogs/settings_view.cpp | 146 +++++++++++------- .../interface/widgets/dialogs/settings_view.h | 27 ++-- 2 files changed, 101 insertions(+), 72 deletions(-) diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp index c2d9860c3..483aa31bf 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -53,7 +52,7 @@ #include #include #include -#include +#include #include #include #include @@ -111,8 +110,8 @@ void SettingsView::setupUi() auto *searchLayout = new QHBoxLayout; searchLayout->addWidget(searchEdit); - // Tab bar (built in setupTabBar) - setupTabBar(); + // Navigation tree (built in setupNavTree) + setupNavTree(); // Pages stacked widget pagesWidget = new QStackedWidget; @@ -163,8 +162,10 @@ void SettingsView::setupUi() connect(&SettingsCache::instance(), &SettingsCache::themeChanged, this, [this] { const QStringList icons = pageIconResources(); - for (int i = 0; i < tabButtons.size() && i < icons.size(); ++i) { - tabButtons[i]->setIcon(QPixmap(icons[i])); + for (int i = 0; i < pageItems.size() && i < icons.size(); ++i) { + if (pageItems[i]) { + pageItems[i]->setIcon(0, QPixmap(icons[i])); + } } searchDelegate->setPageIcons(icons); searchResultsView->viewport()->update(); @@ -181,16 +182,14 @@ void SettingsView::setupUi() containerLayout->addWidget(searchResultsView); pagesContainer->setLayout(containerLayout); - // Main layout + // Main layout: search on top, navigation + pages side by side below + auto *contentLayout = new QHBoxLayout; + contentLayout->addWidget(navTree); + contentLayout->addWidget(pagesContainer, 1); + auto *mainLayout = new QVBoxLayout; mainLayout->addLayout(searchLayout); - mainLayout->addWidget(tabBarWidget); - auto *separator = new QFrame; - separator->setFrameShape(QFrame::HLine); - separator->setFrameShadow(QFrame::Sunken); - mainLayout->addWidget(separator); - mainLayout->addWidget(pagesContainer); - mainLayout->addSpacing(4); + mainLayout->addLayout(contentLayout); setLayout(mainLayout); // Keyboard shortcuts @@ -199,13 +198,13 @@ void SettingsView::setupUi() auto *nextTabShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_Tab), this); connect(nextTabShortcut, &QShortcut::activated, this, [this] { - int next = (currentTabIndex + 1) % tabButtons.size(); + int next = (currentTabIndex + 1) % pages.size(); setActiveTab(next); }); auto *prevTabShortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_Tab), this); connect(prevTabShortcut, &QShortcut::activated, this, [this] { - int prev = (currentTabIndex - 1 + tabButtons.size()) % tabButtons.size(); + int prev = (currentTabIndex - 1 + pages.size()) % pages.size(); setActiveTab(prev); }); @@ -213,32 +212,56 @@ void SettingsView::setupUi() setActiveTab(0); } -void SettingsView::setupTabBar() +void SettingsView::setupNavTree() { - tabBarWidget = new QWidget; - auto *tabLayout = new QHBoxLayout; - tabLayout->setContentsMargins(0, 0, 0, 0); - tabLayout->setSpacing(2); + navTree = new QTreeWidget; + navTree->setHeaderHidden(true); + navTree->setRootIsDecorated(true); + navTree->setIconSize(QSize(24, 24)); + navTree->setMinimumWidth(230); + navTree->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + navTree->setStyleSheet( + "QTreeWidget::item:selected { background: palette(highlight); color: palette(highlighted-text); }"); + // Groups of SettingsPage values, in navigation order + static const QList> groupedPages = { + {GeneralPage, AppearancePage, UserInterfacePage}, + {DeckEditorPage, StoragePage}, + {MessagesPage, SoundPage}, + {ShortcutsPage}, + }; + + const QStringList groupNames = translatedGroupNames(); + const QStringList pageNames = translatedPageNames(); const QStringList iconResources = pageIconResources(); - for (int i = 0; i < iconResources.size(); ++i) { - auto *tabButton = new QToolButton; - tabButton->setCheckable(true); - tabButton->setIcon(QPixmap(iconResources[i])); - tabButton->setIconSize(QSize(48, 48)); - tabButton->setToolButtonStyle(Qt::ToolButtonTextUnderIcon); - tabButton->setAutoExclusive(true); - tabButton->setMinimumHeight(85); - tabButton->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Fixed); - - connect(tabButton, &QToolButton::clicked, this, [this, idx = i] { onTabClicked(idx); }); - - tabButtons.append(tabButton); - tabLayout->addWidget(tabButton); + pageItems.resize(NumPages); + for (int i = 0; i < groupedPages.size(); ++i) { + auto *group = new QTreeWidgetItem(navTree, {groupNames[i]}); + group->setFlags(Qt::ItemIsEnabled); // header row: expandable, not selectable + group->setExpanded(true); + groupItems.append(group); + for (SettingsPage page : groupedPages[i]) { + auto *item = new QTreeWidgetItem(group, {pageNames[page]}); + item->setIcon(0, QPixmap(iconResources[page])); + item->setData(0, Qt::UserRole, static_cast(page)); + pageItems[page] = item; + } } - tabBarWidget->setLayout(tabLayout); + Q_ASSERT(groupItems.size() == groupedPages.size()); + Q_ASSERT(!pageItems.contains(nullptr)); + + connect(navTree, &QTreeWidget::currentItemChanged, this, [this](QTreeWidgetItem *current) { + if (!current) { + return; + } + bool ok = false; + int index = current->data(0, Qt::UserRole).toInt(&ok); + if (ok) { + onTabClicked(index); + } + }); } void SettingsView::buildSearchIndex() @@ -266,35 +289,27 @@ void SettingsView::onTabClicked(int index) { if (searchActive) { switchToTabMode(); + } else if (index == currentTabIndex) { + return; } setActiveTab(index); } void SettingsView::setActiveTab(int index) { - if (index < 0 || index >= tabButtons.size()) { + if (index < 0 || index >= pageItems.size() || !pageItems[index]) { return; } currentTabIndex = index; pagesWidget->setCurrentIndex(index); - for (int i = 0; i < tabButtons.size(); ++i) { - tabButtons[i]->setChecked(i == index); - } - - // Style active tab with a thick accent border + subtle background tint - for (int i = 0; i < tabButtons.size(); ++i) { - if (i == index) { - tabButtons[i]->setStyleSheet("QToolButton { border: none; border-bottom: 3px solid palette(highlight); " - "border-top-left-radius: 4px; border-top-right-radius: 4px; " - "background: palette(window); padding-bottom: 1px; }"); - } else { - tabButtons[i]->setStyleSheet("QToolButton { border: none; border-bottom: 1px solid transparent; " - "border-top-left-radius: 4px; border-top-right-radius: 4px; " - "background: transparent; }"); - } + // Selecting the item also expands a collapsed group; selecting the already + // current item emits nothing, so the currentItemChanged handler cannot loop. + if (QTreeWidgetItem *parent = pageItems[index]->parent()) { + parent->setExpanded(true); } + navTree->setCurrentItem(pageItems[index]); } void SettingsView::flashWidget(QWidget *widget) @@ -356,7 +371,7 @@ void SettingsView::onSearchTextChanged(const QString &text) void SettingsView::switchToSearchMode() { searchActive = true; - tabBarWidget->setVisible(false); + navTree->setVisible(false); pagesWidget->setVisible(false); searchResultsView->setVisible(true); if (searchModel->rowCount(QModelIndex()) > 0) { @@ -367,7 +382,7 @@ void SettingsView::switchToSearchMode() void SettingsView::switchToTabMode() { searchActive = false; - tabBarWidget->setVisible(true); + navTree->setVisible(true); pagesWidget->setVisible(true); searchResultsView->setVisible(false); searchEdit->blockSignals(true); @@ -409,7 +424,7 @@ void SettingsView::navigateToSearchResult(const QModelIndex &index) void SettingsView::setTab(int index) { - if (index >= 0 && index < tabButtons.size()) { + if (index >= 0 && index < pageItems.size()) { setActiveTab(index); } } @@ -464,7 +479,7 @@ bool SettingsView::eventFilter(QObject *watched, QEvent *event) void SettingsView::retranslateUi() { - retranslateTabNames(); + retranslateNavNames(); // NOTE: the "DlgSettings" context is deliberate — these strings lived in DlgSettings // before the view was extracted, and keeping the context preserves existing translations. @@ -487,11 +502,22 @@ QStringList SettingsView::translatedPageNames() QCoreApplication::translate("DlgSettings", "Shortcuts")}; } -void SettingsView::retranslateTabNames() +void SettingsView::retranslateNavNames() { - const QStringList tabLabels = translatedPageNames(); + const QStringList groupNames = translatedGroupNames(); + for (int i = 0; i < groupItems.size() && i < groupNames.size(); ++i) { + groupItems[i]->setText(0, groupNames[i]); + } - for (int i = 0; i < tabButtons.size() && i < tabLabels.size(); ++i) { - tabButtons[i]->setText(tabLabels[i]); + const QStringList pageNames = translatedPageNames(); + for (int i = 0; i < pageItems.size() && i < pageNames.size(); ++i) { + if (pageItems[i]) { + pageItems[i]->setText(0, pageNames[i]); + } } } + +QStringList SettingsView::translatedGroupNames() +{ + return {tr("Application"), tr("Cards & Data"), tr("Chat & Sound"), tr("Advanced")}; +} diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.h b/cockatrice/src/interface/widgets/dialogs/settings_view.h index f1d248679..214353660 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.h +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.h @@ -17,7 +17,8 @@ class QObject; class QEvent; class QStackedWidget; -class QToolButton; +class QTreeWidget; +class QTreeWidgetItem; class QListView; class QLineEdit; class AbstractSettingsPage; @@ -25,9 +26,9 @@ class SettingsSearchModel; class SettingsSearchDelegate; /** - * @brief Settings pages with tabbed navigation and search, without any dialog chrome + * @brief Settings pages with grouped left navigation and search, without any dialog chrome * - * Owns the search bar, the tab bar, the stacked settings pages and the search + * Owns the search bar, the navigation tree, the stacked settings pages and the search * results view. The settings dialog only adds its own frame (buttons, titles). */ class SettingsView : public QWidget @@ -35,7 +36,7 @@ class SettingsView : public QWidget Q_OBJECT public: /** - * @brief Page order in the tab bar, matching the order pages are added in setupUi() + * @brief Page order in the navigation, matching the order pages are added in setupUi() * * Use these values instead of raw indices so reordering pages never silently * breaks external callers like tab_room.cpp. @@ -64,12 +65,13 @@ private slots: private: // UI elements - QLineEdit *searchEdit; ///< Search bar for filtering settings - QWidget *tabBarWidget; ///< Container widget for the tab buttons - QList tabButtons; ///< Navigation tab buttons - QStackedWidget *pagesWidget; ///< Stacked widget containing settings pages - QListView *searchResultsView; ///< Search results list view - QWidget *pagesContainer; ///< Container stacking pages and search results + QLineEdit *searchEdit; ///< Search bar for filtering settings + QTreeWidget *navTree; ///< Grouped navigation tree for the pages + QList groupItems; ///< Group header rows, indexed by group + QList pageItems; ///< Page rows, indexed by SettingsPage + QStackedWidget *pagesWidget; ///< Stacked widget containing settings pages + QListView *searchResultsView; ///< Search results list view + QWidget *pagesContainer; ///< Container stacking pages and search results // Data QList pages; ///< All settings page instances @@ -79,7 +81,7 @@ private: bool searchActive; ///< Whether search mode is active void setupUi(); - void setupTabBar(); + void setupNavTree(); void buildSearchIndex(); void switchToTabMode(); void switchToSearchMode(); @@ -87,9 +89,10 @@ private: void setActiveTab(int index); static void flashWidget(QWidget *widget); static QStringList translatedPageNames(); + static QStringList translatedGroupNames(); void retranslateUi(); - void retranslateTabNames(); + void retranslateNavNames(); protected: bool eventFilter(QObject *watched, QEvent *event) override; From 4974cac59af26c8885bf90005b58faec8e059313 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:48:51 +0200 Subject: [PATCH 4/8] [Client] Settings: add top-right cogwheel shortcut (#7485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Settings: add top-right cogwheel shortcut Add a subtle flat settings button to the tab bar corner that opens the settings dialog via the existing menu action slot. Icon and tooltip follow the menu action in retranslateUi. * [Client] Settings: flat black-cogwheel corner entry per review --------- Co-authored-by: Lukas Brübach --- cockatrice/src/interface/window_main.cpp | 18 ++++++++++++++++++ cockatrice/src/interface/window_main.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index d23ce254c..636cee281 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -82,10 +82,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -355,6 +357,9 @@ void MainWindow::retranslateUi() aForgotPassword->setText(tr("&Restore password...")); aSettings->setText(tr("&Settings...")); aSettings->setIcon(themePixmap(QStringLiteral("icons/settings"))); + settingsButton->setToolTip(tr("Settings")); + settingsButton->setAccessibleName(tr("Settings")); + settingsButton->setIcon(themePixmap(QStringLiteral("icons/cogwheel"))); aExit->setText(tr("&Exit")); #if defined(__APPLE__) /* For OSX */ @@ -559,6 +564,19 @@ MainWindow::MainWindow(QWidget *parent) setCentralWidget(tabSupervisor); + // Unobtrusive settings entry in the tab bar corner: flat icon-only button that blends into the tab + // bar until hovered, mirroring the menu action for discoverability + settingsButton = new QToolButton(tabSupervisor); + settingsButton->setAutoRaise(true); + settingsButton->setToolButtonStyle(Qt::ToolButtonIconOnly); + settingsButton->setIconSize(QSize(16, 16)); + settingsButton->setCursor(Qt::PointingHandCursor); + settingsButton->setFocusPolicy(Qt::NoFocus); + settingsButton->setStyleSheet( + QStringLiteral("QToolButton { border: none; background: transparent; padding: 4px; }")); + connect(settingsButton, &QToolButton::clicked, this, &MainWindow::actSettings); + tabSupervisor->setCornerWidget(settingsButton, Qt::TopRightCorner); + retranslateUi(); if (!restoreGeometry(SettingsCache::instance().layouts().getMainWindowGeometry())) { diff --git a/cockatrice/src/interface/window_main.h b/cockatrice/src/interface/window_main.h index 4b3da716d..bcf709cb1 100644 --- a/cockatrice/src/interface/window_main.h +++ b/cockatrice/src/interface/window_main.h @@ -46,6 +46,7 @@ #include class QAction; +class QToolButton; class QWidget; struct LocalGameOptions; @@ -178,6 +179,7 @@ private: QList tabMenus; QMenu *cockatriceMenu, *dbMenu, *tabsMenu, *helpMenu, *trayIconMenu; QAction *aAbout, *aSettings, *aShow, *aExit; + QToolButton *settingsButton = nullptr; QAction *aConnect, *aDisconnect, *aRegister, *aForgotPassword, *aSinglePlayer, *aWatchReplay, *aFullScreen; QAction *aManageSets, *aEditTokens, *aOpenCustomFolder, *aOpenCustomsetsFolder, *aAddCustomSet, *aReloadCardDatabase; From 4750e67b7fe109adfa6225525f563bd7a1ff3850 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:48:51 +0200 Subject: [PATCH 5/8] [Client] Settings: sub-page rows deep-linking into page sections (#7487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add AbstractSettingsPage::getSections built from each page's group boxes. Show each multi-group page's sections as child rows in the navigation tree. Section rows show the parent page and scroll to and highlight the group box. Start pages collapsed. Keep single-group pages flat. Reworked to stay in the enlarged modal instead of the settings tab. Co-authored-by: Lukas Brübach --- .../widgets/dialogs/settings_view.cpp | 102 +++++++++++++++--- .../interface/widgets/dialogs/settings_view.h | 2 + .../settings_page/abstract_settings_page.cpp | 24 +++++ .../settings_page/abstract_settings_page.h | 17 +++ 4 files changed, 132 insertions(+), 13 deletions(-) diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp index 483aa31bf..3feda61b5 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp @@ -54,6 +54,7 @@ #include #include #include +#include #include #include #include @@ -88,6 +89,33 @@ static QStringList pageIconResources() QStringLiteral("theme:config/sound"), QStringLiteral("theme:config/shorcuts")}; } +namespace +{ +/// Item data role holding a sub-page row's section widget, stored as a WidgetId +constexpr int SectionWidgetRole = Qt::UserRole + 1; +using WidgetId = quintptr; + +/** + * @brief Packs a section widget for storage in a tree item's data role + * @param widget The section's group box (owned by its settings page) + * @return The widget as an opaque id + */ +WidgetId toWidgetId(QWidget *widget) +{ + return reinterpret_cast(widget); +} + +/** + * @brief Unpacks a section widget from a tree item's data role + * @param id The stored id, or 0 for page rows without a section + * @return The section widget, or nullptr when unset + */ +QWidget *fromWidgetId(WidgetId id) +{ + return reinterpret_cast(id); +} +} // namespace + SettingsView::SettingsView(QWidget *parent) : QWidget(parent), currentTabIndex(0), searchActive(false) { setupUi(); @@ -110,9 +138,6 @@ void SettingsView::setupUi() auto *searchLayout = new QHBoxLayout; searchLayout->addWidget(searchEdit); - // Navigation tree (built in setupNavTree) - setupNavTree(); - // Pages stacked widget pagesWidget = new QStackedWidget; @@ -145,6 +170,9 @@ void SettingsView::setupUi() Q_ASSERT(pages.size() == NumPages); + // Navigation tree (needs the pages for their sections) + setupNavTree(); + // Search results view (hidden by default) searchResultsView = new QListView; searchResultsView->setUniformItemSizes(false); @@ -246,6 +274,12 @@ void SettingsView::setupNavTree() item->setIcon(0, QPixmap(iconResources[page])); item->setData(0, Qt::UserRole, static_cast(page)); pageItems[page] = item; + // Sub-page rows deep-link into the page's sections (scroll + highlight) + for (const SettingsPageSection §ion : pages[page]->getSections()) { + auto *sectionItem = new QTreeWidgetItem(item, {section.title}); + sectionItem->setData(0, Qt::UserRole, static_cast(page)); + sectionItem->setData(0, SectionWidgetRole, QVariant::fromValue(toWidgetId(section.sectionWidget))); + } } } @@ -258,7 +292,12 @@ void SettingsView::setupNavTree() } bool ok = false; int index = current->data(0, Qt::UserRole).toInt(&ok); - if (ok) { + if (!ok) { + return; + } + if (QWidget *section = fromWidgetId(current->data(0, SectionWidgetRole).value())) { + navigateToSection(index, section); + } else { onTabClicked(index); } }); @@ -409,19 +448,44 @@ void SettingsView::navigateToSearchResult(const QModelIndex &index) // Scroll to the widget, focus it, and flash to highlight it if (entry.widget) { - QWidget *widget = entry.widget; - while (widget) { - if (auto *scrollArea = qobject_cast(widget)) { - scrollArea->ensureWidgetVisible(entry.widget); - break; - } - widget = widget->parentWidget(); - } + scrollToAndFlash(entry.widget); entry.widget->setFocus(); - flashWidget(entry.widget); } } +void SettingsView::navigateToSection(int pageIndex, QWidget *sectionWidget) +{ + if (pageIndex < 0 || pageIndex >= pages.size() || !sectionWidget) { + return; + } + + if (currentTabIndex != pageIndex) { + if (searchActive) { + switchToTabMode(); + } + setActiveTab(pageIndex); + } + + // Focus stays in the navigation tree; only scroll and highlight the section + scrollToAndFlash(sectionWidget); +} + +void SettingsView::scrollToAndFlash(QWidget *target) +{ + if (!target) { + return; + } + QWidget *widget = target; + while (widget) { + if (auto *scrollArea = qobject_cast(widget)) { + scrollArea->ensureWidgetVisible(target); + break; + } + widget = widget->parentWidget(); + } + flashWidget(target); +} + void SettingsView::setTab(int index) { if (index >= 0 && index < pageItems.size()) { @@ -515,6 +579,18 @@ void SettingsView::retranslateNavNames() pageItems[i]->setText(0, pageNames[i]); } } + + // Sub-page rows mirror the pages' group box titles (already retranslated by the pages) + for (int i = 0; i < pageItems.size() && i < pages.size(); ++i) { + if (!pageItems[i]) { + continue; + } + const QList sections = pages[i]->getSections(); + Q_ASSERT(pageItems[i]->childCount() == sections.size()); + for (int c = 0; c < pageItems[i]->childCount() && c < sections.size(); ++c) { + pageItems[i]->child(c)->setText(0, sections[c].title); + } + } } QStringList SettingsView::translatedGroupNames() diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.h b/cockatrice/src/interface/widgets/dialogs/settings_view.h index 214353660..8d188bf51 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.h +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.h @@ -86,8 +86,10 @@ private: void switchToTabMode(); void switchToSearchMode(); void navigateToSearchResult(const QModelIndex &index); + void navigateToSection(int pageIndex, QWidget *sectionWidget); void setActiveTab(int index); static void flashWidget(QWidget *widget); + static void scrollToAndFlash(QWidget *target); static QStringList translatedPageNames(); static QStringList translatedGroupNames(); diff --git a/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.cpp index 9c43d1f7b..063f1b2a8 100644 --- a/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.cpp @@ -137,6 +137,30 @@ QList AbstractSettingsPage::getSearchEntries() return autoDetectSearchEntries(this, -1); } +QList AbstractSettingsPage::getSections() +{ + QList sections; + + // Walk the top-level layout in display order (creation order differs on some pages) + if (QLayout *mainLayout = layout()) { + for (int i = 0; i < mainLayout->count(); ++i) { + QLayoutItem *item = mainLayout->itemAt(i); + QWidget *widget = item ? item->widget() : nullptr; + auto *groupBox = qobject_cast(widget); + if (!groupBox || groupBox->title().isEmpty()) { + continue; + } + sections.append({groupBox->title(), groupBox}); + } + } + + // A single group is the page itself; sub-page entries would only duplicate it + if (sections.size() < 2) { + sections.clear(); + } + return sections; +} + QList AbstractSettingsPage::autoDetectSearchEntries(QWidget *page, int pageIndex) { QList entries; diff --git a/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.h b/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.h index 722a91e62..c38a5214b 100644 --- a/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/abstract_settings_page.h @@ -2,6 +2,7 @@ #define COCKATRICE_ABSTRACT_SETTINGS_PAGE_H #include +#include #include #include @@ -11,12 +12,28 @@ struct SettingsSearchEntry; +/** + * @brief A navigable section of a settings page (one group box) + */ +struct SettingsPageSection +{ + QString title; ///< Translated group title + QWidget *sectionWidget = nullptr; ///< Group box to scroll to and highlight +}; + class AbstractSettingsPage : public QWidget { Q_OBJECT public: virtual void retranslateUi() = 0; virtual QList getSearchEntries(); + /** + * @brief Sections of this page for sub-page navigation, in display order + * + * Scans the page's direct-child group boxes; pages with a single group + * report no sections so the nav stays flat for them. + */ + QList getSections(); protected: static QList autoDetectSearchEntries(QWidget *page, int pageIndex); From 43435445881a846de0561f4be24cb726ef983393 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:48:52 +0200 Subject: [PATCH 6/8] [Client] Settings: isolate the selected section when drilling down (#7488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Settings: isolate the selected section when drilling down Show only the selected section's group box. Restore the overview on page rows. Filter search results to the control's containing section. Reset the scroll position on every change. Reworked to stay in the enlarged modal instead of the settings tab. * [Client] Settings: re-clicking an isolated section restores the overview * [Settings] Keep section row selected when drilling down per review * [Settings] Stop stealing nav selection mid-gesture per review * [Settings] Sample nav selection pre-press so re-click detection works per review * Don't flash the section on drill-down * [Client] Settings: clear the section filter when selecting the page row per review onTabClicked() returned early for the already-current page before switchPageContent() could call applySectionFilter(index, nullptr), so clicking the page row while a section was isolated moved the selection but left the page filtered. Only skip the restore when no section is isolated. * [Settings] Select the search result's section in the sidebar per review Reaching a setting through the search isolated its section and flashed the widget, but left the navigation selection on the page row, so the sidebar looked like it had only jumped to the section's page. Extract the section-row selection shared with drill-down into selectSectionRow() and use it for search results too, keeping the gesture/re-click behavior untouched. --------- Co-authored-by: Lukas Brübach --- .../widgets/dialogs/settings_view.cpp | 177 +++++++++++++++++- .../interface/widgets/dialogs/settings_view.h | 6 + 2 files changed, 175 insertions(+), 8 deletions(-) diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp index 3feda61b5..ebac4e22f 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.cpp +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -251,6 +253,13 @@ void SettingsView::setupNavTree() navTree->setStyleSheet( "QTreeWidget::item:selected { background: palette(highlight); color: palette(highlighted-text); }"); + // Re-click detection has to sample the selection before the view handles the + // press: QAbstractItemView updates the current item (emitting + // currentItemChanged) and only then emits itemPressed, so itemPressed always + // sees the pressed item as "current" and cannot tell a re-click from a first + // click. The viewport event filter runs before QAbstractItemView::mousePressEvent. + navTree->viewport()->installEventFilter(this); + // Groups of SettingsPage values, in navigation order static const QList> groupedPages = { {GeneralPage, AppearancePage, UserInterfacePage}, @@ -301,6 +310,26 @@ void SettingsView::setupNavTree() onTabClicked(index); } }); + + connect(navTree, &QTreeWidget::itemClicked, this, [this](QTreeWidgetItem *item) { + const bool wasReClick = pressWasOnCurrentItem; + pressWasOnCurrentItem = false; + // A fresh click already navigated via currentItemChanged; only a press that + // started on the already-current item (captured by the viewport event filter + // before the view updated the selection) is a re-click. Re-clicking the + // isolated section restores the full page. + if (!wasReClick || item == nullptr || item != navTree->currentItem() || isolatedSection == nullptr || + fromWidgetId(item->data(0, SectionWidgetRole).value()) != isolatedSection) { + return; + } + bool ok = false; + const int index = item->data(0, Qt::UserRole).toInt(&ok); + if (ok) { + // setActiveTab, not onTabClicked: the page is already current and + // only setActiveTab clears the section filter + setActiveTab(index); + } + }); } void SettingsView::buildSearchIndex() @@ -328,7 +357,9 @@ void SettingsView::onTabClicked(int index) { if (searchActive) { switchToTabMode(); - } else if (index == currentTabIndex) { + } else if (index == currentTabIndex && isolatedSection == nullptr) { + // Re-selecting the current page row is a no-op, but it must still clear + // an isolated section so the page's widgets come back. return; } setActiveTab(index); @@ -340,15 +371,28 @@ void SettingsView::setActiveTab(int index) return; } - currentTabIndex = index; - pagesWidget->setCurrentIndex(index); + switchPageContent(index); // Selecting the item also expands a collapsed group; selecting the already // current item emits nothing, so the currentItemChanged handler cannot loop. + navTree->setCurrentItem(pageItems[index]); +} + +void SettingsView::switchPageContent(int index) +{ + if (index < 0 || index >= pageItems.size() || !pageItems[index]) { + return; + } + + currentTabIndex = index; + pagesWidget->setCurrentIndex(index); + + // A page row always shows the full page; sections filter down from here + applySectionFilter(index, nullptr); + if (QTreeWidgetItem *parent = pageItems[index]->parent()) { parent->setExpanded(true); } - navTree->setCurrentItem(pageItems[index]); } void SettingsView::flashWidget(QWidget *widget) @@ -446,8 +490,19 @@ void SettingsView::navigateToSearchResult(const QModelIndex &index) switchToTabMode(); setActiveTab(entry.pageIndex); - // Scroll to the widget, focus it, and flash to highlight it + // Isolate the control's section, then scroll to the widget, focus it, and flash it if (entry.widget) { + QWidget *section = nullptr; + for (const SettingsPageSection &candidate : pages[entry.pageIndex]->getSections()) { + if (candidate.sectionWidget == entry.widget || + (candidate.sectionWidget && candidate.sectionWidget->isAncestorOf(entry.widget))) { + section = candidate.sectionWidget; + break; + } + } + applySectionFilter(entry.pageIndex, section); + // Keep the sidebar in sync with the deep link: highlight the setting's section row + selectSectionRow(entry.pageIndex, section); scrollToAndFlash(entry.widget); entry.widget->setFocus(); } @@ -459,15 +514,110 @@ void SettingsView::navigateToSection(int pageIndex, QWidget *sectionWidget) return; } + // Never steal the view's own selection mid-gesture: when this runs from + // currentItemChanged the section row is already current (mouse press or arrow key), + // and re-selecting the page row here is what made the selection visibly snap back + // to the parent on mouse release. selectSectionRow only acts on a known section + // that is not current yet, so it syncs programmatic callers without disturbing a gesture. if (currentTabIndex != pageIndex) { if (searchActive) { switchToTabMode(); } - setActiveTab(pageIndex); + const QSignalBlocker blocker(navTree); + switchPageContent(pageIndex); + } + applySectionFilter(pageIndex, sectionWidget); + + // Unknown widgets show the full page; known sections keep their row selected + selectSectionRow(pageIndex, sectionWidget); +} + +/** + * @brief Selects the nav row for a section, expanding its page row + * + * A null or unknown section (a page whose sections are its own group boxes) leaves + * the current selection untouched, so callers can pass a lookup result directly. + * + * @param pageIndex SettingsPage owning the section + * @param sectionWidget Section's group box, or nullptr for no section + */ +void SettingsView::selectSectionRow(int pageIndex, QWidget *sectionWidget) +{ + if (!sectionWidget) { + return; } - // Focus stays in the navigation tree; only scroll and highlight the section - scrollToAndFlash(sectionWidget); + QTreeWidgetItem *sectionItem = findSectionItem(pageIndex, sectionWidget); + if (!sectionItem) { + return; + } + + pageItems[pageIndex]->setExpanded(true); + if (navTree->currentItem() != sectionItem) { + const QSignalBlocker blocker(navTree); + navTree->setCurrentItem(sectionItem); + } +} + +/** + * @brief Finds the nav row for a section widget + * @param pageIndex SettingsPage owning the section + * @param sectionWidget Section's group box + * @return The section's tree item, or nullptr when it has no row + */ +QTreeWidgetItem *SettingsView::findSectionItem(int pageIndex, QWidget *sectionWidget) const +{ + if (pageIndex < 0 || pageIndex >= pageItems.size() || !pageItems[pageIndex]) { + return nullptr; + } + for (int c = 0; c < pageItems[pageIndex]->childCount(); ++c) { + QTreeWidgetItem *child = pageItems[pageIndex]->child(c); + if (fromWidgetId(child->data(0, SectionWidgetRole).value()) == sectionWidget) { + return child; + } + } + return nullptr; +} + +/** + * @brief Shows only the given section of a page, hiding its siblings + * + * A null sectionWidget restores the full page. Unknown widgets fall back to + * the full page so a stale reference can never leave a blank page. + * + * @param pageIndex SettingsPage to filter + * @param sectionWidget Section's group box, or nullptr to show all sections + */ +void SettingsView::applySectionFilter(int pageIndex, QWidget *sectionWidget) +{ + if (pageIndex < 0 || pageIndex >= pages.size()) { + return; + } + + const QList sections = pages[pageIndex]->getSections(); + + bool known = sectionWidget == nullptr; + for (const SettingsPageSection §ion : sections) { + if (section.sectionWidget == sectionWidget) { + known = true; + } + } + if (!known) { + sectionWidget = nullptr; + } + isolatedSection = sectionWidget; + + for (const SettingsPageSection §ion : sections) { + if (section.sectionWidget) { + section.sectionWidget->setVisible(sectionWidget == nullptr || section.sectionWidget == sectionWidget); + } + } + + if (auto *scrollArea = qobject_cast(pagesWidget->widget(pageIndex))) { + if (QScrollBar *scrollBar = scrollArea->verticalScrollBar()) { + scrollBar->setValue(0); + } + } } void SettingsView::scrollToAndFlash(QWidget *target) @@ -500,6 +650,17 @@ AbstractSettingsPage *SettingsView::page(SettingsPage which) const bool SettingsView::eventFilter(QObject *watched, QEvent *event) { + if (watched == navTree->viewport()) { + if (event->type() == QEvent::MouseButtonPress) { + auto *mouseEvent = static_cast(event); + // Left button only: context presses must never toggle the filter + QTreeWidgetItem *pressedItem = navTree->itemAt(mouseEvent->position().toPoint()); + pressWasOnCurrentItem = mouseEvent->button() == Qt::LeftButton && pressedItem != nullptr && + pressedItem == navTree->currentItem(); + } + return false; // observation only: the view still handles the press + } + if (watched == searchEdit && event->type() == QEvent::KeyPress) { auto *keyEvent = static_cast(event); if (keyEvent->key() == Qt::Key_Escape) { diff --git a/cockatrice/src/interface/widgets/dialogs/settings_view.h b/cockatrice/src/interface/widgets/dialogs/settings_view.h index 8d188bf51..90dc5636f 100644 --- a/cockatrice/src/interface/widgets/dialogs/settings_view.h +++ b/cockatrice/src/interface/widgets/dialogs/settings_view.h @@ -79,6 +79,8 @@ private: SettingsSearchDelegate *searchDelegate; ///< Delegate for search result rendering int currentTabIndex; ///< Currently active tab index bool searchActive; ///< Whether search mode is active + QWidget *isolatedSection = nullptr; ///< Isolated section group box, or nullptr for full pages + bool pressWasOnCurrentItem = false; ///< True when the mouse press started on the already-current item void setupUi(); void setupNavTree(); @@ -88,6 +90,10 @@ private: void navigateToSearchResult(const QModelIndex &index); void navigateToSection(int pageIndex, QWidget *sectionWidget); void setActiveTab(int index); + void switchPageContent(int index); + void applySectionFilter(int pageIndex, QWidget *sectionWidget); + void selectSectionRow(int pageIndex, QWidget *sectionWidget); + QTreeWidgetItem *findSectionItem(int pageIndex, QWidget *sectionWidget) const; static void flashWidget(QWidget *widget); static void scrollToAndFlash(QWidget *target); static QStringList translatedPageNames(); From 46cdbac830b69584bdc3bb61998c401c5eb28d43 Mon Sep 17 00:00:00 2001 From: Zach H Date: Sun, 18 Oct 2026 16:50:04 -0400 Subject: [PATCH 7/8] [Servatrice] Match trusted_sources by address, not substring (#7582) trusted_sources was checked with QString::contains. A dual-stack listener reports a local proxy as ::ffff:127.0.0.1, which never matches the default "127.0.0.1,::1". Behind nginx the forwarded client IP header was then ignored, every websocket user shared the proxy's address, and logins failed with "too many concurrent connections" once max_users_per_address was reached. Add isTrustedSource(), which parses each entry as a QHostAddress and compares exact addresses with IPv4-mapped addresses normalized, and use it at all four call sites. The ban command also read server/trusted_sources instead of the documented security/trusted_sources; it now uses the same getter. Entries that are not IP addresses are logged at startup since they no longer match anything. Co-authored-by: Claude Opus 5.5 --- servatrice/CMakeLists.txt | 1 + servatrice/servatrice.ini.example | 3 +- servatrice/src/servatrice.cpp | 17 ++++++- servatrice/src/servatrice.h | 2 + servatrice/src/serversocketinterface.cpp | 10 ++-- servatrice/src/trusted_sources.cpp | 23 ++++++++++ servatrice/src/trusted_sources.h | 20 ++++++++ tests/CMakeLists.txt | 11 +++++ tests/trusted_sources_test.cpp | 58 ++++++++++++++++++++++++ 9 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 servatrice/src/trusted_sources.cpp create mode 100644 servatrice/src/trusted_sources.h create mode 100644 tests/trusted_sources_test.cpp diff --git a/servatrice/CMakeLists.txt b/servatrice/CMakeLists.txt index f6d28d6c1..4db1683d0 100644 --- a/servatrice/CMakeLists.txt +++ b/servatrice/CMakeLists.txt @@ -18,6 +18,7 @@ set(servatrice_SOURCES src/settingscache.cpp src/isl_interface.cpp src/signalhandler.cpp + src/trusted_sources.cpp ${VERSION_STRING_CPP} src/smtpclient.cpp src/smtp/qxthmac.cpp diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 68478810b..63f8864ba 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -465,7 +465,8 @@ legacy_backup_ttl_days=180 ; You may want to allow an unlimited number of users from a trusted source. This setting can contain a ; comma-separed list of IP addresses which will allow an unlimited number of connections from each of the -; IP addresses listed (ignoring the max_users_per_address). Default is "127.0.0.1,::1"; example: "192.73.233.244,81.4.100.74" +; IP addresses listed (ignoring the max_users_per_address). Entries must be exact IP addresses; hostnames, prefixes +; and subnets are ignored. Default is "127.0.0.1,::1"; example: "192.73.233.244,81.4.100.74" trusted_sources="127.0.0.1,::1" ; Servatrice can avoid users from flooding rooms with large number of messages in an interval of time. diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index a306de523..77ddff1fc 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -33,6 +33,7 @@ #include "serversocketinterface.h" #include "settingscache.h" #include "smtpclient.h" +#include "trusted_sources.h" #include #include @@ -324,6 +325,14 @@ bool Servatrice::initServer() qDebug() << "Maximum websocket user limit:" << getMaxWebSocketUserLimit(); } + // trusted_sources entries are matched as exact addresses; older configs may hold hostnames, + // prefixes or subnets that substring matching used to accept and that now never match. + for (const QString &entry : getTrustedSources().split(',', Qt::SkipEmptyParts)) { + if (QHostAddress(entry.trimmed()).isNull()) { + qWarning() << "security/trusted_sources entry is not an IP address and will be ignored:" << entry.trimmed(); + } + } + qDebug() << "Accept registered users only:" << getRegOnlyServerEnabled(); qDebug() << "Registration enabled:" << getRegistrationEnabled(); if (getRegistrationEnabled()) { @@ -1324,6 +1333,11 @@ int Servatrice::getMaxUsersPerAddress() const return settingsCache->value("security/max_users_per_address", 4).toInt(); } +QString Servatrice::getTrustedSources() const +{ + return settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString(); +} + int Servatrice::getMessageCountingInterval() const { return settingsCache->value("security/message_counting_interval", 10).toInt(); @@ -1538,8 +1552,7 @@ bool Servatrice::isRateLimitExempt(const QString &ipAddress) const // Mirrors the max_users_per_address bypass in initSession: trusted sources // (e.g. 127.0.0.1 for Webatrice behind local nginx, where every websocket // client shares one address) are not subject to per-address limits. - const QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString(); - return trustedSources.contains(ipAddress, Qt::CaseInsensitive); + return isTrustedSource(getTrustedSources(), ipAddress); } void Servatrice::clearFailedLogins(const QString &ipAddress, const QString &userName) diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index 119adc8b2..3dba23234 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -321,6 +321,8 @@ public: int getDeckShareMaxDecksPerShare() const; int getDeckShareMaxSharesPerDay() const; int getMaxUsersPerAddress() const; + /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). + QString getTrustedSources() const; int getMessageCountingInterval() const override; int getMaxMessageCountPerInterval() const override; int getMaxMessageSizePerInterval() const override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index b24126d93..f8b84b6c8 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -42,6 +42,7 @@ #include "server_logger.h" #include "server_protocolhandler.h" #include "settingscache.h" +#include "trusted_sources.h" #include "version_string.h" #include @@ -220,8 +221,7 @@ bool AbstractServerSocketInterface::initSession() delete identSe; // allow unlimited number of connections from the trusted sources - QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString(); - if (trustedSources.contains(getAddress(), Qt::CaseInsensitive)) { + if (isTrustedSource(servatrice->getTrustedSources(), getPeerAddress())) { return true; } @@ -1908,9 +1908,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com if (amountRemove != 0) { removeSaidMessages(userName, amountRemove); } - QString trustedSources = settingsCache->value("server/trusted_sources", "127.0.0.1,::1").toString(); int minutes = cmd.minutes(); - if (trustedSources.contains(address, Qt::CaseInsensitive)) { + if (isTrustedSource(servatrice->getTrustedSources(), address)) { address = ""; } @@ -4560,8 +4559,7 @@ void WebsocketServerSocketInterface::initConnection(void *_socket) if (websocketIPHeader.length() > 0 && socket->request().hasRawHeader(websocketIPHeader)) { // Only trust the forwarded header if the connection itself comes from a trusted proxy, // otherwise a client could spoof its address to bypass bans and rate limits. - QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString(); - if (trustedSources.contains(socket->peerAddress().toString(), Qt::CaseInsensitive)) { + if (isTrustedSource(servatrice->getTrustedSources(), socket->peerAddress())) { QString header(socket->request().rawHeader(websocketIPHeader)); QHostAddress parsed(header); if (!parsed.isNull()) { diff --git a/servatrice/src/trusted_sources.cpp b/servatrice/src/trusted_sources.cpp new file mode 100644 index 000000000..327491936 --- /dev/null +++ b/servatrice/src/trusted_sources.cpp @@ -0,0 +1,23 @@ +#include "trusted_sources.h" + +#include + +bool isTrustedSource(const QString &trustedSources, const QHostAddress &address) +{ + if (address.isNull()) { + return false; + } + const QStringList entries = trustedSources.split(',', Qt::SkipEmptyParts); + for (const QString &entry : entries) { + const QHostAddress trusted(entry.trimmed()); + if (!trusted.isNull() && trusted.isEqual(address, QHostAddress::ConvertV4MappedToIPv4)) { + return true; + } + } + return false; +} + +bool isTrustedSource(const QString &trustedSources, const QString &address) +{ + return isTrustedSource(trustedSources, QHostAddress(address.trimmed())); +} diff --git a/servatrice/src/trusted_sources.h b/servatrice/src/trusted_sources.h new file mode 100644 index 000000000..3aea55e41 --- /dev/null +++ b/servatrice/src/trusted_sources.h @@ -0,0 +1,20 @@ +#ifndef TRUSTED_SOURCES_H +#define TRUSTED_SOURCES_H + +#include +#include + +/** + * @brief True if @p address is listed in the comma-separated @p trustedSources (security/trusted_sources). + * + * Entries are parsed as addresses and compared exactly, never as substrings, so + * 27.0.0.1 does not match 127.0.0.1. IPv4-mapped IPv6 addresses compare equal to + * their IPv4 form: a dual-stack listener reports a local proxy as ::ffff:127.0.0.1, + * which matches a 127.0.0.1 entry. Malformed entries and null addresses never match. + */ +bool isTrustedSource(const QString &trustedSources, const QHostAddress &address); + +/** @brief Overload for an address in string form; an unparsable string never matches. */ +bool isTrustedSource(const QString &trustedSources, const QString &address); + +#endif diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b58942f3f..dfc2cb3f3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -208,6 +208,17 @@ cockatrice_add_test( LABELS server ) +cockatrice_add_test( + NAME + trusted_sources_test + SOURCES + ${CMAKE_SOURCE_DIR}/servatrice/src/trusted_sources.cpp + trusted_sources_test.cpp + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/servatrice/src + LABELS + server +) cockatrice_add_test( NAME server_card_counter_test diff --git a/tests/trusted_sources_test.cpp b/tests/trusted_sources_test.cpp new file mode 100644 index 000000000..a16b21f2f --- /dev/null +++ b/tests/trusted_sources_test.cpp @@ -0,0 +1,58 @@ +#include "trusted_sources.h" + +#include "gtest/gtest.h" +#include + +namespace +{ + +const QString kDefault = QStringLiteral("127.0.0.1,::1"); + +TEST(TrustedSourcesTest, MatchesListedAddresses) +{ + ASSERT_TRUE(isTrustedSource(kDefault, QHostAddress("127.0.0.1"))); + ASSERT_TRUE(isTrustedSource(kDefault, QHostAddress("::1"))); + ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress("10.0.0.1"))); +} + +TEST(TrustedSourcesTest, MatchesIpv4MappedPeerAgainstPlainIpv4Entry) +{ + // A dual-stack listener reports a local IPv4 proxy as ::ffff:127.0.0.1; + // it must still match the default 127.0.0.1 entry. + ASSERT_TRUE(isTrustedSource(kDefault, QHostAddress("::ffff:127.0.0.1"))); +} + +TEST(TrustedSourcesTest, MatchesPlainIpv4PeerAgainstMappedEntry) +{ + ASSERT_TRUE(isTrustedSource("::ffff:192.0.2.7", QHostAddress("192.0.2.7"))); +} + +TEST(TrustedSourcesTest, DoesNotMatchSubstrings) +{ + // Substring matching would let 27.0.0.1 or 0.0.1 pass as 127.0.0.1. + ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress("27.0.0.1"))); + ASSERT_FALSE(isTrustedSource("192.0.2.10", QHostAddress("192.0.2.1"))); + ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress(":"))); +} + +TEST(TrustedSourcesTest, ToleratesWhitespaceAndCase) +{ + ASSERT_TRUE(isTrustedSource(" 192.0.2.7 , 2001:DB8::1 ", QHostAddress("2001:db8::1"))); + ASSERT_TRUE(isTrustedSource(" 192.0.2.7 , 2001:DB8::1 ", QHostAddress("192.0.2.7"))); +} + +TEST(TrustedSourcesTest, IgnoresMalformedEntriesAndNullAddresses) +{ + ASSERT_FALSE(isTrustedSource("not-an-ip,,", QHostAddress("127.0.0.1"))); + ASSERT_FALSE(isTrustedSource("", QHostAddress("127.0.0.1"))); + ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress())); +} + +TEST(TrustedSourcesTest, StringOverloadParsesAddress) +{ + ASSERT_TRUE(isTrustedSource(kDefault, QStringLiteral("::ffff:127.0.0.1"))); + ASSERT_FALSE(isTrustedSource(kDefault, QStringLiteral("27.0.0.1"))); + ASSERT_FALSE(isTrustedSource(kDefault, QString())); +} + +} // namespace From ea6af0048cbd6a8f07c1d68c271d3d0ebec593cd Mon Sep 17 00:00:00 2001 From: Zach H Date: Sun, 18 Oct 2026 16:53:55 -0400 Subject: [PATCH 8/8] [Server] Parse the rightmost address from forwarded header lists (#7586) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Forwarded-For carries a comma-separated list, and constructing a QHostAddress from the raw header value returns a null address for it, so a configured X-Forwarded-For or a multi-hop header never resolved and websocket clients stayed on the proxy's address. - Add parseForwardedAddress, returning the rightmost usable entry of a forwarded header list - Use it for the websocket handshake instead of parsing the raw value - Exempt loopback from per-address auth rate limits so a same-host proxy with a customized trusted_sources list cannot lock every user out - Add forwarded_address_test Co-authored-by: Lukas Brübach --- servatrice/CMakeLists.txt | 1 + servatrice/servatrice.ini.example | 8 +++-- servatrice/src/forwarded_address.cpp | 21 ++++++++++++++ servatrice/src/forwarded_address.h | 18 ++++++++++++ servatrice/src/servatrice.cpp | 7 +++-- servatrice/src/serversocketinterface.cpp | 4 +-- tests/CMakeLists.txt | 11 +++++++ tests/forwarded_address_test.cpp | 37 ++++++++++++++++++++++++ 8 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 servatrice/src/forwarded_address.cpp create mode 100644 servatrice/src/forwarded_address.h create mode 100644 tests/forwarded_address_test.cpp diff --git a/servatrice/CMakeLists.txt b/servatrice/CMakeLists.txt index 4db1683d0..255469934 100644 --- a/servatrice/CMakeLists.txt +++ b/servatrice/CMakeLists.txt @@ -7,6 +7,7 @@ project(Servatrice VERSION "${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${ set(servatrice_SOURCES src/email_parser.cpp src/event_loop_watchdog.cpp + src/forwarded_address.cpp src/main.cpp src/metrics_registry.cpp src/ratelimiter.cpp diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 63f8864ba..021f5157f 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -42,9 +42,11 @@ websocket_host=any websocket_port=4748 ; If websockets are served through a reverse proxy (e.g. nginx), the real client address can be -; read from an HTTP header such as X-Forwarded-For. The header is ONLY trusted when the websocket -; connection itself originates from an address listed in security/trusted_sources (default 127.0.0.1,::1), -; so that clients cannot spoof their address to bypass bans or rate limits. +; read from an HTTP header such as X-Real-IP or X-Forwarded-For. A comma-separated X-Forwarded-For +; list resolves to its rightmost entry, which is the one the proxy appended. The header is ONLY +; trusted when the websocket connection itself originates from an address listed in +; security/trusted_sources (default 127.0.0.1,::1), so that clients cannot spoof their address to +; bypass bans or rate limits. web_socket_ip_header= ; When database is enabled, servatrice writes the server status in the "update" database table; this diff --git a/servatrice/src/forwarded_address.cpp b/servatrice/src/forwarded_address.cpp new file mode 100644 index 000000000..8d5d3f4c1 --- /dev/null +++ b/servatrice/src/forwarded_address.cpp @@ -0,0 +1,21 @@ +#include "forwarded_address.h" + +#include + +QHostAddress parseForwardedAddress(const QByteArray &headerValue) +{ + const QList entries = headerValue.split(','); + for (auto it = entries.crbegin(); it != entries.crend(); ++it) { + const QByteArray entry = it->trimmed(); + if (entry.isEmpty()) { + continue; + } + + const QHostAddress parsed(QString::fromLatin1(entry)); + if (!parsed.isNull()) { + return parsed; + } + } + + return {}; +} diff --git a/servatrice/src/forwarded_address.h b/servatrice/src/forwarded_address.h new file mode 100644 index 000000000..d77e041bd --- /dev/null +++ b/servatrice/src/forwarded_address.h @@ -0,0 +1,18 @@ +#ifndef FORWARDED_ADDRESS_H +#define FORWARDED_ADDRESS_H + +#include +#include + +/** + * @brief Parses the real client address out of a forwarded header value (X-Real-IP, X-Forwarded-For). + * + * A comma-separated list resolves to its rightmost usable entry. That entry was appended by the + * proxy closest to the server and is the only one a direct client cannot choose, since the earlier + * entries are passed through verbatim from whatever the client sent. + * + * @return the parsed address, or a null QHostAddress when the value holds none. + */ +QHostAddress parseForwardedAddress(const QByteArray &headerValue); + +#endif diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index 77ddff1fc..b6eaf11f4 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -1551,8 +1551,11 @@ bool Servatrice::isRateLimitExempt(const QString &ipAddress) const { // Mirrors the max_users_per_address bypass in initSession: trusted sources // (e.g. 127.0.0.1 for Webatrice behind local nginx, where every websocket - // client shares one address) are not subject to per-address limits. - return isTrustedSource(getTrustedSources(), ipAddress); + // client shares one address) are not subject to per-address limits. Loopback + // is always exempt, so a same-host proxy whose forwarded address could not be + // resolved cannot lock every user out together when trusted_sources is + // customized. + return QHostAddress(ipAddress).isLoopback() || isTrustedSource(getTrustedSources(), ipAddress); } void Servatrice::clearFailedLogins(const QString &ipAddress, const QString &userName) diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index f8b84b6c8..890a93fbc 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -22,6 +22,7 @@ #include "deck_tag_serialization.h" #include "email_parser.h" +#include "forwarded_address.h" #include "libcockatrice/protocol/pb/admin_commands.pb.h" #include "libcockatrice/protocol/pb/developer_commands.pb.h" #include "libcockatrice/protocol/pb/moderator_commands.pb.h" @@ -4560,8 +4561,7 @@ void WebsocketServerSocketInterface::initConnection(void *_socket) // Only trust the forwarded header if the connection itself comes from a trusted proxy, // otherwise a client could spoof its address to bypass bans and rate limits. if (isTrustedSource(servatrice->getTrustedSources(), socket->peerAddress())) { - QString header(socket->request().rawHeader(websocketIPHeader)); - QHostAddress parsed(header); + const QHostAddress parsed = parseForwardedAddress(socket->request().rawHeader(websocketIPHeader)); if (!parsed.isNull()) { address = parsed; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index dfc2cb3f3..8811b8623 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -208,6 +208,17 @@ cockatrice_add_test( LABELS server ) +cockatrice_add_test( + NAME + forwarded_address_test + SOURCES + ${CMAKE_SOURCE_DIR}/servatrice/src/forwarded_address.cpp + forwarded_address_test.cpp + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/servatrice/src + LABELS + server +) cockatrice_add_test( NAME trusted_sources_test diff --git a/tests/forwarded_address_test.cpp b/tests/forwarded_address_test.cpp new file mode 100644 index 000000000..3ae2bc471 --- /dev/null +++ b/tests/forwarded_address_test.cpp @@ -0,0 +1,37 @@ +#include "forwarded_address.h" + +#include "gtest/gtest.h" + +namespace +{ + +TEST(ForwardedAddressTest, ParsesSingleAddress) +{ + EXPECT_EQ(QHostAddress("203.0.113.7"), parseForwardedAddress("203.0.113.7")); + EXPECT_EQ(QHostAddress("::1"), parseForwardedAddress("::1")); + EXPECT_EQ(QHostAddress("203.0.113.7"), parseForwardedAddress(" 203.0.113.7 ")); +} + +TEST(ForwardedAddressTest, UsesRightmostEntryOfAList) +{ + // nginx's $proxy_add_x_forwarded_for appends its own view of the client to + // whatever the client sent, so only the trailing entry is trustworthy. + EXPECT_EQ(QHostAddress("203.0.113.7"), parseForwardedAddress("198.51.100.9, 203.0.113.7")); + EXPECT_EQ(QHostAddress("203.0.113.7"), parseForwardedAddress("spoofed.invalid, 203.0.113.7")); + EXPECT_EQ(QHostAddress("203.0.113.7"), parseForwardedAddress("203.0.113.7, ")); +} + +TEST(ForwardedAddressTest, KeepsIpv4MappedEntries) +{ + EXPECT_EQ(QHostAddress("::ffff:203.0.113.7"), parseForwardedAddress("::ffff:203.0.113.7")); +} + +TEST(ForwardedAddressTest, RejectsValuesWithoutAnAddress) +{ + EXPECT_TRUE(parseForwardedAddress("").isNull()); + EXPECT_TRUE(parseForwardedAddress(",").isNull()); + EXPECT_TRUE(parseForwardedAddress("unknown").isNull()); + EXPECT_TRUE(parseForwardedAddress("garbage, more garbage").isNull()); +} + +} // namespace