summaryrefslogtreecommitdiffstats
path: root/tests/test_mainwindow.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-07 18:22:45 +0200
committerDanilo M. <danix@danix.xyz>2026-08-07 18:22:45 +0200
commit48c243d7aee3382713f7a4653be2179d313b432c (patch)
treeb5eb63907e3f7cff18815844baccce71f4492dc1 /tests/test_mainwindow.cpp
parentd62a9eb9694beee1674f9c63f9a77029a81627ee (diff)
downloadqtmaildir-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.cpp90
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);