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. --- tests/test_mainwindow.cpp | 87 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 20 deletions(-) (limited to 'tests/test_mainwindow.cpp') 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