diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-04 18:16:31 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 18:16:31 +0200 |
| commit | 987a9e728995cbbfc77e470db72d552ebe096ba2 (patch) | |
| tree | 595164649d7fb49b73b98240382e6df6982d9123 /tests/test_mainwindow.cpp | |
| parent | 1407f70352c05d59fa2e1bb7c59b048b767972e5 (diff) | |
| download | qtmaildir-987a9e728995cbbfc77e470db72d552ebe096ba2.tar.gz qtmaildir-987a9e728995cbbfc77e470db72d552ebe096ba2.zip | |
feat(ui): make multi-select discoverable and stop it opening threads
Multi-select already worked by Ctrl+click and Shift+click, but nothing in
the UI said so and every tag action was keyboard-only, so the Ctrl+T tag
dialog could not be reached with a mouse at all.
Adds a select_all action on Ctrl+A, registered like every other action so
it reaches the Edit menu, the shortcut reference and [keys]; a right-click
menu on the thread list built from the same QActions rather than parallel
copies; a selection count in the status bar, which is the part that
actually teaches the feature by acknowledging a selection while it is
being built; and a note in the shortcut dialog for the mouse gestures,
which are view behaviour and cannot appear in the generated table.
A selection gesture must not open mail or mutate it. Selecting several
rows now blanks the message pane and cancels any pending mark-read,
rather than rendering each row swept through and queueing it to be marked
read.
Two Qt behaviours shaped this, both established by probe rather than from
memory:
- selectAll() emits no currentRowChanged at all and leaves the current
index invalid.
- currentRowChanged is emitted BEFORE the selection model is updated.
The second one caused two distinct faults. Collapsing a multi-row
selection back to one row reported the old count, so the guard swallowed
the load and the pane stayed blank; that case is handled in
onSelectionChanged, which sees the true count. And a Ctrl+click taking
the selection from one row to two also reported one, so the thread was
loaded, blanked, and then painted back when the queued reply returned
from the worker. By the third row the id was already cleared and the
reply was discarded, which is why the fault presented as an off-by-one in
the threshold rather than as a race.
Tests cover the synchronous half. The late-reply guard has no test:
MainWindow in tests has no worker, so threadLoaded never fires and the
repaint cannot be reproduced in process. Verified by hand instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'tests/test_mainwindow.cpp')
| -rw-r--r-- | tests/test_mainwindow.cpp | 255 |
1 files changed, 255 insertions, 0 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index d307a99..a2142b9 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -25,6 +25,7 @@ #include <QKeyEvent> #include <QLabel> #include <QLineEdit> +#include <QMenu> #include <QFile> #include <QSettings> #include <QStandardPaths> @@ -66,6 +67,13 @@ private slots: void aFailedSyncDoesNotClearThePendingCount(); void closingWithNoPendingEditsDoesNotPrompt(); void syncOnExitNeverClosesSilently(); + void selectAllIsBoundAndSelectsEveryRow(); + void aMultiRowSelectionDoesNotArmTheMarkReadTimer(); + void growingASelectionCancelsAnAlreadyArmedTimer(); + void collapsingBackToOneRowLoadsThatThreadAgain(); + void theStatusBarReportsAMultiRowSelection(); + void theThreadListOffersAContextMenu(); + void aSecondRowBlanksThePaneNotOnlyAThird(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -591,6 +599,253 @@ void TestMainWindow::syncOnExitNeverClosesSilently() "sync_on_exit=never must close without prompting"); } +void TestMainWindow::selectAllIsBoundAndSelectsEveryRow() +{ + // Multi-select already worked by Ctrl+click and Shift+click; what was + // missing was a keyboard and menu route to it. The action has to exist as a + // registered action, not as a raw view shortcut, so it reaches the menu, + // the shortcut reference and [keys] like every other binding. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("select_all")); + QVERIFY2(action, "no select_all action registered"); + QCOMPARE(action->shortcut(), QKeySequence(QStringLiteral("Ctrl+A"))); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}), + makeThread(QStringLiteral("t3"), {}) }); + + action->trigger(); + + QCOMPARE(view->selectionModel()->selectedRows().size(), 3); +} + +void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer() +{ + // A selection gesture must never mutate mail. current follows the keyboard + // cursor as a selection extends, so without a guard every row swept through + // by Shift+arrow would be queued to be marked read: threads the user only + // ever selected, never opened. + // + // Note selectAll() on a fresh view is NOT the case to test here: it leaves + // current invalid and emits no currentRowChanged at all (verified against + // Qt 6.11), so it would pass without any guard in place. The real path is a + // row already current, which is how a user reaches select-all: click a + // thread, then Ctrl+A. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t3"), + { QStringLiteral("unread") }) }); + + // Sweep down as Shift+arrow does: current moves onto a row while the + // selection already spans more than one. + view->selectRow(0); + view->selectionModel()->select( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + view->selectionModel()->setCurrentIndex( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + + QVERIFY2(view->selectionModel()->selectedRows().size() > 1, + "test setup failed to build a multi-row selection"); + QVERIFY2(!timer->isActive(), + "a multi-row selection armed the mark-read timer"); +} + +void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer() +{ + // The ordering trap: clicking one row arms the timer legitimately, and only + // then does the selection grow. Guarding the new selection alone is not + // enough, the timer already running for the first row has to be cancelled + // or that thread goes read behind a pane that no longer shows it. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }) }); + + view->selectRow(0); + QVERIFY2(timer->isActive(), "no timer armed for a single unread thread"); + + // Extend to a second row, as Shift+click would. + view->selectionModel()->select( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + + QVERIFY2(!timer->isActive(), + "extending the selection left the first row's timer running"); +} + +void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain() +{ + // The guard must not be a one-way door. Narrowing a multi-row selection + // back to a single row is ordinary reading again, so the timer arms as it + // always did. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }) }); + + view->selectAll(); + QVERIFY(!timer->isActive()); + + // Back to one row, as a plain click would leave it. + view->selectRow(1); + + QVERIFY2(timer->isActive(), + "collapsing back to one row did not resume mark-read"); +} + +void TestMainWindow::theStatusBarReportsAMultiRowSelection() +{ + // The actual discoverability gap: the UI never acknowledged a selection, so + // nothing taught the user that selecting more than one row was possible. + // A count that appears while the selection is being built does. + 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")); + QVERIFY2(status, "no status label to report into"); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}), + makeThread(QStringLiteral("t3"), {}) }); + + view->selectAll(); + + QVERIFY2(status->text().contains(QStringLiteral("3")), + qPrintable(QStringLiteral("status bar does not report the selection " + "size, it says '%1'").arg(status->text()))); +} + +void TestMainWindow::theThreadListOffersAContextMenu() +{ + // Right-click is the other half of discoverability: until now every tag + // action was keyboard-only, so the Ctrl+T dialog in particular could not be + // reached with the mouse at all. + const Config config; + MainWindow window(config); + + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + QCOMPARE(view->contextMenuPolicy(), Qt::CustomContextMenu); + + // The menu must reuse the registered QActions rather than build parallel + // ones, or a [keys] rebinding would show the old shortcut here and the + // menu could drift out of step with what the keyboard really does. + auto *menu = window.findChild<QMenu *>(QStringLiteral("threadContextMenu")); + QVERIFY2(menu, "no thread-list context menu"); + + const QStringList expected = { QStringLiteral("archive"), + QStringLiteral("delete"), + QStringLiteral("spam"), + QStringLiteral("toggle_unread"), + QStringLiteral("edit_tags"), + QStringLiteral("flag") }; + for (const QString &name : expected) { + QAction *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no action '%1'").arg(name))); + QVERIFY2(menu->actions().contains(action), + qPrintable(QStringLiteral("context menu is missing the " + "registered '%1' action").arg(name))); + } +} + +void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird() +{ + // Reported by hand testing: selecting a second thread left it displayed, + // and only a third blanked the pane. The cause is that currentRowChanged is + // emitted before the selection model updates, so the Ctrl+click that makes + // the count two arrives at onThreadSelected still reporting one, which + // loads the thread; onSelectionChanged then blanks the pane, and the load, + // being queued to the worker, paints over the blank when it returns. By the + // third row m_currentThreadId is already cleared, so the late result is + // discarded and the blank survives, which is why the fault looked like an + // off-by-one in the threshold rather than a race. + // + // Two rows must behave exactly as three do. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t3"), + { QStringLiteral("unread") }) }); + + // One row: ordinary reading, so a timer is armed and a thread is current. + view->selectRow(0); + QCOMPARE(view->selectionModel()->selectedRows().size(), 1); + QVERIFY(timer->isActive()); + + // Ctrl+click a second row. This is the exact gesture that failed: the + // selection becomes two while currentRowChanged still reports one. + view->selectionModel()->setCurrentIndex( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + + QCOMPARE(view->selectionModel()->selectedRows().size(), 2); + QVERIFY2(!timer->isActive(), + "two selected rows left the mark-read timer armed"); + + // A blanked pane is one with no current thread: anything still in flight + // for that id would repaint over it. + QVERIFY2(window.currentThreadId().isEmpty(), + qPrintable(QStringLiteral("two selected rows left thread '%1' " + "loaded in the pane") + .arg(window.currentThreadId()))); +} + // 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. |
