From 3e3b3a572004c010fe2e34b017bf634e9f82116d Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Fri, 11 Mar 2022 14:29:33 +0100 Subject: [PATCH] Fix filter menus - 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 --- changelog/unreleased/9513 | 7 +++ src/gui/activitywidget.cpp | 6 ++- src/gui/activitywidget.h | 4 +- src/gui/commonstrings.cpp | 5 ++ src/gui/commonstrings.h | 3 +- src/gui/issueswidget.cpp | 93 ++++++++++++++++++++++++++++++-------- src/gui/issueswidget.h | 4 +- src/gui/models/models.cpp | 19 ++++++-- src/gui/models/models.h | 20 ++++++-- src/gui/protocolwidget.cpp | 13 ++++-- src/gui/protocolwidget.h | 7 ++- 11 files changed, 144 insertions(+), 37 deletions(-) create mode 100644 changelog/unreleased/9513 diff --git a/changelog/unreleased/9513 b/changelog/unreleased/9513 new file mode 100644 index 00000000000..837d7dc994b --- /dev/null +++ b/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 diff --git a/src/gui/activitywidget.cpp b/src/gui/activitywidget.cpp index 270d2aa9728..2ae61141978 100644 --- a/src/gui/activitywidget.cpp +++ b/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(ActivityListModel::ActivityRole::Account), tr("Account")); + menu->addSeparator(); header->addResetActionToMenu(menu); }); connect(_ui->_filterButton, &QAbstractButton::clicked, this, [this] { ProtocolWidget::showFilterMenu(_ui->_filterButton, _sortModel, static_cast(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); diff --git a/src/gui/activitywidget.h b/src/gui/activitywidget.h index cf683b5434a..4cb24f68916 100644 --- a/src/gui/activitywidget.h +++ b/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; }; diff --git a/src/gui/commonstrings.cpp b/src/gui/commonstrings.cpp index 0e65bcefa5f..ba061da637a 100644 --- a/src/gui/commonstrings.cpp +++ b/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); +} diff --git a/src/gui/commonstrings.h b/src/gui/commonstrings.h index 15d31f06770..5a25f800d42 100644 --- a/src/gui/commonstrings.h +++ b/src/gui/commonstrings.h @@ -22,5 +22,6 @@ namespace CommonStrings { QString showInFileBrowser(); QString showInWebBrowser(); QString copyToClipBoard(); + QString filterButtonText(int filterCount); +} } -} \ No newline at end of file diff --git a/src/gui/issueswidget.cpp b/src/gui/issueswidget.cpp index e53c7d7945d..f527ad3ee8b 100644 --- a/src/gui/issueswidget.cpp +++ b/src/gui/issueswidget.cpp @@ -15,23 +15,24 @@ #include #include -#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; 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(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(); diff --git a/src/gui/issueswidget.h b/src/gui/issueswidget.h index cda9830ed52..d64d3033a83 100644 --- a/src/gui/issueswidget.h +++ b/src/gui/issueswidget.h @@ -21,6 +21,7 @@ #include #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 addStatusFilter(QMenu *menu); ProtocolItemModel *_model; - QSortFilterProxyModel *_sortModel; + SignalledQSortFilterProxyModel *_sortModel; SyncFileItemStatusSetSortFilterProxyModel *_statusSortModel; Ui::IssuesWidget *_ui; diff --git a/src/gui/models/models.cpp b/src/gui/models/models.cpp index dc0b41987af..0199d5e9be2 100644 --- a/src/gui/models/models.cpp +++ b/src/gui/models/models.cpp @@ -22,6 +22,17 @@ #include +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 OCC::Models::addFilterMenuItems(QMenu *menu, const QStringList &candidates, QSortFilterProxyModel *model, int column, const QString &columnName, int role) +std::function 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 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 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); diff --git a/src/gui/models/models.h b/src/gui/models/models.h index 9b302171ff4..eb0e7d48723 100644 --- a/src/gui/models/models.h +++ b/src/gui/models/models.h @@ -14,6 +14,7 @@ #pragma once #include +#include #include #include @@ -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 addFilterMenuItems(QMenu *menu, const QStringList &candidates, QSortFilterProxyModel *model, int column, const QString &columnName, int role); + std::function 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(0, end); } -}; -} +} // OCC::Models namespace +} // OCC namespace diff --git a/src/gui/protocolwidget.cpp b/src/gui/protocolwidget.cpp index c3ed3378dea..b325b2435d1 100644 --- a/src/gui/protocolwidget.cpp +++ b/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(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 diff --git a/src/gui/protocolwidget.h b/src/gui/protocolwidget.h index 5dfb3631921..cf540cbbb12 100644 --- a/src/gui/protocolwidget.h +++ b/src/gui/protocolwidget.h @@ -26,6 +26,8 @@ #include "protocolitem.h" #include "ui_protocolwidget.h" +#include "models/models.h" + class QPushButton; class QSortFilterProxyModel; @@ -49,17 +51,18 @@ class ProtocolWidget : public QWidget ~ProtocolWidget() override; static void showContextMenu(QWidget *parent, ProtocolItemModel *model, const QModelIndexList &items); - static QMenu *showFilterMenu(QWidget *parent, QSortFilterProxyModel *model, int role, const QString &columnName); + static QMenu *showFilterMenu(QWidget *parent, SignalledQSortFilterProxyModel *model, int role, const QString &columnName); public slots: void slotItemCompleted(const QString &folder, const SyncFileItemPtr &item); + void filterDidChange(); private slots: void slotItemContextMenu(); private: ProtocolItemModel *_model; - QSortFilterProxyModel *_sortModel; + SignalledQSortFilterProxyModel *_sortModel; Ui::ProtocolWidget *_ui; }; }