Fix filter menus

changelog/unreleased/9513

@@ -0,0 +1,7 @@
+Bugfix: improve filter pop-up menu and button
+
+- replaced "No filter" option text with "All", to avoid the "No filter is not enabled" situation
+- replace the "Filter" label on the button with "1 Filter"/"2 Filters" when a filter is active, so a user can immediately see that without having to open the filter pop-up
+
+https://github.com/owncloud/client/issues/9425
+https://github.com/owncloud/client/pull/9513

src/gui/activitywidget.cpp

@@ -59,7 +59,7 @@ ActivityWidget::ActivityWidget(QWidget *parent)
     _ui->setupUi(this);
 
     _model = new ActivityListModel(this);
-    _sortModel = new QSortFilterProxyModel(this);
+    _sortModel = new SignalledQSortFilterProxyModel(this);
     _sortModel->setSourceModel(_model);
     _ui->_activityList->setModel(_sortModel);
     _sortModel->setSortRole(Models::UnderlyingDataRole);
@@ -103,12 +103,16 @@ ActivityWidget::ActivityWidget(QWidget *parent)
     header->setContextMenuPolicy(Qt::CustomContextMenu);
     connect(header, &QListView::customContextMenuRequested, header, [header, this] {
         auto menu = ProtocolWidget::showFilterMenu(header, _sortModel, static_cast<int>(ActivityListModel::ActivityRole::Account), tr("Account"));
+        menu->addSeparator();
         header->addResetActionToMenu(menu);
     });
 
     connect(_ui->_filterButton, &QAbstractButton::clicked, this, [this] {
         ProtocolWidget::showFilterMenu(_ui->_filterButton, _sortModel, static_cast<int>(ActivityListModel::ActivityRole::Account), tr("Account"));
     });
+    connect(_sortModel, &SignalledQSortFilterProxyModel::filterChanged, [this]() {
+        _ui->_filterButton->setText(CommonStrings::filterButtonText(_sortModel->filterRegExp().isEmpty() ? 0 : 1));
+    });
 
     connect(&_removeTimer, &QTimer::timeout, this, &ActivityWidget::slotCheckToCleanWidgets);
     _removeTimer.setInterval(1000);

src/gui/activitywidget.h

@@ -26,6 +26,8 @@
 #include "account.h"
 #include "activitydata.h"
 
+#include "models/models.h"
+
 #include "ui_activitywidget.h"
 
 class QPushButton;
@@ -113,7 +115,7 @@ private slots:
     int _notificationRequestsRunning;
 
     ActivityListModel *_model;
-    QSortFilterProxyModel *_sortModel;
+    SignalledQSortFilterProxyModel *_sortModel;
     QVBoxLayout *_notificationsLayout;
 };
 

src/gui/commonstrings.cpp

@@ -42,3 +42,8 @@ QString CommonStrings::copyToClipBoard()
 {
     return QCoreApplication::translate("CommonStrings", "Copy");
 }
+
+QString CommonStrings::filterButtonText(int filterCount)
+{
+    return QCoreApplication::translate("CommonStrings", "%n Filter(s)").arg(filterCount);
+}

src/gui/commonstrings.h

@@ -22,5 +22,6 @@ namespace CommonStrings {
     QString showInFileBrowser();
     QString showInWebBrowser();
     QString copyToClipBoard();
+    QString filterButtonText(int filterCount);
+}
 }
-}

src/gui/issueswidget.cpp

@@ -15,23 +15,24 @@
 #include <QtGui>
 #include <QtWidgets>
 
-#include "issueswidget.h"
+#include "account.h"
+#include "accountmanager.h"
+#include "accountstate.h"
+#include "common/syncjournalfilerecord.h"
+#include "commonstrings.h"
 #include "configfile.h"
-#include "syncresult.h"
-#include "syncengine.h"
-#include "logger.h"
-#include "theme.h"
-#include "folderman.h"
-#include "syncfileitem.h"
+#include "elidedlabel.h"
 #include "folder.h"
+#include "folderman.h"
+#include "issueswidget.h"
+#include "logger.h"
 #include "models/models.h"
 #include "openfilemanager.h"
 #include "protocolwidget.h"
