Address review findings

This commit is contained in:
Lukas Brübach 2026-11-08 14:52:33 +01:00 • committed by BruebachL
parent 79ecf65df6
commit b089a51be4
2 changed files with 34 additions and 49 deletions

View file

@ -177,12 +177,6 @@ GameSelector::GameSelector(AbstractClient *_client,
connect(clearFilterButton, &QPushButton::clicked, this, &GameSelector::actClearFilter); connect(clearFilterButton, &QPushButton::clicked, this, &GameSelector::actClearFilter);
connect(gameListProxyModel, &GamesProxyModel::filtersChanged, this, &GameSelector::checkClearFilterButtonState); connect(gameListProxyModel, &GamesProxyModel::filtersChanged, this, &GameSelector::checkClearFilterButtonState);
hostGameButton = nullptr;
hostGameMenuButton = nullptr;
hostGameButtonWidget = nullptr;
hostGameMenu = nullptr;
hostGameAction = nullptr;
reviewGameAction = nullptr;
if (room) { if (room) {
hostGameAction = new QAction(this); hostGameAction = new QAction(this);
hostGameAction->setShortcutContext(Qt::WidgetWithChildrenShortcut); hostGameAction->setShortcutContext(Qt::WidgetWithChildrenShortcut);
@ -456,6 +450,10 @@ void GameSelector::applyHostGameStyle()
hostGameMenuButton->setMenu(hostGameMenu); hostGameMenuButton->setMenu(hostGameMenu);
hostGameMenuButton->setVisible(true); 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) void GameSelector::changeEvent(QEvent *event)
@ -471,7 +469,12 @@ void GameSelector::setHostCommandPending(bool pending)
hostCommandPending = pending; hostCommandPending = pending;
// A join may still be in flight: its response enables these controls again, so keep them // 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. // 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) { if (hostGameAction) {
hostGameAction->setEnabled(hostControlsEnabled); hostGameAction->setEnabled(hostControlsEnabled);
} }
@ -701,18 +704,7 @@ bool GameSelector::joinGameById(const int gameId, const bool asSpectator)
void GameSelector::disableButtons() void GameSelector::disableButtons()
{ {
joinCommandPending = true; joinCommandPending = true;
if (hostGameAction) { updateHostControlsEnabled();
hostGameAction->setEnabled(false);
}
if (hostGameButton) {
hostGameButton->setEnabled(false);
}
if (hostGameMenuButton) {
hostGameMenuButton->setEnabled(false);
}
if (reviewGameAction) {
reviewGameAction->setEnabled(false);
}
joinButton->setEnabled(false); joinButton->setEnabled(false);
spectateButton->setEnabled(false); spectateButton->setEnabled(false);
@ -720,19 +712,7 @@ void GameSelector::disableButtons()
void GameSelector::enableButtons() void GameSelector::enableButtons()
{ {
const bool hostControlsEnabled = !hostCommandPending && !joinCommandPending; updateHostControlsEnabled();
if (hostGameAction) {
hostGameAction->setEnabled(hostControlsEnabled);
}
if (hostGameButton) {
hostGameButton->setEnabled(hostControlsEnabled);
}
if (hostGameMenuButton) {
hostGameMenuButton->setEnabled(hostControlsEnabled);
}
if (reviewGameAction) {
reviewGameAction->setEnabled(hostControlsEnabled);
}
// Enable buttons for the currently selected game // Enable buttons for the currently selected game
enableButtonsForIndex(gameListView->currentIndex()); enableButtonsForIndex(gameListView->currentIndex());
@ -794,15 +774,18 @@ void GameSelector::refreshShortcuts()
void GameSelector::updateHostGameToolTip() void GameSelector::updateHostGameToolTip()
{ {
const QString shortcut = SettingsCache::instance().shortcuts().getShortcutString("GameSelector/aQuickHostGame"); const QString shortcut = SettingsCache::instance().shortcuts().getShortcutString("GameSelector/aQuickHostGame");
const QString tooltip = shortcut.isEmpty() ? tr("Host a game with your last-used settings") 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); : 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, // 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. // since a QPushButton does not inherit an action's tooltip.
if (hostGameAction) { if (hostGameAction) {
hostGameAction->setToolTip(tooltip); hostGameAction->setToolTip(quickHostTooltip);
} }
if (hostGameButton) { 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);
} }
} }

View file

@ -164,12 +164,12 @@ private:
QPushButton *filterButton; /**< Button to open the filter dialog. */ QPushButton *filterButton; /**< Button to open the filter dialog. */
QPushButton *clearFilterButton; /**< Button to clear active filters. */ QPushButton *clearFilterButton; /**< Button to clear active filters. */
QPushButton *hostGameButton; /**< Host button, in the same family as the join buttons. */ QPushButton *hostGameButton = nullptr; /**< Host button, in the same family as the join buttons. */
QToolButton *hostGameMenuButton; /**< Split arrow that opens the host menu (non-macOS styles only). */ QToolButton *hostGameMenuButton = nullptr; /**< Split arrow that opens the host menu (non-macOS styles only). */
QWidget *hostGameButtonWidget; /**< Container that keeps the split button and its arrow together. */ QWidget *hostGameButtonWidget = nullptr; /**< Container that keeps the split button and its arrow together. */
QMenu *hostGameMenu; /**< Menu behind the arrow, or behind the pull-down button on macOS. */ QMenu *hostGameMenu = nullptr; /**< Menu behind the arrow, or behind the pull-down button on macOS. */
QAction *hostGameAction; /**< Quick-host action, carrying the keyboard shortcut. */ QAction *hostGameAction = nullptr; /**< Quick-host action, carrying the keyboard shortcut. */
QAction *reviewGameAction; /**< Menu action that opens the full create dialog. */ QAction *reviewGameAction = nullptr; /**< Menu action that opens the full create dialog. */
QLabel *hostErrorLabel; /**< Inline failure message for a quick-host request. */ QLabel *hostErrorLabel; /**< Inline failure message for a quick-host request. */
QPushButton *joinButton; /**< Button to join the selected game. */ QPushButton *joinButton; /**< Button to join the selected game. */
QPushButton *joinAsJudgeButton; /**< Button to join the selected game as a judge. */ QPushButton *joinAsJudgeButton; /**< Button to join the selected game as a judge. */
@ -198,6 +198,8 @@ private:
void applyHostGameStyle(); void applyHostGameStyle();
/** @brief Enables or disables the quick-host and review controls while a command is in flight. */ /** @brief Enables or disables the quick-host and review controls while a command is in flight. */
void setHostCommandPending(bool pending); 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. */ /** @brief Keeps the quick-host button tooltip in sync with the current shortcut. */
void updateHostGameToolTip(); void updateHostGameToolTip();