Use a native pull-down on macOS and a split button elsewhere

- Gate on the active application style instead of the platform, so
  choosing Fusion on macOS still gets the split button: QMacStyle ignores
  autoRaise/flat and draws a QToolButton menu as a separate square.
- Rebuild the host control on QEvent::StyleChange, because the appearance
  settings switch styles while the room is open.
- Keep the review action, its arrow and the host button disabled while a
  host or join command is still in flight.
- Announce inline quick-host failures to screen readers.

cockatrice/src/interface/widgets/server/game_selector.cpp

@@ -17,9 +17,13 @@
 #include "libcockatrice/protocol/pb/serverinfo_user.pb.h"
 #include "user/user_list_manager.h"
 
+#include <QAccessible>
+#include <QAccessibleEvent>
 #include <QAction>
+#include <QApplication>
 #include <QClipboard>
 #include <QDebug>
+#include <QEvent>
 #include <QGuiApplication>
 #include <QHBoxLayout>
 #include <QHeaderView>
@@ -34,10 +38,12 @@
 #include <QSize>
 #include <QString>
 #include <QStringLiteral>
+#include <QStyle>
 #include <QToolButton>
 #include <QTreeView>
 #include <QVBoxLayout>
 #include <QVariant>
+#include <QWidget>
 #include <QtGlobal>
 #include <libcockatrice/network/client/abstract/abstract_client.h>
 #include <libcockatrice/protocol/pb/response.pb.h>
@@ -56,6 +62,33 @@ class QPoint;
 class QWidget;
 template <typename T> class QList;
 