-#include "accountstate.h"
-#include "account.h"
-#include "accountmanager.h"
-#include "common/syncjournalfilerecord.h"
-#include "elidedlabel.h"
+#include "syncengine.h"
+#include "syncfileitem.h"
+#include "syncresult.h"
+#include "theme.h"
 
 
 #include "ui_issueswidget.h"
@@ -49,13 +50,13 @@ bool persistsUntilLocalDiscovery(const OCC::ProtocolItem &data)
 }
 namespace OCC {
 
-class SyncFileItemStatusSetSortFilterProxyModel : public QSortFilterProxyModel
+class SyncFileItemStatusSetSortFilterProxyModel : public SignalledQSortFilterProxyModel
 {
 public:
     using StatusSet = std::array<bool, SyncFileItem::StatusCount>;
 
     explicit SyncFileItemStatusSetSortFilterProxyModel(QObject *parent = nullptr)
-        : QSortFilterProxyModel(parent)
+        : SignalledQSortFilterProxyModel(parent)
     {
         resetFilter();
     }
@@ -74,16 +75,13 @@ class SyncFileItemStatusSetSortFilterProxyModel : public QSortFilterProxyModel
         if (_filter != newFilter) {
             _filter = newFilter;
             invalidateFilter();
+            emit filterChanged();
         }
     }
 
     void resetFilter()
     {
-        StatusSet defaultFilter;
-        defaultFilter.fill(true);
-        defaultFilter[SyncFileItem::NoStatus] = false;
-        defaultFilter[SyncFileItem::Success] = false;
-        setFilter(defaultFilter);
+        setFilter(defaultFilter());
     }
 
     bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override
@@ -99,6 +97,47 @@ class SyncFileItemStatusSetSortFilterProxyModel : public QSortFilterProxyModel
         return _filter[static_cast<SyncFileItem::Status>(sourceData)];
     }
 
+    int filterCount()
+    {
+        StatusSet defaultSet = defaultFilter();
+        OC_ASSERT(defaultSet.size() == _filter.size());
+
+        int count = 0;
+        // The number of filters is the number of items in the current filter that differ from the default set.
+        for (size_t i = 0, ei = defaultSet.size(); i != ei; ++i) {
+            // All errors are shown as a single filter, and they are all turned on or off together.
+            // So to show them as 1 filter, ignore the first three errors...
+            switch (i) {
+            case SyncFileItem::Status::FatalError:
+                Q_FALLTHROUGH();
+            case SyncFileItem::Status::NormalError:
+                Q_FALLTHROUGH();
+            case SyncFileItem::Status::SoftError:
+                break;
+
+                // ... but *do* count the last one ...
+            case SyncFileItem::Status::DetailError:
+                Q_FALLTHROUGH();
+            default:
+                // ... just like the other status items:
+                if (defaultSet[i] != _filter[i]) {
+                    count += 1;
+                }
+            }
+        }
+        return count;
+    }
+
+private:
+    static StatusSet defaultFilter()
+    {
+        StatusSet defaultSet;
+        defaultSet.fill(true);
+        defaultSet[SyncFileItem::NoStatus] = false;
+        defaultSet[SyncFileItem::Success] = false;
+        return defaultSet;
+    }
+
 private:
     StatusSet _filter;
 };
@@ -141,9 +180,11 @@ IssuesWidget::IssuesWidget(QWidget *parent)
     });
 
     _model = new ProtocolItemModel(20000, true, this);
-    _sortModel = new QSortFilterProxyModel(this);
+    _sortModel = new SignalledQSortFilterProxyModel(this);
+    connect(_sortModel, &SignalledQSortFilterProxyModel::filterChanged, this, &IssuesWidget::filterDidChange);
     _sortModel->setSourceModel(_model);
     _statusSortModel = new SyncFileItemStatusSetSortFilterProxyModel(this);
+    connect(_statusSortModel, &SignalledQSortFilterProxyModel::filterChanged, this, &IssuesWidget::filterDidChange);
     _statusSortModel->setSourceModel(_sortModel);
     _statusSortModel->setSortRole(Qt::DisplayRole); // Sorting should be done based on the text in the column cells, but...
     _statusSortModel->setFilterRole(Models::UnderlyingDataRole); // ... filtering should be done on the underlying enum value.
