Turn free space label into a flat button

The usage of a label to show this space information is problematic
because there are features/actions hidden behind clicking the
label. A button makes this clear. It also fixes the accessibility
of this control because previously this info would never be read
out by screen readers as it couldn't get keyboard focus.

A side-effect of this change is that the free disk space bar looks
more normal with non-Breeze styles. This had been a long-standing
issue.

diff --git a/src/statusbar/dolphinstatusbar.cpp b/src/statusbar/dolphinstatusbar.cpp
index 9451566fd987e684a00dab81f14a3949b405ad8b..9bc6210dc5f468cc0d3f8afa79928e6951674627 100644 (file)
--- a/src/statusbar/dolphinstatusbar.cpp
+++ b/src/statusbar/dolphinstatusbar.cpp
@@ -46,8 +46,6 @@ DolphinStatusBar::DolphinStatusBar(QWidget *parent)
     , m_delayUpdateTimer(nullptr)
     , m_textTimestamp()
 {
-    setContentsMargins(4, 0, 4, 0);
-
     // Initialize text label
     m_label = new KSqueezedTextLabel(m_text, this);
     m_label->setWordWrap(true);
@@ -117,7 +115,7 @@ DolphinStatusBar::DolphinStatusBar(QWidget *parent)
     m_progressBar->setMaximumWidth(fontMetrics.averageCharWidth() * 20);
 
     QHBoxLayout *topLayout = new QHBoxLayout(this);
-    topLayout->setContentsMargins(2, 0, 2, 0);
+    updateContentsMargins();
     topLayout->setSpacing(4);
     topLayout->addWidget(m_label, 1);
     topLayout->addWidget(m_zoomLabel);
@@ -284,6 +282,7 @@ void DolphinStatusBar::contextMenuEvent(QContextMenuEvent *event)
         GeneralSettings::setShowSpaceInfo(visible);
         m_spaceInfo->setVisible(visible);
     }
+    updateContentsMargins();
 }
 
 void DolphinStatusBar::showZoomSliderToolTip(int zoomLevel)
@@ -338,10 +337,22 @@ void DolphinStatusBar::setExtensionsVisible(bool visible)
     m_spaceInfo->setVisible(showSpaceInfo);
     m_zoomSlider->setVisible(showZoomSlider);
     m_zoomLabel->setVisible(showZoomSlider);
