diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-06 20:06:45 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-06 20:06:45 +0200 |
| commit | ca89402d7e244e2616fdf13eb35c083afeb21826 (patch) | |
| tree | 34a3278510205e783e023f2c3cbcb2622e501067 /tests | |
| parent | eef7f6cc1b2f845d2ca1eba53336f64597c49e62 (diff) | |
| download | qtmaildir-ca89402d7e244e2616fdf13eb35c083afeb21826.tar.gz qtmaildir-ca89402d7e244e2616fdf13eb35c083afeb21826.zip | |
fix(ui): one Sync control, and a query bar that looks finished
Sync had two controls that behaved differently. The QPushButton beside the
query bar cleared the log, opened the pane and disabled itself; the QAction
behind the toolbar, menu and shortcut called start() and did nothing else,
discarding its return value so a rejected start was silent. Worse, item 29's
"disable Sync during a background sync" set the button only, so the toolbar
entry stayed clickable through a cron sync and could only produce the
script's EX_TEMPFAIL skip.
startSync() is now the single handler behind every route in, and the enabled
state lives on the QAction, which reaches the toolbar, the menu and the
shortcut at once. It also reports when no sync command is configured rather
than doing nothing.
The QPushButton is gone. It read as a Search button given it sat beside a
text field, which is the user's own observation and the reason the toolbar
one survives instead. Its unavailable-command tooltip moved to the action,
since that is the only thing that says why the control is dead.
Removing it left the query field running flush to the window edge, so the
saved-query buttons move from their own row onto the query row. The bar is
now framed by the account dropdown on the left and the saved queries on the
right, the empty row is gone, and the thread list gains the space. The field
also gains setClearButtonEnabled, which is Qt's own themed clear icon rather
than a hand-rolled button. A "Search" button was considered and rejected:
Return already runs the query.
No overflow handling for [queries], which is unbounded. Three entries fit;
item 23 already specifies buttons-plus-menu and is where that belongs.
CLAUDE.md's architecture diagram named four widget classes that have never
existed, QueryBar, SavedQueryBar, HeaderWidget and AttachmentBar. The query
row and the message header are built inline. Corrected, and the components
that do exist but were missing from it added.
Tests: the new action test was verified red first and load-bearing by
mutation. The old button test is deleted rather than repointed, being an
exact duplicate of it, and the unobservable-lock test now drives the action.
The clear button and the row layout were confirmed by hand; no test clicks
the icon, which is a mouse path.
Backlog: 45 done and reclassified as a defect rather than a cosmetic
redundancy, 47 added for the bar.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_mainwindow.cpp | 43 |
1 files changed, 19 insertions, 24 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 6a4f903..480be39 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -79,8 +79,8 @@ private slots: void aLocalSyncIsNotReportedAsABackgroundOne(); void aLocalSyncsOwnLockIsNeverReportedAsBackground(); void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); - void theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock(); void anUnobservableLockTableLeavesTheSyncButtonUsable(); + void theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock(); void escapeBlanksTheMessagePane(); void deleteTogglesOnAnAlreadyDeletedThread(); void deleteOnAMixedSelectionDeletesRatherThanSplittingIt(); @@ -985,11 +985,13 @@ void TestMainWindow::aSkippedLocalSyncStillReportsTheOtherRunFinishing() "says '%1'").arg(status->text()))); } -void TestMainWindow::theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock() +void TestMainWindow::theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock() { - // Item 27 specified this and it shipped unbuilt: while a cron sync holds - // the lock the button stayed clickable, and pressing it could only produce - // the EX_TEMPFAIL skip. + // Item 29 shipped for the QPushButton only: onExternalSyncStateChanged + // disabled m_syncButton and never touched the QAction, so the toolbar and + // menu Sync stayed clickable during a cron sync and could only produce the + // EX_TEMPFAIL skip. The button-based test passed throughout, because it + // drove the half that worked. QTemporaryDir dir; QVERIFY(dir.isValid()); QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); @@ -999,10 +1001,6 @@ void TestMainWindow::theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock() s.setValue(QStringLiteral("sync/command"), QStringLiteral("/bin/true")); } - // An empty lock table, so construction observes no sync. Against the real - // /proc/locks this assertion fails whenever the user's cron sync happens to - // be running: cron fires every ten minutes and a run lasts ~35s, so roughly - // 6% of runs landed inside one and the failure looked like flakiness. const QString locks = dir.filePath(QStringLiteral("locks")); { QFile f(locks); @@ -1014,25 +1012,22 @@ void TestMainWindow::theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock() config.load(conf); MainWindow window(config); - auto *button = window.findChild<QPushButton *>(QStringLiteral("syncButton")); - QVERIFY2(button, "no sync button to check"); - QVERIFY2(button->isEnabled(), "the button starts disabled with a command set"); + auto *action = window.findChild<QAction *>(QStringLiteral("sync")); + QVERIFY2(action, "no sync action to check"); + QVERIFY2(action->isEnabled(), "the action starts disabled with a command set"); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Running)); - QVERIFY2(!button->isEnabled(), - "the sync button stayed enabled during a background sync"); + QVERIFY2(!action->isEnabled(), + "the sync action stayed enabled during a background sync"); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Idle)); - QVERIFY2(button->isEnabled(), - "the sync button was not re-enabled after the background sync"); + QVERIFY2(action->isEnabled(), + "the sync action was not re-enabled after the background sync"); - // The override is process-wide, and QTemporaryDir takes the file with it at - // the end of this scope: leaving it set would point every later window at a - // path that no longer exists. MainWindow::setLocksPathForTesting(QStringLiteral("/proc/locks")); } @@ -1054,19 +1049,19 @@ void TestMainWindow::anUnobservableLockTableLeavesTheSyncButtonUsable() config.load(conf); MainWindow window(config); - auto *button = window.findChild<QPushButton *>(QStringLiteral("syncButton")); - QVERIFY(button); + auto *action = window.findChild<QAction *>(QStringLiteral("sync")); + QVERIFY(action); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Running)); - QVERIFY(!button->isEnabled()); + QVERIFY(!action->isEnabled()); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Unknown)); - QVERIFY2(button->isEnabled(), - "an unobservable lock table left the sync button disabled"); + QVERIFY2(action->isEnabled(), + "an unobservable lock table left the sync action disabled"); } void TestMainWindow::escapeBlanksTheMessagePane() |