@@ -281,6 +322,18 @@ void IssuesWidget::slotItemCompleted(const QString &folder, const SyncFileItemPt
     _model->addProtocolItem(ProtocolItem { folder, item });
 }
 
+void IssuesWidget::filterDidChange()
+{
+    // We have two filters here: the filter by status (which can have multiple items checked *off*...
+    int filterCount = _statusSortModel->filterCount();
+    // .. and the filter on the account name, which can only be 1 item checked:
+    if (!_sortModel->filterRegExp().isEmpty()) {
+        filterCount += 1;
+    }
+
+    _ui->_filterButton->setText(CommonStrings::filterButtonText(filterCount));
+}
+
 void IssuesWidget::slotItemContextMenu()
 {
     auto rows = _ui->_tableView->selectionModel()->selectedRows();

src/gui/issueswidget.h

@@ -21,6 +21,7 @@
 #include <QTimer>
 
 #include "models/expandingheaderview.h"
+#include "models/models.h"
 #include "models/protocolitemmodel.h"
 #include "owncloudgui.h"
 #include "progressdispatcher.h"
@@ -52,6 +53,7 @@ class IssuesWidget : public QWidget
 public slots:
     void slotProgressInfo(const QString &folder, const ProgressInfo &progress);
     void slotItemCompleted(const QString &folder, const SyncFileItemPtr &item);
+    void filterDidChange();
 
 signals:
     void issueCountUpdated(int);
@@ -65,7 +67,7 @@ private slots:
     std::function<void()> addStatusFilter(QMenu *menu);
 
     ProtocolItemModel *_model;
-    QSortFilterProxyModel *_sortModel;
+    SignalledQSortFilterProxyModel *_sortModel;
     SyncFileItemStatusSetSortFilterProxyModel *_statusSortModel;
 
     Ui::IssuesWidget *_ui;

src/gui/models/models.cpp

@@ -22,6 +22,17 @@
 
 #include <functional>
 
+OCC::SignalledQSortFilterProxyModel::SignalledQSortFilterProxyModel(QObject *parent)
+    : QSortFilterProxyModel(parent)
+{
+}
+
+void OCC::SignalledQSortFilterProxyModel::setFilterFixedStringSignalled(const QString &pattern)
+{
+    setFilterFixedString(pattern);
+    emit filterChanged();
+}
+
 QString OCC::Models::formatSelection(const QModelIndexList &items, int dataRole)
 {
     if (items.isEmpty()) {
@@ -77,9 +88,9 @@ QString OCC::Models::formatSelection(const QModelIndexList &items, int dataRole)
     return out;
 }
 
-std::function<void()> OCC::Models::addFilterMenuItems(QMenu *menu, const QStringList &candidates, QSortFilterProxyModel *model, int column, const QString &columnName, int role)
+std::function<void()> OCC::Models::addFilterMenuItems(QMenu *menu, const QStringList &candidates, SignalledQSortFilterProxyModel *model, int column, const QString &columnName, int role)
 {
-    menu->addAction(qApp->translate("OCC::Models", "%1 Filter:").arg(columnName))->setEnabled(false);
+    menu->addAction(QApplication::translate("OCC::Models", "%1 Filter:").arg(columnName))->setEnabled(false);
 
     auto filterGroup = new QActionGroup(menu);
     filterGroup->setExclusive(true);
@@ -89,7 +100,7 @@ std::function<void()> OCC::Models::addFilterMenuItems(QMenu *menu, const QString
         auto action = menu->addAction(s, menu, [=]() {
             model->setFilterRole(role);
             model->setFilterKeyColumn(column);
-            model->setFilterFixedString(filter);
+            model->setFilterFixedStringSignalled(filter);
         });
         action->setCheckable(true);
         if (currentFilter == filter) {
@@ -100,7 +111,7 @@ std::function<void()> OCC::Models::addFilterMenuItems(QMenu *menu, const QString
     };
 
 
-    auto noFilter = addAction(QApplication::translate("OCC::Models", "No filter"), QString());
+    auto noFilter = addAction(QApplication::translate("OCC::Models", "All"), QString());
 
     for (const auto &c : candidates) {
         addAction(c, c);

src/gui/models/models.h

@@ -14,6 +14,7 @@
 #pragma once
 
 #include <QModelIndexList>
+#include <QSortFilterProxyModel>
 #include <QString>
 #include <QtGlobal>
 
@@ -22,6 +23,19 @@ class QMenu;
 
 namespace OCC {
 
+class SignalledQSortFilterProxyModel : public QSortFilterProxyModel
+{
+    Q_OBJECT
+
+public:
+    SignalledQSortFilterProxyModel(QObject *parent = nullptr);
+
+    void setFilterFixedStringSignalled(const QString &pattern);
+
+signals:
+    void filterChanged();
+};
+
 namespace Models {
     enum DataRoles {
         UnderlyingDataRole = Qt::UserRole + 100,
@@ -33,7 +47,7 @@ namespace Models {
      */
     QString formatSelection(const QModelIndexList &items, int dataRole = Qt::DisplayRole);
 
-    std::function<void()> addFilterMenuItems(QMenu *menu, const QStringList &candidates, QSortFilterProxyModel *model, int column, const QString &columnName, int role);
+    std::function<void()> addFilterMenuItems(QMenu *menu, const QStringList &candidates, SignalledQSortFilterProxyModel *model, int column, const QString &columnName, int role);
 
     /**
      * Returns a vector with indices
@@ -55,5 +69,5 @@ namespace Models {
     {
         return range<T>(0, end);
     }
-};
-}
+} // OCC::Models namespace
+} // OCC namespace

src/gui/protocolwidget.cpp

@@ -32,7 +32,6 @@
 
 #include "models/activitylistmodel.h"
 #include "models/expandingheaderview.h"
-#include "models/models.h"
 
 #include "ui_protocolwidget.h"
 
@@ -52,7 +51,8 @@ ProtocolWidget::ProtocolWidget(QWidget *parent)
     // Build the model-view "stack":
     //  _model <- _sortModel <- _statusSortModel <- _tableView
     _model = new ProtocolItemModel(2000, false, this);
-    _sortModel = new QSortFilterProxyModel(this);
+    _sortModel = new SignalledQSortFilterProxyModel(this);
+    connect(_sortModel, &SignalledQSortFilterProxyModel::filterChanged, this, &ProtocolWidget::filterDidChange);
     _sortModel->setSourceModel(_model);
     _sortModel->setSortRole(Models::UnderlyingDataRole);
     _ui->_tableView->setModel(_sortModel);
@@ -66,6 +66,7 @@ ProtocolWidget::ProtocolWidget(QWidget *parent)
     header->setContextMenuPolicy(Qt::CustomContextMenu);
     connect(header, &QHeaderView::customContextMenuRequested, header, [header, this] {
         auto menu = showFilterMenu(header, _sortModel, static_cast<int>(ProtocolItemModel::ProtocolItemRole::Account), tr("Account"));
+        menu->addSeparator();
         header->addResetActionToMenu(menu);
     });
 
@@ -94,12 +95,11 @@ ProtocolWidget::~ProtocolWidget()
  * @param columnName the name column on which the filter is done
  * @return
  */
-QMenu *ProtocolWidget::showFilterMenu(QWidget *parent, QSortFilterProxyModel *model, int role, const QString &columnName)
+QMenu *ProtocolWidget::showFilterMenu(QWidget *parent, SignalledQSortFilterProxyModel *model, int role, const QString &columnName)
 {
     auto menu = new QMenu(parent);
     menu->setAttribute(Qt::WA_DeleteOnClose);
     Models::addFilterMenuItems(menu, AccountManager::instance()->accountNames(), model, role, columnName, Qt::DisplayRole);
-    menu->addSeparator();
     QTimer::singleShot(0, menu, [menu] {
         menu->popup(QCursor::pos());
     });
@@ -177,4 +177,9 @@ void ProtocolWidget::slotItemCompleted(const QString &folder, const SyncFileItem
     _model->addProtocolItem(ProtocolItem { folder, item });
 }
 
+void ProtocolWidget::filterDidChange()
+{
+    _ui->_filterButton->setText(CommonStrings::filterButtonText(_sortModel->filterRegExp().isEmpty() ? 0 : 1));
 }
+
+} // OCC namespace