+    updateContentsMargins();
+}
+
+void DolphinStatusBar::updateContentsMargins()
+{
+    if (GeneralSettings::showSpaceInfo()) {
+        // We reduce the outside margin for the flat button so it visually has the same margin as the status bar text label on the other end of the bar.
+        layout()->setContentsMargins(6, 0, 2, 0);
+    } else {
+        layout()->setContentsMargins(6, 0, 6, 0);
+    }
 }
 
 void DolphinStatusBar::paintEvent(QPaintEvent *paintEvent)
 {
+    Q_UNUSED(paintEvent)
     QPainter p(this);
     QStyleOption opt;
     opt.initFrom(this);

diff --git a/src/statusbar/dolphinstatusbar.h b/src/statusbar/dolphinstatusbar.h
index 983f5b6dac1d99d5cad80b28cc9dc7c4264ce547..5cb1c4b6044f29e7f219410e92416c976b0736c9 100644 (file)
--- a/src/statusbar/dolphinstatusbar.h
+++ b/src/statusbar/dolphinstatusbar.h
@@ -121,6 +121,8 @@ private:
      */
     void setExtensionsVisible(bool visible);
 
+    void updateContentsMargins();
+
 private:
     QString m_text;
     QString m_defaultText;

diff --git a/src/statusbar/statusbarspaceinfo.cpp b/src/statusbar/statusbarspaceinfo.cpp
index 546c217a7ad3ff62b54547acf0afb9ee5413f3e8..b9dafe997f44e1c1f91a16957eb3acd8aee94e0d 100644 (file)
--- a/src/statusbar/statusbarspaceinfo.cpp
+++ b/src/statusbar/statusbarspaceinfo.cpp
@@ -8,20 +8,35 @@
 
 #include "spaceinfoobserver.h"
 
+#include <KCapacityBar>
 #include <KIO/ApplicationLauncherJob>
 #include <KIO/Global>
 #include <KLocalizedString>
 #include <KService>
 
+#include <QHBoxLayout>
 #include <QMenu>
 #include <QMouseEvent>
 #include <QStorageInfo>
+#include <QToolButton>
 
 StatusBarSpaceInfo::StatusBarSpaceInfo(QWidget *parent)
-    : KCapacityBar(KCapacityBar::DrawTextInline, parent)
+    : QWidget(parent)
     , m_observer(nullptr)
 {
-    setCursor(Qt::PointingHandCursor);
+    m_capacityBar = new KCapacityBar(KCapacityBar::DrawTextInline, this);
+    m_textInfoButton = new QToolButton(this);
+    m_textInfoButton->setAutoRaise(true);
+    m_textInfoButton->setPopupMode(QToolButton::InstantPopup);
+    m_buttonMenu = new QMenu(this);
+    m_textInfoButton->setMenu(m_buttonMenu);
+    connect(m_buttonMenu, &QMenu::aboutToShow, this, &StatusBarSpaceInfo::updateMenu);
+
+    auto layout = new QHBoxLayout(this);
+    // We reduce the outside margin of the flat button so it visually has the same margin as the status bar text label on the other end of the bar.
+    layout->setContentsMargins(2, -1, 0, -1); // "-1" makes it so the fixed height won't be ignored.
+    layout->addWidget(m_capacityBar);
+    layout->addWidget(m_textInfoButton);
 }
 
 StatusBarSpaceInfo::~StatusBarSpaceInfo()
@@ -65,7 +80,7 @@ void StatusBarSpaceInfo::showEvent(QShowEvent *event)
 {
     if (m_shown) {
         if (m_ready) {
-            KCapacityBar::showEvent(event);
+            QWidget::showEvent(event);
         }
 
         if (m_observer.isNull()) {
@@ -81,71 +96,68 @@ void StatusBarSpaceInfo::hideEvent(QHideEvent *event)
         m_observer.reset();
         m_ready = false;
     }
-    KCapacityBar::hideEvent(event);
+    QWidget::hideEvent(event);
 }
 
-void StatusBarSpaceInfo::mousePressEvent(QMouseEvent *event)
+void StatusBarSpaceInfo::updateMenu()
 {
-    if (event->button() == Qt::LeftButton) {
-        // Creates a menu with tools that help to find out more about free
-        // disk space for the given url.
+    m_buttonMenu->clear();
 
-        const KService::Ptr filelight = KService::serviceByDesktopName(QStringLiteral("org.kde.filelight"));
-        const KService::Ptr kdiskfree = KService::serviceByDesktopName(QStringLiteral("org.kde.kdf"));
+    // Creates a menu with tools that help to find out more about free
+    // disk space for the given url.
 
-        if (!filelight && !kdiskfree) {
-            // nothing to show
-            return;
-        }
-
-        QMenu *menu = new QMenu(this);
-
-        if (filelight) {
-            QAction *filelightFolderAction = menu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Disk Usage Statistics - current folder"));
+    const KService::Ptr filelight = KService::serviceByDesktopName(QStringLiteral("org.kde.filelight"));
+    const KService::Ptr kdiskfree = KService::serviceByDesktopName(QStringLiteral("org.kde.kdf"));
 
-            menu->connect(filelightFolderAction, &QAction::triggered, menu, [this, filelight](bool) {
-                auto *job = new KIO::ApplicationLauncherJob(filelight);
-                job->setUrls({m_url});
-                job->start();
-            });
+    if (!filelight && !kdiskfree) {
+        // nothing to show
+        return;
+    }
 
-            // For remote URLs like FTP analyzing the device makes no sense
-            if (m_url.isLocalFile()) {
-                QAction *filelightDiskAction = menu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Disk Usage Statistics - current device"));
+    if (filelight) {
+        QAction *filelightFolderAction = m_buttonMenu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Disk Usage Statistics - current folder"));
 
-                menu->connect(filelightDiskAction, &QAction::triggered, menu, [this, filelight](bool) {
-                    const QStorageInfo info(m_url.toLocalFile());
+        m_buttonMenu->connect(filelightFolderAction, &QAction::triggered, m_buttonMenu, [this, filelight](bool) {
+            auto *job = new KIO::ApplicationLauncherJob(filelight);
+            job->setUrls({m_url});
+            job->start();
+        });
 
-                    if (info.isValid() && info.isReady()) {
-                        auto *job = new KIO::ApplicationLauncherJob(filelight);
-                        job->setUrls({QUrl::fromLocalFile(info.rootPath())});
-                        job->start();
-                    }
-                });
-            }
+        // For remote URLs like FTP analyzing the device makes no sense
+        if (m_url.isLocalFile()) {
+            QAction *filelightDiskAction =
+                m_buttonMenu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Disk Usage Statistics - current device"));
 
-            QAction *filelightAllAction = menu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Disk Usage Statistics - all devices"));
-
-            menu->connect(filelightAllAction, &QAction::triggered, menu, [this, filelight](bool) {
+            m_buttonMenu->connect(filelightDiskAction, &QAction::triggered, m_buttonMenu, [this, filelight](bool) {
                 const QStorageInfo info(m_url.toLocalFile());
 
                 if (info.isValid() && info.isReady()) {
                     auto *job = new KIO::ApplicationLauncherJob(filelight);
+                    job->setUrls({QUrl::fromLocalFile(info.rootPath())});
                     job->start();
                 }
             });
         }
 
-        if (kdiskfree) {
-            QAction *kdiskfreeAction = menu->addAction(QIcon::fromTheme(QStringLiteral("kdf")), i18n("KDiskFree"));
+        QAction *filelightAllAction = m_buttonMenu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Disk Usage Statistics - all devices"));
+
+        m_buttonMenu->connect(filelightAllAction, &QAction::triggered, m_buttonMenu, [this, filelight](bool) {
+            const QStorageInfo info(m_url.toLocalFile());
 
-            connect(kdiskfreeAction, &QAction::triggered, this, [kdiskfree] {
-                auto *job = new KIO::ApplicationLauncherJob(kdiskfree);
+            if (info.isValid() && info.isReady()) {
+                auto *job = new KIO::ApplicationLauncherJob(filelight);
                 job->start();
-            });
-        }
+            }
+        });
+    }
+
+    if (kdiskfree) {
+        QAction *kdiskfreeAction = m_buttonMenu->addAction(QIcon::fromTheme(QStringLiteral("kdf")), i18n("KDiskFree"));
 
-        menu->exec(QCursor::pos());
+        connect(kdiskfreeAction, &QAction::triggered, this, [kdiskfree] {
+            auto *job = new KIO::ApplicationLauncherJob(kdiskfree);
+            job->start();
+        });
     }
 }
 
@@ -165,10 +177,11 @@ void StatusBarSpaceInfo::slotValuesChanged()
     const quint64 used = size - available;
     const int percentUsed = qRound(100.0 * qreal(used) / qreal(size));
 
-    setText(i18nc("@info:status Free disk space", "%1 free", KIO::convertSize(available)));
+    m_textInfoButton->setText(i18nc("@info:status Free disk space", "%1 free", KIO::convertSize(available)));
     setToolTip(i18nc("tooltip:status Free disk space", "%1 free out of %2 (%3% used)", KIO::convertSize(available), KIO::convertSize(size), percentUsed));
+    m_textInfoButton->setToolTip(toolTip());
     setUpdatesEnabled(false);
-    setValue(percentUsed);
+    m_capacityBar->setValue(percentUsed);
     setUpdatesEnabled(true);
 
     if (!isVisible()) {

diff --git a/src/statusbar/statusbarspaceinfo.h b/src/statusbar/statusbarspaceinfo.h
index 932f09b7b7b878dcc7871b64fbb38479aac9822d..9048bd10cfa2e08f2e8a11f114ca7ba17c3b2b72 100644 (file)
--- a/src/statusbar/statusbarspaceinfo.h
+++ b/src/statusbar/statusbarspaceinfo.h
@@ -6,13 +6,16 @@
 #ifndef STATUSBARSPACEINFO_H
 #define STATUSBARSPACEINFO_H
 
-#include <KCapacityBar>
-
 #include <QUrl>
+#include <QWidget>
 
 class QHideEvent;
 class QShowEvent;
+class QMenu;
 class QMouseEvent;
+class QToolButton;
+
+class KCapacityBar;
 
 class SpaceInfoObserver;
 
@@ -20,7 +23,7 @@ class SpaceInfoObserver;
  * @short Shows the available space for the volume represented
  *        by the given URL as part of the status bar.
  */
-class StatusBarSpaceInfo : public KCapacityBar
+class StatusBarSpaceInfo : public QWidget
 {
     Q_OBJECT
 
@@ -40,13 +43,17 @@ public:
 protected:
     void showEvent(QShowEvent *event) override;
     void hideEvent(QHideEvent *event) override;
-    void mousePressEvent(QMouseEvent *event) override;
+
+    void updateMenu();
 
 private Q_SLOTS:
     void slotValuesChanged();
 
 private:
     QScopedPointer<SpaceInfoObserver> m_observer;
+    KCapacityBar *m_capacityBar;
+    QToolButton *m_textInfoButton;
+    QMenu *m_buttonMenu;
     QUrl m_url;
     bool m_ready;
     bool m_shown;