From b089a51be4c4084097d78883f0fb30db4f96067c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <44814898+BruebachL@users.noreply.github.com> Date: Sun, 8 Nov 2026 14:52:33 +0100 Subject: [PATCH] Address review findings --- .../widgets/server/game_selector.cpp | 55 +++++++------------ .../interface/widgets/server/game_selector.h | 28 +++++----- 2 files changed, 34 insertions(+), 49 deletions(-) diff --git a/cockatrice/src/interface/widgets/server/game_selector.cpp b/cockatrice/src/interface/widgets/server/game_selector.cpp index 43018a1c9..973919a15 100644 --- a/cockatrice/src/interface/widgets/server/game_selector.cpp +++ b/cockatrice/src/interface/widgets/server/game_selector.cpp @@ -177,12 +177,6 @@ GameSelector::GameSelector(AbstractClient *_client, connect(clearFilterButton, &QPushButton::clicked, this, &GameSelector::actClearFilter); connect(gameListProxyModel, &GamesProxyModel::filtersChanged, this, &GameSelector::checkClearFilterButtonState); - hostGameButton = nullptr; - hostGameMenuButton = nullptr; - hostGameButtonWidget = nullptr; - hostGameMenu = nullptr; - hostGameAction = nullptr; - reviewGameAction = nullptr; if (room) { hostGameAction = new QAction(this); hostGameAction->setShortcutContext(Qt::WidgetWithChildrenShortcut); @@ -456,6 +450,10 @@ void GameSelector::applyHostGameStyle() hostGameMenuButton->setMenu(hostGameMenu); hostGameMenuButton->setVisible(true); } + + // The tooltip depends on the control type: on macOS a click opens the menu, elsewhere it + // quick-hosts, and the appearance settings can switch between the two while the room is open. + updateHostGameToolTip(); } void GameSelector::changeEvent(QEvent *event) @@ -471,7 +469,12 @@ void GameSelector::setHostCommandPending(bool pending) hostCommandPending = pending; // A join may still be in flight: its response enables these controls again, so keep them // disabled until that response arrives instead of re-enabling on the host response alone. - const bool hostControlsEnabled = !pending && !joinCommandPending; + updateHostControlsEnabled(); +} + +void GameSelector::updateHostControlsEnabled() +{ + const bool hostControlsEnabled = !hostCommandPending && !joinCommandPending; if (hostGameAction) { hostGameAction->setEnabled(hostControlsEnabled); } @@ -701,18 +704,7 @@ bool GameSelector::joinGameById(const int gameId, const bool asSpectator) void GameSelector::disableButtons() { joinCommandPending = true; - if (hostGameAction) { - hostGameAction->setEnabled(false); - } - if (hostGameButton) { - hostGameButton->setEnabled(false); - } - if (hostGameMenuButton) { - hostGameMenuButton->setEnabled(false); - } - if (reviewGameAction) { - reviewGameAction->setEnabled(false); - } + updateHostControlsEnabled(); joinButton->setEnabled(false); spectateButton->setEnabled(false); @@ -720,19 +712,7 @@ void GameSelector::disableButtons() void GameSelector::enableButtons() { - const bool hostControlsEnabled = !hostCommandPending && !joinCommandPending; - if (hostGameAction) { - hostGameAction->setEnabled(hostControlsEnabled); - } - if (hostGameButton) { - hostGameButton->setEnabled(hostControlsEnabled); - } - if (hostGameMenuButton) { - hostGameMenuButton->setEnabled(hostControlsEnabled); - } - if (reviewGameAction) { - reviewGameAction->setEnabled(hostControlsEnabled); - } + updateHostControlsEnabled(); // Enable buttons for the currently selected game enableButtonsForIndex(gameListView->currentIndex()); @@ -794,15 +774,18 @@ void GameSelector::refreshShortcuts() void GameSelector::updateHostGameToolTip() { const QString shortcut = SettingsCache::instance().shortcuts().getShortcutString("GameSelector/aQuickHostGame"); - const QString tooltip = shortcut.isEmpty() ? tr("Host a game with your last-used settings") - : tr("Host a game with your last-used settings (%1)").arg(shortcut); + const QString quickHostTooltip = shortcut.isEmpty() + ? tr("Host a game with your last-used settings") + : tr("Host a game with your last-used settings (%1)").arg(shortcut); // The tooltip lives on the action for the shortcut and on the button for the mouse, // since a QPushButton does not inherit an action's tooltip. if (hostGameAction) { - hostGameAction->setToolTip(tooltip); + hostGameAction->setToolTip(quickHostTooltip); } if (hostGameButton) { - hostGameButton->setToolTip(tooltip); + // Under the native macOS style the button is a pull-down, so a click opens the menu + // instead of quick-hosting and the tooltip names the menu rather than the quick host. + hostGameButton->setToolTip(usesMacStyle() ? tr("Open the host menu") : quickHostTooltip); } } diff --git a/cockatrice/src/interface/widgets/server/game_selector.h b/cockatrice/src/interface/widgets/server/game_selector.h index ccd4e9fc4..727655079 100644 --- a/cockatrice/src/interface/widgets/server/game_selector.h +++ b/cockatrice/src/interface/widgets/server/game_selector.h @@ -162,19 +162,19 @@ private: GameSelectorQuickFilterToolBar *quickFilterToolBar; - QPushButton *filterButton; /**< Button to open the filter dialog. */ - QPushButton *clearFilterButton; /**< Button to clear active filters. */ - QPushButton *hostGameButton; /**< Host button, in the same family as the join buttons. */ - QToolButton *hostGameMenuButton; /**< Split arrow that opens the host menu (non-macOS styles only). */ - QWidget *hostGameButtonWidget; /**< Container that keeps the split button and its arrow together. */ - QMenu *hostGameMenu; /**< Menu behind the arrow, or behind the pull-down button on macOS. */ - QAction *hostGameAction; /**< Quick-host action, carrying the keyboard shortcut. */ - QAction *reviewGameAction; /**< Menu action that opens the full create dialog. */ - QLabel *hostErrorLabel; /**< Inline failure message for a quick-host request. */ - QPushButton *joinButton; /**< Button to join the selected game. */ - QPushButton *joinAsJudgeButton; /**< Button to join the selected game as a judge. */ - QPushButton *spectateButton; /**< Button to spectate the selected game. */ - QPushButton *joinAsJudgeSpectatorButton; /**< Button to join the selected game as a spectating judge. */ + QPushButton *filterButton; /**< Button to open the filter dialog. */ + QPushButton *clearFilterButton; /**< Button to clear active filters. */ + QPushButton *hostGameButton = nullptr; /**< Host button, in the same family as the join buttons. */ + QToolButton *hostGameMenuButton = nullptr; /**< Split arrow that opens the host menu (non-macOS styles only). */ + QWidget *hostGameButtonWidget = nullptr; /**< Container that keeps the split button and its arrow together. */ + QMenu *hostGameMenu = nullptr; /**< Menu behind the arrow, or behind the pull-down button on macOS. */ + QAction *hostGameAction = nullptr; /**< Quick-host action, carrying the keyboard shortcut. */ + QAction *reviewGameAction = nullptr; /**< Menu action that opens the full create dialog. */ + QLabel *hostErrorLabel; /**< Inline failure message for a quick-host request. */ + QPushButton *joinButton; /**< Button to join the selected game. */ + QPushButton *joinAsJudgeButton; /**< Button to join the selected game as a judge. */ + QPushButton *spectateButton; /**< Button to spectate the selected game. */ + QPushButton *joinAsJudgeSpectatorButton; /**< Button to join the selected game as a spectating judge. */ const bool showFilters; /**< Determines whether filter buttons are displayed. */ GameTypeMap gameTypeMap; /**< Mapping of game types for the current room. */ @@ -198,6 +198,8 @@ private: void applyHostGameStyle(); /** @brief Enables or disables the quick-host and review controls while a command is in flight. */ void setHostCommandPending(bool pending); + /** @brief Applies the in-flight state of both host and join commands to the quick-host and review controls. */ + void updateHostControlsEnabled(); /** @brief Keeps the quick-host button tooltip in sync with the current shortcut. */ void updateHostGameToolTip();