Merge pull request #791 from matty0ung/3.0.11

Fix for text in side tabs not displaying on Mac

src/Application.cpp

@@ -1,6 +1,6 @@
 /*
  * Application.cpp is part of Brewtarget, and is Copyright the following
- * authors 2009-2022
+ * authors 2009-2024
  * - A.J. Drobnich <[email protected]>
  * - Dan Cavanagh <[email protected]>
  * - Matt Young <[email protected]>
@@ -326,7 +326,7 @@ namespace {
             "constant for resource dir:" << CONFIG_DATA_DIR;
          path = QString(CONFIG_DATA_DIR);
       }
-#elif defined(Q_OS_MAC)
+#elif defined(Q_OS_MACOS)
       // === Mac ===
       // We should be inside an app bundle.
       path += "../Resources/";
@@ -345,7 +345,7 @@ namespace {
 }
 
 const QDir Application::getConfigDir() {
-#if defined(Q_OS_LINUX) || defined(Q_OS_MAC) // Linux OS or Mac OS.
+#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS) // Linux OS or Mac OS.
    QDir dir;
    QFileInfo fileInfo;
 
@@ -388,7 +388,7 @@ QDir Application::getUserDataDir() {
 }
 
 QDir Application::getDefaultUserDataDir() {
-#if defined(Q_OS_LINUX) || defined(Q_OS_MAC) // Linux OS or Mac OS.#if defined(Q_OS_LINUX) || defined(Q_OS_MAC) // Linux OS or Mac OS.
+#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS) // Linux OS or Mac OS
    return getConfigDir();
 #elif defined(Q_OS_WIN) // Windows OS.
    // On Windows the Programs directory is normally not writable so we need to get the appData path from the environment instead.
@@ -441,7 +441,7 @@ bool Application::initialize() {
 
    Localization::loadTranslations(); // Do internationalization.
 
-#if defined(Q_OS_MAC)
+#if defined(Q_OS_MACOS)
    qt_set_sequence_auto_mnemonic(true); // turns on Mac Keyboard shortcuts
 #endif
 

src/BtHorizontalTabs.cpp

@@ -22,15 +22,45 @@
 #include <QDebug>
 #include <QStyleOptionTab>
 
+BtHorizontalTabs::BtHorizontalTabs([[maybe_unused]] bool const forceRotate) :
+   QProxyStyle{},
+   m_forceRotate{
+//
+// This may look a bit odd, but it saves having lots of other #ifdefs below
+//
+// You can sort of simulate Mac behaviour by changing this #ifdef to #ifndef
+//
+#ifdef Q_OS_MACOS
+      // On Mac, we only attempt to rotate the side tabs if you insist (eg because you know they are icons)
+      forceRotate
+#else
+      // On other platforms, it's safe always to rotate the size tabs
+      true
+#endif
+   } {
+   return;
+}
+
+BtHorizontalTabs::~BtHorizontalTabs() = default;
+
+
 QSize BtHorizontalTabs::sizeFromContents(ContentsType type,
                                          QStyleOption const * option,
                                          QSize const & contentsSize,
                                          QWidget const * widget) const {
    // By default, we're just going to return the same size as our parent class...
    QSize size = QProxyStyle::sizeFromContents(type, option, contentsSize, widget);
-   if (type == QStyle::CT_TabBarTab) {
+
+   //
+   // On Mac, the results of this function are correctly used by QTabBar to reserve the right size and shape space to
+   // draw a horizontal-oriented tab.  However, when QTabBar comes to draw the tab, it seems to ignore the width
+   // returned from this function.
+   //
+   // See https://github.com/Brewtarget/brewtarget/issues/787#issuecomment-1921341649 for example screenshot (after
+   // overriding the behaviour that would otherwise prevent text that does not fit in the tab from being displayed).
+   //
+   if (this->m_forceRotate && type == QStyle::CT_TabBarTab ) {
       // ...but, if it's a tab, then we want to swap the X and Y of the size rectangle we return
-//      qDebug() << Q_FUNC_INFO << "Transposing width:" << size.width() << ", height:" << size.height();
       size.transpose();
    }
    return size;
@@ -40,8 +70,17 @@ void BtHorizontalTabs::drawControl(ControlElement element,
                                    QStyleOption const * option,
                                    QPainter * painter,
                                    QWidget const * widget) const {
-   // Special handling is only for tabs
-   if (element == QStyle::CE_TabBarTabLabel) {
+   //
+   // Special handling is only for the text or icon inside tabs.  We want the tab itself (and the highlighting for
+   // "current tab") to be drawn as a "side" tab -- so we leave default handling for QStyle::CE_TabBarTabShape and
+   // QStyle::CE_TabBarTab.
+   //
+   // Of course, it would be cute, on Mac OS, to be able to force the correct widths of QStyle::CE_TabBarTabShape and
+   // QStyle::CE_TabBarTab by consulting widget->rect() (yes, of course, rect is a member function on QWidget even
+   // though it's a member variable on QStyleOption).  However, we cannot modify option.rect because option is read-only
+   // for us, and we do not own the QStyleOption object, so it is not safe for us to cast away its "const".
+   //
+   if (this->m_forceRotate && element == QStyle::CE_TabBarTabLabel) {
       QStyleOptionTab const * tabOption = qstyleoption_cast<QStyleOptionTab const *>(option);
       if (tabOption) {
          QStyleOptionTab modifiedTabOption{*tabOption};
@@ -51,14 +90,12 @@ void BtHorizontalTabs::drawControl(ControlElement element,
          //   - QTabBar::RoundedNorth = The normal rounded look above the pages
          //   - QTabBar::RoundedWest  = The normal rounded look on the left side of the pages
          //
-         // Normally, when the tabs are on the left (QTabBar::RoundedWest), the text is rotated so that (in
-         // left-to-right languages such as English) it reads bottom-to-top.  If we change the shape attribute for
-         // drawing the tab label to QTabBar::RoundedNorth, then the text should be drawn unrotated as though the tab
-         // were above the widget.
-         //
-         // Normally keep the debug statements below commented out as they generates a lot of output!
+         // Normally, when the tabs are on the left (QTabBar::RoundedWest), the text or icon is rotated 90°
+         // anticlockwise, so that (in left-to-right languages such as English) the text reads bottom-to-top.  If we
+         // change the shape attribute for drawing the tab label to QTabBar::RoundedNorth, then the text or icon should
+         // be drawn unrotated as though the tab were above the widget.  And, the work in sizeFromContents() above
+         // should have ensured there is enough space for this.
          //
-//         qDebug() << Q_FUNC_INFO << "Changing shape from" << modifiedTabOption.shape << "to" << QTabBar::RoundedNorth;
          modifiedTabOption.shape = QTabBar::RoundedNorth;
          QProxyStyle::drawControl(element, &modifiedTabOption, painter, widget);
          return;

src/BtHorizontalTabs.h

@@ -17,7 +17,6 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
-
 #ifndef BTHORIZONTALTABS_H
 #define BTHORIZONTALTABS_H
 #pragma once
@@ -27,12 +26,40 @@
 #include <QSize>
 
 /**
- * \brief A custom style class for \c QTabWidget with tabs on the left (\c QTabBar::RoundedWest) to rotate the tab so
- *        that its text is horizontal instead of the default vertical.  This looks neater when we have potentially long
- *        tab text on a widget that is not hugely tall.
+ * \brief A custom style class for \c QTabBar (eg of a \c QTabWidget with tabs on the left \c QTabBar::RoundedWest) to
+ *        rotate the tabs so that their text is horizontal instead of the default vertical.  This looks neater when we
+ *        have potentially long tab text on a widget that is not hugely tall.
+ *
+ *        To use on, say, \c myQTabWidget, a pointer to \c QTabWidget, call
+ *        \c myQTabWidget->tabBar()->setStyle(new BtHorizontalTabs);
+ *
+ *        NOTE: This is a commonly-used approach that works on Windows and Linux, but not entirely on Mac.  It's fine on
+ *              Mac for tabs with square icons in, but doesn't work properly for tabs with text. (If you look at the Qt
+ *              source code, eg https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qtabbar.cpp?h=5.15,
+ *              you'll see some special handling for Mac OS, so perhaps this is something to do with it.)
+ *
+ *              If we wanted a more sure-fire way of achieving horizontal text in left-hand tabs, we could, in
+ *              principle, create our own classes that inherit from \c QTabWidget and \c QTabBar.  But I fear we'd end
+ *              up re-implementing a lot of the inner workings of those classes, which would be rather painful.  For
+ *              now, we leave text in its default orientation on Mac.
  */
 class BtHorizontalTabs : public QProxyStyle {
+
 public:
+
+   /**
+    * \brief This is a slightly horrible way of getting things to work on Mac OS
+    *
+    * \param forceRotate If set to \c true then the class will do it's stuff even on Mac, which is what you want
+    *                    for tabs with square icons in.  Otherwise, on Mac, the class will do nothing, leaving the
+    *                    default behaviour of side tabs, which is what you want for text tabs.
+    *
+    *                    NOTE that, on other platforms (ie Linux and Windows), this parameter has no effect -- ie we'll
+    *                    always try to rotate the side tab text to horizontal.
+    */
+   BtHorizontalTabs(bool const forceRotate = false);
+   virtual ~BtHorizontalTabs();
+
    /**
     * \brief Reimplements \c QStyle::sizeFromContents (and various derivatives thereof)
     *
@@ -51,5 +78,9 @@ class BtHorizontalTabs : public QProxyStyle {
                             QStyleOption const * option,
                             QPainter * painter,
                             QWidget const * widget) const;
+
+private:
+   bool const m_forceRotate;
 };
+
 #endif

src/MainWindow.cpp

@@ -1,6 +1,6 @@
 /*
  * MainWindow.cpp is part of Brewtarget, and is Copyright the following
- * authors 2009-2023
+ * authors 2009-2024
  * - A.J. Drobnich <[email protected]>
  * - Dan Cavanagh <[email protected]>
  * - David Grundberg <[email protected]>
@@ -368,8 +368,8 @@ MainWindow::MainWindow(QWidget* parent) : QMainWindow(parent), pimpl{std::make_u
    // Stop things looking ridiculously tiny on high DPI displays
    this->setSizesInPixelsBasedOnDpi();
 
-   // Horizontal tabs, please
-   tabWidget_Trees->tabBar()->setStyle(new BtHorizontalTabs);
+   // Horizontal tabs, please -- even on Mac OS, as the tabs contain square icons
+   tabWidget_Trees->tabBar()->setStyle(new BtHorizontalTabs(true));
 
    /* PLEASE DO NOT REMOVE.
     This code is left here, commented out, intentionally. The only way I can