diff options
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 40 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 80 | ||||
| -rw-r--r-- | src/mainwindow.h | 22 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 110 |
4 files changed, 239 insertions, 13 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 7f79100..67668d4 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -60,7 +60,7 @@ taking that too literally. | 13 | No visual feedback that an action stuck | feedback | S | **done** | | 14 | Tag column unreadable, tags need another home | presentation | M | **done** | | 15 | Attachments are parsed but unreachable from the UI | information | M | **done** | -| 16 | Delete on an already-deleted thread should undelete | behavior | S | open | +| 16 | Delete on an already-deleted thread should undelete | behavior | S | **done** | | 17 | No completion for tags in the query bar | workflow | M | **done** | | 18 | No visual cue that there are unsynced edits | feedback | S | **done** | | 19 | No prompt to sync on exit when edits are pending | behavior | S | **done** | @@ -77,7 +77,7 @@ taking that too literally. | 30 | The blank right pane is wasted space | presentation | M | open | | 31 | The quit prompt has no highlighted default button | discoverability | XS | **done** | | 32 | Esc does not blank the right pane | workflow | XS | **done** | -| 33 | Status bar messages never expire | feedback | S | open | +| 33 | Status bar messages never expire | feedback | S | **done** | | 34 | No overview of the Maildir itself | information | M | open | | 35 | No refresh of the thread list after a sync | workflow | M | open | | 36 | `test_mainwindow` cannot reach the worker | testing | S | open, on demand | @@ -797,6 +797,18 @@ Deleting a thread that already carries `deleted` removes it instead. - The same question applies to `spam` and `flag`. Do not change those in this item; note whether the answer generalises once `delete` is built. +### Outcome (done) + +Built as specced, including the all-or-nothing rule: undelete only when every +selected thread already carries `deleted`, otherwise delete the whole selection. +A test covers the mixed case and passed before the change, since the old +always-delete behaviour satisfies it; it is there to stop a later "improvement" +from toggling per row. + +**It generalises to `spam` and `flag`, and they were still left alone.** The +same shape would work, but neither has been asked for, and `flag` in particular +is already reachable both ways through the tag dialog. + ## 17. No completion for tags in the query bar **Observed:** typing a query means remembering the exact tag name, including @@ -1583,6 +1595,30 @@ completes. pane, which does persist, but the status text should outlast a two-second timeout. +### Outcome (done) + +`showTransientStatus()` sets the text and arms a 6 s single-shot timer that +restores the last query's thread count. Messages were classified rather than +blanket-timed, which is the whole substance of the item: + +- **Events expire:** "Sync complete", "Nothing to undo", "Sync already + running", the skip notice, the background-sync notice, and the per-action + "Archive: 3 threads". +- **State persists:** "Searching...", "Syncing...", "Syncing before + quitting...", "Background sync running...", the selection count, and + **"Sync failed (exit N)"**, per the constraint that an error must not vanish + before it is read. + +**A test caught a real mistake while routing them.** Making the per-action +message transient armed the timer during `selectAll()`, because `tagSelected()` +runs on a selection that `onSelectionChanged()` had just described. The count is +state and must outlive any transient still counting down, so writing it now +cancels the timer. + +`QStatusBar::showMessage()` was considered and not used: the label is added with +`addWidget()` alongside permanent widgets, so switching would mean reworking that +arrangement for the same behaviour. + ## 34. No overview of the Maildir itself **Observed (user, 2026-08-04):** wants "info on the maildir": total messages, diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index b62d8d7..a7f9b40 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -337,6 +337,26 @@ void MainWindow::buildUi() m_statusLabel->setObjectName(QStringLiteral("statusMessage")); statusBar()->addWidget(m_statusLabel); + // Transient messages describe an EVENT and go stale: "Sync complete" reads + // as the present tense until something else overwrites it. State messages, + // the selection count above all, describe what is true right now and must + // not expire while it stays true, so only showTransientStatus() arms this. + // + // ponytail: one timer beside the label, not QStatusBar::showMessage(). + // That would mean moving off addWidget() and reworking the permanent + // widgets beside it, for the same behaviour. + m_statusTimer = new QTimer(this); + m_statusTimer->setObjectName(QStringLiteral("statusTimer")); + m_statusTimer->setSingleShot(true); + m_statusTimer->setInterval(kStatusMessageMs); + connect(m_statusTimer, &QTimer::timeout, this, [this]() { + // Only take back a message this timer armed. Anything written since is + // newer and more relevant than the default. + if (m_statusLabel->text() == m_transientMessage) + m_statusLabel->setText(m_defaultStatus); + m_transientMessage.clear(); + }); + // Beside the sync status rather than as a widget competing with it: the two // say related things and reading them apart would be worse than reading // them together. @@ -428,7 +448,7 @@ void MainWindow::buildUi() } connect(m_syncButton, &QPushButton::clicked, this, [this]() { if (!m_sync->start()) { - m_statusLabel->setText(tr("Sync already running")); + showTransientStatus(tr("Sync already running")); return; } // Fresh run, fresh output: leaving the previous run's lines in place @@ -594,8 +614,29 @@ void MainWindow::registerActions() tagSelected({}, { QStringLiteral("inbox") }, tr("Archive")); }); addAction(QStringLiteral("delete"), tr("&Delete"), - tr("Add the deleted tag"), [this]() { - tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete")); + tr("Add or remove the deleted tag"), [this]() { + // A toggle, like toggle_unread: pressing Delete twice is the natural + // way to say "no, put it back", and adding a tag that is already there + // is a no-op the user cannot see. + // + // One direction for the WHOLE selection. Toggling each thread + // independently would leave one keystroke with the selection in two + // states, which is worse than either outcome, so undelete only when + // every selected thread is already deleted. + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + bool allDeleted = !rows.isEmpty(); + for (const QModelIndex &index : rows) { + if (!m_model->threadAt(index.row()).isDeleted()) { + allDeleted = false; + break; + } + } + + if (allDeleted) + tagSelected({}, { QStringLiteral("deleted") }, tr("Undelete")); + else + tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete")); }); addAction(QStringLiteral("spam"), tr("Mark &spam"), tr("Add spam and remove inbox"), [this]() { @@ -675,7 +716,7 @@ void MainWindow::registerActions() if (m_undoStack.canUndo()) m_undoStack.undo(); else - m_statusLabel->setText(tr("Nothing to undo")); + showTransientStatus(tr("Nothing to undo")); }); addAction(QStringLiteral("sync"), tr("&Sync"), tr("Run the configured sync command"), [this]() { @@ -1061,7 +1102,11 @@ void MainWindow::onQueryFinished(int total, quint64 generation) { if (generation != m_generation) return; - m_statusLabel->setText(tr("%n thread(s)", "", total)); + // The query's own result is what the bar says when nothing more pressing + // is happening, so a transient message falls back to it rather than to + // nothing. + m_defaultStatus = tr("%n thread(s)", "", total); + m_statusLabel->setText(m_defaultStatus); } void MainWindow::showThreadContextMenu(const QPoint &pos) @@ -1112,6 +1157,12 @@ void MainWindow::onSelectionChanged() m_selectionMessage = tr("%n thread(s) selected", "", selected); m_statusLabel->setText(m_selectionMessage); + // State, not an event: it must persist while the selection does. Cancel any + // transient message still counting down, or that timer fires and replaces a + // count that is still true. + m_statusTimer->stop(); + m_transientMessage.clear(); + // Ctrl+click and selectAll() reach a multi-row selection without moving // current, so onThreadSelected never runs and its guard never fires. The // pane and the pending timer have to be dealt with here as well. @@ -1251,7 +1302,7 @@ void MainWindow::onSyncFinished(bool success, int exitCode) m_pendingEdits = 0; updatePendingIndicator(); - m_statusLabel->setText(tr("Sync complete")); + showTransientStatus(tr("Sync complete")); if (m_syncingForExit) { // The work is safely across, so finish the quit the user asked for. @@ -1274,8 +1325,8 @@ void MainWindow::onSyncFinished(bool success, int exitCode) // Not a failure: another run holds the lock and is doing the work. // The user's cron fires every ten minutes, so a click landing inside // one is routine and must not raise an error or the log pane. - m_statusLabel->setText(tr("A sync is already running (started " - "elsewhere); this one was skipped")); + showTransientStatus(tr("A sync is already running (started " + "elsewhere); this one was skipped")); if (m_syncingForExit) { // The other run is syncing, but this application cannot see when @@ -1374,12 +1425,19 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state) // be read, so nothing was observed, and "sync finished" would be a claim // this cannot support. if (state == SyncMonitor::State::Idle) { - m_statusLabel->setText( + showTransientStatus( tr("Background sync completed. Press Enter in the query bar to " "refresh.")); } } +void MainWindow::showTransientStatus(const QString &text) +{ + m_transientMessage = text; + m_statusLabel->setText(text); + m_statusTimer->start(); +} + void MainWindow::setSyncBusy(bool busy) { m_localSyncBusy = busy; @@ -1501,7 +1559,7 @@ void MainWindow::editTagsOnSelection() const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); if (rows.isEmpty()) { - m_statusLabel->setText(tr("Select a thread first")); + showTransientStatus(tr("Select a thread first")); return; } @@ -1551,7 +1609,7 @@ void MainWindow::tagSelected(const QStringList &add, const QStringList &remove, m_undoStack.push(new ThreadTagCommand(this, threadIds, add, remove, description)); - m_statusLabel->setText( + showTransientStatus( tr("%1: %n thread(s)", "", threadIds.size()).arg(description)); } diff --git a/src/mainwindow.h b/src/mainwindow.h index 61722aa..2edeb79 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -84,6 +84,11 @@ public: /// `assets/mailsync.sh` is the reference implementation of this contract. static constexpr int kSyncSkippedExitCode = 75; + /// How long a transient status message stays before the bar falls back to + /// the thread count. Long enough to read a sentence, short enough that a + /// stale "Sync complete" does not sit there describing the present. + static constexpr int kStatusMessageMs = 6000; + /// Path of the machine-written UI state file. Deliberately not /// Config::defaultPath(): the config is hand-edited and must never gain a /// base64 geometry blob, nor be rewritten on exit (QSettings does not @@ -115,6 +120,14 @@ private slots: void onWorkerError(const QString &message); void onSyncFinished(bool success, int exitCode); + /// Shows a message that describes an event and takes it back after a few + /// seconds, restoring the last query's thread count. + /// + /// Use this for events ("Sync complete"), never for state: the selection + /// count must persist while the selection does. A private slot so tests can + /// drive it through the meta-object. + void showTransientStatus(const QString &text); + /// Reacts to a sync started outside this window, by cron or by hand. /// /// A private slot rather than a plain method so tests can drive it through @@ -254,6 +267,15 @@ private: QPushButton *m_syncButton = nullptr; QLabel *m_statusLabel = nullptr; + /// Expires a transient status message. See showTransientStatus(). + QTimer *m_statusTimer = nullptr; + + /// The message m_statusTimer armed for, so it takes back only its own. + QString m_transientMessage; + + /// What the status bar falls back to: the last query's thread count. + QString m_defaultStatus; + /// Says how many tag changes have not been seen to reach the mail store. /// Hidden entirely at zero rather than reading "0 unsynced", which is noise. QLabel *m_pendingLabel = nullptr; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 8a1dde3..f791d8f 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -82,6 +82,10 @@ private slots: void theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock(); void anUnobservableLockTableLeavesTheSyncButtonUsable(); void escapeBlanksTheMessagePane(); + void deleteTogglesOnAnAlreadyDeletedThread(); + void deleteOnAMixedSelectionDeletesRatherThanSplittingIt(); + void aTransientStatusMessageExpires(); + void theSelectionCountIsStateAndDoesNotExpire(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -1070,6 +1074,112 @@ void TestMainWindow::escapeBlanksTheMessagePane() QCOMPARE(view->selectionModel()->selectedRows().size(), 1); } +void TestMainWindow::deleteTogglesOnAnAlreadyDeletedThread() +{ + // Hitting Delete twice is the natural way to say "no, put it back", and + // adding a tag that is already present is a no-op the user cannot see. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("deleted") }) }); + view->selectRow(0); + + action->trigger(); + + // The optimistic model update is synchronous, so the row reflects the + // change without a worker. + QVERIFY2(!model->threadAt(0).isDeleted(), + "delete on an already-deleted thread did not undelete it"); +} + +void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt() +{ + // The constraint that makes this more than a one-liner: toggling each + // thread independently would leave one keystroke with the selection in two + // states, which is worse than either outcome. Undelete only when every + // selected thread is already deleted. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("deleted") }), + makeThread(QStringLiteral("t2"), {}) }); + + view->selectAll(); + QCOMPARE(view->selectionModel()->selectedRows().size(), 2); + + action->trigger(); + + QVERIFY2(model->threadAt(0).isDeleted() && model->threadAt(1).isDeleted(), + "a mixed selection split instead of deleting the whole selection"); +} + +void TestMainWindow::aTransientStatusMessageExpires() +{ + // "Sync complete" describes an event, not a state, and reads as though it + // describes the present until something else overwrites it. + const Config config; + MainWindow window(config); + + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + auto *timer = window.findChild<QTimer *>(QStringLiteral("statusTimer")); + QVERIFY2(timer, "no status expiry timer"); + + QMetaObject::invokeMethod(&window, "showTransientStatus", + Q_ARG(QString, QStringLiteral("Sync complete"))); + QCOMPARE(status->text(), QStringLiteral("Sync complete")); + QVERIFY(timer->isActive()); + + // Fire it rather than waiting out the real interval. + timer->setInterval(0); + QTRY_VERIFY_WITH_TIMEOUT(status->text() != QStringLiteral("Sync complete"), + 2000); +} + +void TestMainWindow::theSelectionCountIsStateAndDoesNotExpire() +{ + // Not everything in the status bar is an event. The selection count + // describes what is true right now and must persist while it stays true; + // expiring it would undo the 0.8.0 discoverability work. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + auto *timer = window.findChild<QTimer *>(QStringLiteral("statusTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}) }); + view->selectAll(); + + QVERIFY2(status->text().contains(QStringLiteral("2")), + "the selection count was not reported"); + QVERIFY2(!timer->isActive(), + "the selection count armed the expiry timer; it is state, " + "not an event"); +} + // Constructing a MainWindow needs a QApplication and a platform plugin. The // test has no display under ctest, so it runs offscreen unless the caller // asked for something else. |