diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 18:22:45 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 18:22:45 +0200 |
| commit | 48c243d7aee3382713f7a4653be2179d313b432c (patch) | |
| tree | b5eb63907e3f7cff18815844baccce71f4492dc1 /tests/test_mainwindow.cpp | |
| parent | d62a9eb9694beee1674f9c63f9a77029a81627ee (diff) | |
| download | qtmaildir-48c243d7aee3382713f7a4653be2179d313b432c.tar.gz qtmaildir-48c243d7aee3382713f7a4653be2179d313b432c.zip | |
feat(ui): make Escape deselect the row as well as blank the pane
Blanking the pane while leaving the row highlighted reads as half an
action, and deselecting is what Escape means nearly everywhere else. The
user asked for two actions rather than a changed one, so clear_pane keeps
its behaviour and moves to Shift+Esc; clear_selection takes Escape and
does both. Shift+Esc rather than unbound because every action carries a
default and a test enforces it.
Clearing the selection re-adopts the thread it just cleared, unless done
in exactly the right way. clearSelection() leaves currentIndex() valid,
so onSelectionChanged takes its "one or fewer rows" branch, sees a
current row whose id differs from m_currentThreadId, and calls
onThreadSelected for it. Clearing the selection before blanking lets that
run while the id still matches, so nothing reloads, and clearing current
stops a later collapse-to-one-row reaching the same row. All four
arrangements were tried; only this one passes.
The first version of the test could not distinguish any of them. It
asserted showingPlaceholder(), which passes regardless because this
fixture has no worker, so loadThread never replies and the pane is never
repainted. currentThreadId() and currentIndex() are observable without
one, and asserting those is what made the test discriminate.
Diffstat (limited to 'tests/test_mainwindow.cpp')
| -rw-r--r-- | tests/test_mainwindow.cpp | 90 |
1 files changed, 89 insertions, 1 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 7ee8a5a..2625bf5 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -76,6 +76,8 @@ private slots: void growingASelectionCancelsAnAlreadyArmedTimer(); void collapsingBackToOneRowLoadsThatThreadAgain(); void theStatusBarReportsAMultiRowSelection(); + void clearSelectionBlanksThePaneAndDeselects(); + void clearPaneLeavesTheSelectionAlone(); void theThreadListOffersAContextMenu(); void aSecondRowBlanksThePaneNotOnlyAThird(); void aLocalSyncIsNotReportedAsABackgroundOne(); @@ -1096,6 +1098,87 @@ void TestMainWindow::theStatusBarReportsAMultiRowSelection() "size, it says '%1'").arg(status->text()))); } +void TestMainWindow::clearSelectionBlanksThePaneAndDeselects() +{ + // Item 50: the user asked for "two actions instead of one", so this is the + // new action and clearPaneLeavesTheSelectionAlone() below pins the old one. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}) }); + + // From a row that is already current, which is both how a user reaches this + // and what CLAUDE.md requires: selectAll() on a fresh view emits no + // currentRowChanged at all, so a test starting there passes against a + // missing guard. + view->selectRow(0); + QCOMPARE(view->selectionModel()->selectedRows().size(), 1); + + auto *action = window.findChild<QAction *>(QStringLiteral("clear_selection")); + QVERIFY2(action, "no clear_selection action"); + action->trigger(); + + QVERIFY2(view->selectionModel()->selectedRows().isEmpty(), + "the row is still selected: this action's whole point is that it " + "deselects as well as blanking"); + + // The hazard: clearSelection() leaves currentIndex() VALID, so + // onSelectionChanged() takes its "one or fewer rows" branch, sees a current + // row whose id differs from the just-cleared m_currentThreadId, and calls + // onThreadSelected for it, which sets m_currentThreadId again and sends a + // loadThread. The pane would then repaint itself a moment later. + // + // **currentThreadId() is what detects that, not the pane.** This fixture + // has no worker, so loadThread never replies and nothing ever repaints; + // asserting showingPlaceholder() here passes whatever the code does, which + // CLAUDE.md records as the standing limit of test_mainwindow. What IS + // observable is the id the window set on its way to that request. + QVERIFY2(window.currentThreadId().isEmpty(), + qPrintable(QStringLiteral("a thread was re-adopted after the " + "selection was cleared: currentThreadId " + "is '%1', and a loadThread for it is " + "already in flight") + .arg(window.currentThreadId()))); + + // And current itself is gone, so no later collapse-to-one-row can reload + // it either. + QVERIFY2(!view->currentIndex().isValid(), + "currentIndex is still valid, so onSelectionChanged can reload " + "that row on the next selection change"); +} + +void TestMainWindow::clearPaneLeavesTheSelectionAlone() +{ + // The pre-existing action keeps its behaviour. Item 32 built it to blank + // WITHOUT touching the selection, and a user who binds it is entitled to + // that; item 50 adds a second action rather than changing this one. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}) }); + + view->selectRow(0); + QCOMPARE(view->selectionModel()->selectedRows().size(), 1); + + auto *action = window.findChild<QAction *>(QStringLiteral("clear_pane")); + QVERIFY2(action, "no clear_pane action"); + action->trigger(); + + QCOMPARE(view->selectionModel()->selectedRows().size(), 1); +} + void TestMainWindow::theThreadListOffersAContextMenu() { // Right-click is the other half of discoverability: until now every tag @@ -1503,7 +1586,12 @@ void TestMainWindow::escapeBlanksTheMessagePane() auto *action = window.findChild<QAction *>(QStringLiteral("clear_pane")); QVERIFY2(action, "no clear_pane action registered"); - QCOMPARE(action->shortcut(), QKeySequence(Qt::Key_Escape)); + + // Shift+Esc since item 50: plain Escape now clears the selection too, and + // this narrower action kept the same key with a modifier. The behaviour + // asserted below is unchanged, which is the point of keeping both. + QCOMPARE(action->shortcut(), + QKeySequence(Qt::ShiftModifier | Qt::Key_Escape)); auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); |
