From a15505d408895d53f2fb4321836931a1b0742b7d Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 15 Aug 2026 11:24:07 +0200 Subject: fix(filters): label the flagged filter Important, and give the four icons Item 57 renamed the `flag` action to "Important" in 0.14.0, chosen over "Starred" partly because &I was free where &S collided with Mark spam. Item 93 then shipped the filter for the same tag as "Flagged", so one window offered both names for one thing. The generator keeps its own name, `flagged`: that string is stored in queries.json and matched against a closed set, so it is wire format rather than a label. The filters are QToolButtons now, like the Save button at the other end of the row, carrying a themed icon with the text beside it. Icon AND text for the reason the Save button already records: this row is a row of text buttons, so an icon alone reads as a different kind of control than it is. Theme icons rather than the shipped SVGs in Marks, because item 70's split is that the panes are ours and the chrome is the system's, and the query row is chrome. mail-mark-important matches the `flag` action's own icon, since the filter finds what the action marks. The icon test asserts a NAME was requested rather than that the icon resolved: QIcon::fromTheme returns null where no icon theme is installed, so isNull() would fail for a reason unrelated to this code. Dropping the setIcon call fails it. Widening the buttons to QToolButton broke eleven tests that reached them through findChild, which does not match a sibling type. The helpers and the filter lookups take QAbstractButton; savedQueryButton() stays on QPushButton, since the user's own queries really are those. --- CHANGELOG.md | 11 ++++++ src/config.cpp | 7 +++- src/mainwindow.cpp | 31 +++++++++++++++-- tests/test_config.cpp | 20 ++++++++++- tests/test_mainwindow.cpp | 87 ++++++++++++++++++++++++++++++++++++----------- 5 files changed, 132 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 133dd58..7b4df18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,17 @@ point at which they are stable. ## [Unreleased] +### Changed + +- The built-in filters carry icons, like the Save button at the other end of + the query row, with their text beside them. + +### Fixed + +- The flagged filter is labelled **Important**, matching the action of the same + name. It shipped in 0.21.0 as "Flagged", which put the same tag under two + names in one window. + ## [0.21.0] - 2026-08-15 The query row gains four filters the application ships: Unread, Inbox, Flagged diff --git a/src/config.cpp b/src/config.cpp index ece1fb9..0b7fe86 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -719,7 +719,12 @@ SavedQuery Config::builtinFilter(const QString &generator) } else if (generator == QStringLiteral("inbox")) { filter.name = tr("Inbox"); } else if (generator == QStringLiteral("flagged")) { - filter.name = tr("Flagged"); + // "Important", matching the `flag` action, which item 57 renamed from + // "Flag" for exactly this reason. Shipping the filter as "Flagged" + // beside it put the same tag under two names in one window. The + // GENERATOR stays `flagged`: that string is stored in queries.json and + // matched against a closed set, so it is wire format, not a label. + filter.name = tr("Important"); } else if (generator == QStringLiteral("sent")) { filter.name = tr("Sent"); // Messages rather than threads, and the only filter that sets this. A diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index bf1a33b..4343479 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1731,11 +1731,38 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) == Config::matchNothingQuery()) continue; - auto *button = new QPushButton(filter.name, row); + // A QToolButton, like the Save button at the other end of the row, so + // the two shipped controls carry icons the same way. The user's own + // queries stay plain QPushButtons: they have no icon to carry and + // nothing to say about which is which. + auto *button = new QToolButton(row); + button->setText(filter.name); // A stable object name per filter, so a test finds the button without // depending on the label, which is translated. button->setObjectName(filter.generated + QStringLiteral("Button")); - connect(button, &QPushButton::clicked, this, + + // Theme icons, not the shipped SVGs in Marks: item 70's split is that + // the panes are ours and the chrome is the system's, and the query row + // is chrome. A name the running theme lacks degrades to text on its + // own, which is why nothing here checks whether it resolved. + // + // mail-mark-important matches the `flag` action's own icon, since both + // reach the same tag: the filter finds what the action marks. + static const QHash filterIcons = { + { QStringLiteral("unread"), QStringLiteral("mail-mark-unread") }, + { QStringLiteral("inbox"), QStringLiteral("mail-inbox") }, + { QStringLiteral("flagged"), QStringLiteral("mail-mark-important") }, + { QStringLiteral("sent"), QStringLiteral("mail-sent") }, + }; + button->setIcon( + QIcon::fromTheme(filterIcons.value(filter.generated))); + + // Icon AND text, for the reason the Save button records: this row is a + // row of text buttons, so an icon on its own reads as a different kind + // of control than it is. + button->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); + + connect(button, &QToolButton::clicked, this, [this, filter]() { runFilter(filter); }); box->addWidget(button); } diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 6c1815c..e5487b8 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -96,6 +96,7 @@ private slots: void allSentQuerySkipsAccountsWithoutTheKey(); void allSentQueryJoinsEveryConfiguredAccount(); void aStoredGeneratedQueryIsUnpinnedNotDropped(); + void theFlaggedFilterIsCalledImportant(); void everyBuiltinFilterIsAKnownGenerator(); void aFilterAcrossAllAccountsIsTheUnscopedQuery(); void aTagFilterScopedToAnAccountCarriesThatAccountsPath(); @@ -1042,6 +1043,23 @@ static QString writeTwoAccounts(const QTemporaryDir &dir) "maildir=personal\n")); } +void TestConfig::theFlaggedFilterIsCalledImportant() +{ + // Item 57 decided this and item 93 contradicted it. The `flag` ACTION has + // read "&Important" since 0.14.0, chosen over "Starred" partly because &I + // was free where &S collided with Mark spam, and the filter shipped as + // "Flagged" beside it: the same tag under two names in one window. + // + // The generator keeps its own name, `flagged`. That string is stored in + // queries.json and matched against a closed set, so it is wire format and + // must not follow the label. + const SavedQuery filter = + Config::builtinFilter(QStringLiteral("flagged")); + + QCOMPARE(filter.name, QStringLiteral("Important")); + QCOMPARE(filter.generated, QStringLiteral("flagged")); +} + void TestConfig::everyBuiltinFilterIsAKnownGenerator() { // The guard for every case below. A filter whose generator is not in the @@ -1071,7 +1089,7 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() // surface for this would be built and deleted inside two items. QCOMPARE(names, (QStringList{ QStringLiteral("Unread"), QStringLiteral("Inbox"), - QStringLiteral("Flagged"), + QStringLiteral("Important"), QStringLiteral("Sent") })); } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 6a049e3..7563365 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -169,6 +169,7 @@ private slots: void narrowingAnEmptyQueryBarIsAPlainSearch(); void aMalformedAccountIsReportedWithoutBlockingTheConstructor(); void aWorkerBackedWindowReturnsRealThreads(); + void everyBuiltinFilterButtonCarriesAnIconAndItsText(); void aQueryInTheMenuCanActuallyBeRun(); void theFourBuiltinFiltersAreOnTheRowInOrder(); void aFilterComposesWithTheSelectedAccount(); @@ -5098,7 +5099,7 @@ void TestMainWindow::thereIsNoSentButtonWithoutASentKey() QVERIFY(config.allSentQuery().isEmpty()); MainWindow window(config); - QVERIFY(!window.findChild(QStringLiteral("sentButton"))); + QVERIFY(!window.findChild(QStringLiteral("sentButton"))); } void TestMainWindow::theSentButtonRunsEveryConfiguredAccount() @@ -5116,7 +5117,7 @@ void TestMainWindow::theSentButtonRunsEveryConfiguredAccount() })); MainWindow window(config); - auto *button = window.findChild(QStringLiteral("sentButton")); + auto *button = window.findChild(QStringLiteral("sentButton")); QVERIFY(button); auto *queryEdit = window.findChild(QStringLiteral("queryEdit")); @@ -5149,7 +5150,7 @@ void TestMainWindow::theSentButtonSurvivesABracketedPath() })); MainWindow window(config); - auto *button = window.findChild(QStringLiteral("sentButton")); + auto *button = window.findChild(QStringLiteral("sentButton")); QVERIFY(button); auto *queryEdit = window.findChild(QStringLiteral("queryEdit")); QVERIFY(queryEdit); @@ -5370,7 +5371,7 @@ void TestMainWindow::flatModeDoesNotSurviveTheNextQuery() MainWindow window(config); auto *model = window.findChild(); QVERIFY(model); - auto *button = window.findChild(QStringLiteral("sentButton")); + auto *button = window.findChild(QStringLiteral("sentButton")); QVERIFY(button); auto *queryEdit = window.findChild(QStringLiteral("queryEdit")); QVERIFY(queryEdit); @@ -5495,7 +5496,7 @@ static void loadWithQueries(Config &config, QTemporaryDir &dir, /// /// By object name, not by label: the labels are translated, and a user may name /// their own query "Unread" too. -static bool isBuiltinFilterButton(QPushButton *button) +static bool isBuiltinFilterButton(QAbstractButton *button) { for (const SavedQuery &filter : Config::builtinFilters()) { if (button->objectName() == filter.generated + QStringLiteral("Button")) @@ -5510,9 +5511,14 @@ static QStringList savedQueryButtonLabels(MainWindow &window) auto *row = window.findChild(QStringLiteral("savedQueryRow")); if (!row) return labels; - const QList buttons = - row->findChildren(QString(), Qt::FindDirectChildrenOnly); - for (QPushButton *button : buttons) { + // QAbstractButton, not QPushButton: the built-in filters are QToolButtons + // so they can carry an icon beside their text, and findChildren on the + // narrower type would silently skip them, leaving the filter below with + // nothing to filter. + const QList buttons = + row->findChildren(QString(), + Qt::FindDirectChildrenOnly); + for (QAbstractButton *button : buttons) { // The menu button is not a saved query and must not be counted as one. if (button->objectName() == QStringLiteral("savedQueryMenuButton")) continue; @@ -5920,7 +5926,7 @@ void TestMainWindow::aStoredGeneratedQueryRunsFlatAndComposed() MainWindow window(config); auto *button = - window.findChild(QStringLiteral("sentButton")); + window.findChild(QStringLiteral("sentButton")); QVERIFY(button); auto *queryEdit = @@ -5979,7 +5985,7 @@ void TestMainWindow::aRenamedSentEntryKeepsWorking() // The built-in Sent still resolves the same query, so nothing the user // could reach before became unreachable. auto *button = - window.findChild(QStringLiteral("sentButton")); + window.findChild(QStringLiteral("sentButton")); QVERIFY(button); auto *queryEdit = window.findChild(QStringLiteral("queryEdit")); @@ -6014,7 +6020,7 @@ void TestMainWindow::aGeneratedQueryWithNothingToShowIsSkipped() // missing Sent means it was skipped rather than that nothing was built. QCOMPARE(savedQueryButtonLabels(window), QStringList{ QStringLiteral("Inbox") }); - QVERIFY2(!window.findChild(QStringLiteral("sentButton")), + QVERIFY2(!window.findChild(QStringLiteral("sentButton")), "a generated query with nothing to show must not get a button"); } @@ -6100,7 +6106,7 @@ void TestMainWindow::onlyAStoredQueryOffersToBecomeATaggingRule() // By object name, which rebuildSavedQueryRow assigns precisely so a test // need not depend on a label the user can rename. - auto *generated = row->findChild( + auto *generated = row->findChild( QStringLiteral("sentButton")); // savedQueryButton(), not a scan for the label: item 93 puts a BUILT-IN @@ -6145,7 +6151,7 @@ void TestMainWindow::unpinningMovesAQueryToTheMenu() auto *row = window.findChild(QStringLiteral("savedQueryRow")); QVERIFY(row); QCOMPARE(savedQueryButtonLabels(window).size(), 2); - QVERIFY(!window.findChild( + QVERIFY(!window.findChild( QStringLiteral("savedQueryMenuButton"))); auto *button = savedQueryButton(window, QStringLiteral("Inbox")); @@ -6350,6 +6356,45 @@ void TestMainWindow::aWorkerBackedWindowReturnsRealThreads() QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); } +void TestMainWindow::everyBuiltinFilterButtonCarriesAnIconAndItsText() +{ + // The filters are part of the application now, so they carry icons like the + // Save button beside them rather than reading as bare text among the user's + // own queries. + // + // Text BESIDE the icon, not instead of it. The toolbar follows the + // desktop's own button style, but this row is a row of text buttons: an + // icon on its own here reads as a different kind of control than it is, + // which is the same argument the Save button records. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSentConfig(dir, { + {QStringLiteral("work"), QStringLiteral("Sent")}, + })); + + MainWindow window(config); + + for (const SavedQuery &filter : Config::builtinFilters()) { + auto *button = window.findChild( + filter.generated + QStringLiteral("Button")); + QVERIFY2(button, qPrintable(QStringLiteral("no button for filter '%1'") + .arg(filter.generated))); + + QCOMPARE(button->toolButtonStyle(), Qt::ToolButtonTextBesideIcon); + QCOMPARE(button->text(), filter.name); + + // Whether the icon RESOLVES depends on the running icon theme, which a + // test cannot assume: QIcon::fromTheme returns a null icon under a + // platform with no theme installed, so asserting on isNull() would fail + // for a reason that has nothing to do with this code. What is asserted + // is that a name was asked for, which is the part that lives here. + QVERIFY2(!button->icon().name().isEmpty(), + qPrintable(QStringLiteral("filter '%1' was given no themed " + "icon name").arg(filter.generated))); + } +} + void TestMainWindow::aQueryInTheMenuCanActuallyBeRun() { // An unpinned query was UNRUNNABLE. Its action carried both a triggered @@ -6434,8 +6479,10 @@ void TestMainWindow::theFourBuiltinFiltersAreOnTheRowInOrder() auto *row = window.findChild(QStringLiteral("savedQueryRow")); QVERIFY(row); + // QAbstractButton: the filters are QToolButtons, so they carry an icon + // beside their text like the Save button at the other end of the row. QStringList labels; - for (QPushButton *button : row->findChildren()) { + for (QAbstractButton *button : row->findChildren()) { // The overflow menu is a control over the set, not a member of it. if (button->objectName() == QStringLiteral("savedQueryMenuButton")) continue; @@ -6444,7 +6491,7 @@ void TestMainWindow::theFourBuiltinFiltersAreOnTheRowInOrder() QCOMPARE(labels, (QStringList{ QStringLiteral("Unread"), QStringLiteral("Inbox"), - QStringLiteral("Flagged"), + QStringLiteral("Important"), QStringLiteral("Sent") })); } @@ -6468,7 +6515,7 @@ void TestMainWindow::aFilterComposesWithTheSelectedAccount() QCOMPARE(window.selectedAccountForTesting(), QStringLiteral("work")); auto *unread = - window.findChild(QStringLiteral("unreadButton")); + window.findChild(QStringLiteral("unreadButton")); QVERIFY2(unread, "no built-in Unread button"); unread->click(); @@ -6478,7 +6525,7 @@ void TestMainWindow::aFilterComposesWithTheSelectedAccount() // Sent under the same account is the account's OWN folder, not the union // wrapped in a scope. See the Config test of the same name for why a row // count cannot tell the two apart. - auto *sent = window.findChild(QStringLiteral("sentButton")); + auto *sent = window.findChild(QStringLiteral("sentButton")); QVERIFY(sent); sent->click(); QCOMPARE(queryEdit->text(), QStringLiteral("path:\"work/Sent/**\"")); @@ -6503,7 +6550,7 @@ void TestMainWindow::aFilterAcrossAllAccountsIsUnscoped() QVERIFY(window.selectedAccountForTesting().isEmpty()); auto *unread = - window.findChild(QStringLiteral("unreadButton")); + window.findChild(QStringLiteral("unreadButton")); QVERIFY(unread); unread->click(); @@ -6526,7 +6573,7 @@ void TestMainWindow::aFilterDoesNotClearTheAccountSelection() window.selectAccountForTesting(QStringLiteral("work")); auto *unread = - window.findChild(QStringLiteral("unreadButton")); + window.findChild(QStringLiteral("unreadButton")); QVERIFY(unread); unread->click(); @@ -6588,7 +6635,7 @@ void TestMainWindow::aFilterOffersNoEditOrDeleteActions() MainWindow window(config); auto *unread = - window.findChild(QStringLiteral("unreadButton")); + window.findChild(QStringLiteral("unreadButton")); QVERIFY(unread); for (QAction *action : unread->actions()) { -- cgit v1.2.3