+namespace
+{
+
+/**
+ * @brief Returns whether the application paints with the native macOS style.
+ *
+ * QMacStyle ignores autoRaise and flat, and draws a QToolButton menu as a second square
+ * bezel, so the split host control cannot be composed there. A global style sheet wraps the
+ * real application style in a QStyleSheetStyle that takes it as a child, so that layer is
+ * unwrapped before the style is identified.
+ */
+bool usesMacStyle()
+{
+    QStyle *style = QApplication::style();
+    if (style == nullptr) {
+        return false;
+    }
+    if (style->inherits("QStyleSheetStyle")) {
+        if (QStyle *baseStyle = style->findChild<QStyle *>(QString(), Qt::FindDirectChildrenOnly)) {
+            style = baseStyle;
+        }
+    }
+    return style->inherits("QMacStyle");
+}
+
+} // namespace
+
 GameSelector::GameSelector(AbstractClient *_client,
                            TabSupervisor *_tabSupervisor,
                            TabRoom *_room,
@@ -145,6 +178,9 @@ GameSelector::GameSelector(AbstractClient *_client,
     connect(gameListProxyModel, &GamesProxyModel::filtersChanged, this, &GameSelector::checkClearFilterButtonState);
 
     hostGameButton = nullptr;
+    hostGameMenuButton = nullptr;
+    hostGameButtonWidget = nullptr;
+    hostGameMenu = nullptr;
     hostGameAction = nullptr;
     reviewGameAction = nullptr;
     if (room) {
@@ -152,19 +188,37 @@ GameSelector::GameSelector(AbstractClient *_client,
         hostGameAction->setShortcutContext(Qt::WidgetWithChildrenShortcut);
         connect(hostGameAction, &QAction::triggered, this, &GameSelector::actQuickHost);
 
-        hostGameButton = new QToolButton(this);
+        // A plain QPushButton, so the quick host sits in the same control family as the
+        // join buttons. A QPushButton does not take its text, tooltip or enabled state from
+        // an action, so those are mirrored by hand below and in setHostCommandPending().
+        hostGameButton = new QPushButton(this);
         hostGameButton->setObjectName("hostGameButton");
-        hostGameButton->setDefaultAction(hostGameAction);
-        hostGameButton->setPopupMode(QToolButton::MenuButtonPopup);
-        hostGameButton->setToolButtonStyle(Qt::ToolButtonTextOnly);
 
-        auto *hostGameMenu = new QMenu(hostGameButton);
+        hostGameMenu = new QMenu(this);
         hostGameMenu->setObjectName("hostGameMenu");
+        hostGameMenu->addAction(hostGameAction);
         reviewGameAction = hostGameMenu->addAction(QString());
         reviewGameAction->setObjectName("reviewGameAction");
         reviewGameAction->setShortcutContext(Qt::WidgetWithChildrenShortcut);
         connect(reviewGameAction, &QAction::triggered, this, &GameSelector::actCreate);
-        hostGameButton->setMenu(hostGameMenu);
+
+        // The secondary control: a flat arrow next to the primary button. It is hidden when
+        // the active style draws a native pull-down instead (see applyHostGameStyle()).
+        hostGameMenuButton = new QToolButton(this);
+        hostGameMenuButton->setObjectName("hostGameMenuButton");
+        hostGameMenuButton->setArrowType(Qt::DownArrow);
+        hostGameMenuButton->setAutoRaise(true);
+        hostGameMenuButton->setPopupMode(QToolButton::InstantPopup);
+        hostGameMenuButton->setToolButtonStyle(Qt::ToolButtonIconOnly);
+
+        hostGameButtonWidget = new QWidget(this);
+        auto *hostGameButtonLayout = new QHBoxLayout(hostGameButtonWidget);
+        hostGameButtonLayout->setContentsMargins(0, 0, 0, 0);
+        hostGameButtonLayout->setSpacing(0);
+        hostGameButtonLayout->addWidget(hostGameButton);
+        hostGameButtonLayout->addWidget(hostGameMenuButton);
+
+        applyHostGameStyle();
 
         // The shortcuts belong to the whole room tab, not just the game list, so they also
         // fire while the chat or the user list has focus. A QToolButton menu is mouse-only,
@@ -190,7 +244,7 @@ GameSelector::GameSelector(AbstractClient *_client,
     }
     buttonLayout->addStretch();
     if (room) {
-        buttonLayout->addWidget(hostGameButton);
+        buttonLayout->addWidget(hostGameButtonWidget);
     }
     buttonLayout->addWidget(joinButton);
     if (tabSupervisor->getUserInfo()->user_level() & ServerInfo_User::IsJudge) {
@@ -362,20 +416,73 @@ void GameSelector::checkQuickHostResponse(const Response &response)
 void GameSelector::showHostError(const QString &message)
 {
     hostErrorLabel->setText(message);
+    hostErrorLabel->setAccessibleName(message);
     hostErrorLabel->setVisible(true);
+
+    // The label appears above the game list without taking focus, so announce it: a screen
+    // reader would otherwise never reach the message after the button press.
+    QAccessibleEvent alertEvent(hostErrorLabel, QAccessible::Alert);
+    QAccessible::updateAccessibility(&alertEvent);
 }
 
 void GameSelector::clearHostError()
 {
     hostErrorLabel->clear();
+    hostErrorLabel->setAccessibleName(QString());
     hostErrorLabel->setVisible(false);
 }
 
+void GameSelector::applyHostGameStyle()
+{
+    if (hostGameButton == nullptr || hostGameMenu == nullptr) {
+        return;
+    }
+
+    // The appearance settings can switch the application style while the room is open, so
+    // the primary click is wired from scratch instead of once: disconnect first, otherwise
+    // a second pass would quick-host twice.
+    disconnect(hostGameButton, &QPushButton::clicked, hostGameAction, &QAction::trigger);
+
+    if (usesMacStyle()) {
+        // A native pull-down: one bezel with the menu indicator inside it. The separate
+        // arrow is hidden because QMacStyle would draw it as a second square bezel.
+        hostGameButton->setMenu(hostGameMenu);
+        hostGameMenuButton->setMenu(nullptr);
+        hostGameMenuButton->setVisible(false);
+    } else {
+        // The split button: the primary half quick-hosts, the flat arrow opens the menu.
+        hostGameButton->setMenu(nullptr);
+        connect(hostGameButton, &QPushButton::clicked, hostGameAction, &QAction::trigger);
+        hostGameMenuButton->setMenu(hostGameMenu);
+        hostGameMenuButton->setVisible(true);
+    }
+}
+
+void GameSelector::changeEvent(QEvent *event)
+{
+    if (event->type() == QEvent::StyleChange) {
+        applyHostGameStyle();
+    }
+    QGroupBox::changeEvent(event);
+}
+
 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;
     if (hostGameAction) {
-        hostGameAction->setEnabled(!pending);
+        hostGameAction->setEnabled(hostControlsEnabled);
+    }
+    if (hostGameButton) {
+        hostGameButton->setEnabled(hostControlsEnabled);
+    }
+    if (hostGameMenuButton) {
+        hostGameMenuButton->setEnabled(hostControlsEnabled);
+    }
+    if (reviewGameAction) {
+        reviewGameAction->setEnabled(hostControlsEnabled);
     }
 }
 
@@ -393,6 +500,9 @@ void GameSelector::checkResponse(const Response &response)
     // Ideally we should have a way to figure out if the current game is the
     // same as the one we are getting a response for, but it's probably not
     // worth the trouble.
+    // The join response is the one that re-enables the buttons; clear the join flag first so
+    // the host controls become usable again unless a quick host is still in flight.
+    joinCommandPending = false;
     enableButtons();
 
     switch (response.response_code()) {
@@ -590,9 +700,19 @@ 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);
+    }
 
     joinButton->setEnabled(false);
     spectateButton->setEnabled(false);
@@ -600,8 +720,18 @@ void GameSelector::disableButtons()
 
 void GameSelector::enableButtons()
 {
+    const bool hostControlsEnabled = !hostCommandPending && !joinCommandPending;
     if (hostGameAction) {
-        hostGameAction->setEnabled(!hostCommandPending);
+        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
@@ -626,13 +756,21 @@ void GameSelector::retranslateUi()
     filterButton->setText(tr("&Filter games"));
     clearFilterButton->setText(tr("C&lear filter"));
     if (hostGameAction) {
-        // A QToolButton cannot draw a mnemonic from a default action; the shortcut is the
+        hostGameAction->setText(tr("Host with last settings"));
-        // keyboard path here.
+        if (hostGameButton) {
-        hostGameAction->setText(tr("Host game"));
+            // A QPushButton does not draw text from an action; the button keeps the short
+            // label while the menu item spells out what the remembered settings do.
+            hostGameButton->setText(tr("Host game"));
+        }
         updateHostGameToolTip();
     }
     if (reviewGameAction) {
-        reviewGameAction->setText(tr("Review first…"));
+        reviewGameAction->setText(tr("Review settings first…"));
+        if (hostGameMenuButton) {
+            // The split arrow has no text of its own; name it for the menu it opens.
+            hostGameMenuButton->setToolTip(tr("More host options"));
+            hostGameMenuButton->setAccessibleName(tr("More host options"));
+        }
     }
     joinButton->setText(tr("&Join"));
     joinAsJudgeButton->setText(tr("Join as judge"));
@@ -656,8 +794,16 @@ void GameSelector::refreshShortcuts()
 void GameSelector::updateHostGameToolTip()
 {
     const QString shortcut = SettingsCache::instance().shortcuts().getShortcutString("GameSelector/aQuickHostGame");
-    hostGameAction->setToolTip(shortcut.isEmpty() ? tr("Host a game with your last-used settings")
+    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));
+                                               : 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);
+    }
+    if (hostGameButton) {
+        hostGameButton->setToolTip(tooltip);
+    }
 }
 
 void GameSelector::processGameInfo(const ServerInfo_Game &info)

cockatrice/src/interface/widgets/server/game_selector.h

@@ -15,6 +15,7 @@ class QTreeView;
 class GamesModel;
 class GamesProxyModel;
 class QLabel;
+class QMenu;
 class QPushButton;
 class QToolButton;
 class QAction;
@@ -141,6 +142,15 @@ signals:
      */
     void gameJoined(int gameId);
 
+protected:
+    /**
+     * @brief Rebuilds the host control when the application style changes.
+     *
+     * The appearance settings can switch between the native macOS style and a
+     * cross-platform style while the room is open, and the host control follows.
+     */
+    void changeEvent(QEvent *event) override;
+
 private:
     AbstractClient *client;       /**< The network client used to communicate with the server. */
     TabSupervisor *tabSupervisor; /**< Reference to TabSupervisor for managing tabs and rooms. */
@@ -154,7 +164,10 @@ private:
 
     QPushButton *filterButton;               /**< Button to open the filter dialog. */
     QPushButton *clearFilterButton;          /**< Button to clear active filters. */
-    QToolButton *hostGameButton;             /**< Split button: quick host, with the full form in its menu. */
+    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. */
@@ -170,12 +183,20 @@ private:
                                        file on every keystroke. */
 
     bool hostCommandPending = false; /**< True while a quick-host command is in flight. */
+    bool joinCommandPending = false; /**< True while a join/spectate command is in flight. */
 
     /** @brief Shows a quick-host failure above the game list. */
     void showHostError(const QString &message);
     /** @brief Hides the quick-host failure message. */
     void clearHostError();
-    /** @brief Enables or disables the quick-host action while a command is in flight. */
+    /**
+     * @brief Rebuilds the host control for the active style.
+     *
+     * The native macOS style draws a single pull-down button; every other style gets the
+     * split button, whose primary half quick-hosts and whose arrow opens the menu.
+     */
+    void applyHostGameStyle();
+    /** @brief Enables or disables the quick-host and review controls while a command is in flight. */
     void setHostCommandPending(bool pending);
     /** @brief Keeps the quick-host button tooltip in sync with the current shortcut. */
     void updateHostGameToolTip();