summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-15 16:35:56 +0200
committerDanilo M. <danix@danix.xyz>2026-08-15 16:35:56 +0200
commit6d35f7ec87590fd484ca7640d4d06c06a6d97970 (patch)
treea465275f74b376209683cc63f9ec622f9dcc59aa
parentae0c0ec339dba2ce9f2b27a9ce289feb4719dcea (diff)
downloadqtmaildir-6d35f7ec87590fd484ca7640d4d06c06a6d97970.tar.gz
qtmaildir-6d35f7ec87590fd484ca7640d4d06c06a6d97970.zip
feat(ui): open a thread on its own by double-clicking a row
Double-clicking any row drills into its thread: the list becomes that thread alone, expanded, and the pane shows the double-clicked row's own message. A reply therefore opens its WHOLE thread with itself selected, never itself alone, which is what the user asked for and is not the obvious reading of "open it by itself". This is recoverStaleThread() triggered by a gesture. That function already ran thread:<id>, expanded the thread when the row arrived, selected the target message once the replies landed, and fell back to the root when the message had gone; all three cases are existing paths through it, so the new code resolves a row to a thread id and a message id and hands both over. The row is reached through the INDEX and never through index.row(): a tree numbers rows per parent, so threadAt(row) on a reply answers about an unrelated thread. That is item 88's trap, avoided here by construction. The first click of a double-click arms the mark-read timer, and the handler cancels it, because a gesture that navigates must not mutate mail. The timer is armed again for whichever row the recovery lands on, so only the arming for the row being left is cancelled. Its test asserts the timer was active beforehand, so it cannot pass by the timer never having been armed at all. The expander keeps its own double-click: ThreadListView::mousePressEvent accepts a press inside its rect and returns, so Qt never pairs one into a double-click there. Nothing is built for getting back. The filter buttons already are that, per the user. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md100
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md68
-rw-r--r--src/mainwindow.cpp54
-rw-r--r--src/mainwindow.h4
-rw-r--r--tests/test_mainwindow.cpp142
5 files changed, 301 insertions, 67 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
index ef5c68f..30271d5 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
@@ -5524,3 +5524,103 @@ name their tags, so the UI can answer from `~/.config/mailrules/rules.json` at
the moment it is asked, retroactively and with nothing stored. Reopen from
there, not from the provenance design, unless real use shows the per-message
distinction is what is wanted.
+
+## 91. Double-clicking a thread should open it on its own
+
+**Observed (user, notes):** "double clicking a message/thread should open it in
+its own view (?) UX not sure."
+
+**Specified by the user, 2026-08-15**, in three cases. The view is always the
+whole THREAD; what changes is which message the pane shows:
+
+- Double-click a single message: that message in a view by itself, loaded in
+ the right pane.
+- Double-click a thread: the whole thread, expanded, in a view by itself, with
+ the FIRST message in the right pane.
+- Double-click a reply inside a thread: the whole thread, expanded, in a view by
+ itself, with THAT REPLY in the right pane.
+
+So a reply does not drill down to itself alone. It drills to its thread, with
+itself selected. That is the part an implementation is most likely to get wrong,
+because the obvious reading of "open it by itself" is an `id:` query on the
+reply, and it is not what was asked for.
+
+**It is NOT a second window.** The user ruled that out explicitly: "I didn't
+mean open in a new window." No second `QWebEngineView`, no extra Chromium render
+process, nothing reparented.
+
+**Cause (verified in code):** nothing is connected to `doubleClicked` on
+`ThreadListView`, so a double-click today is two single-clicks and selects the
+row twice.
+
+**Approach: this is `recoverStaleThread()` triggered by a gesture.** That
+function already does the entire job for the stale-thread notice: it sets
+`m_recoverThreadId` and `m_recoverMessageId`, runs a `thread:` query, expands
+the thread when the row arrives, selects the target reply once the replies land,
+and falls back to selecting the thread when the message has gone. Every case
+above maps onto it, with the recovery target set to the double-clicked row's own
+message. Reuse it rather than writing a second selection-after-query path.
+
+Two properties of that code are exactly what this item needs and are already
+commented in place: the root card IS the thread's first message and is not among
+the children, so a thread double-click is the empty-target case; and a reply
+cannot be selected until the replies exist, so the first pass selects the thread
+provisionally and refines on the next.
+
+**Expansion is not automatic and must be asked for.** Nothing in the tree
+auto-expands: `expandAll` and `setExpanded` appear nowhere in `mainwindow.cpp`,
+so a plain `thread:` query lands on ONE COLLAPSED CARD and the user would still
+have to click the expander. `recoverStaleThread()` calls `expand()` on the
+thread, which is also what asks the worker for the replies.
+
+**How the user gets back: nothing is built for it.** Answered by the user:
+"I didn't think of a back action, usually I'd go back to a known list like
+unread or inbox at that point." The filter buttons already are that and are one
+click away in every view. A Back action, a history stack and a
+restore-the-previous-query scheme were all drafted and are all unnecessary.
+
+**The undo-stack cost is inherited and acceptable.** `runQuery()` clears the
+undo stack because its rows are about to be discarded, and a drill-down discards
+them exactly as a typed query does. The user already expects that from the query
+bar, so the gesture inherits consistent behaviour rather than a surprise. No
+filter-over-the-model alternative is needed, which is what would have made this
+item 40's machinery and M rather than S.
+
+**Constraint.** A double-click also delivers a single click first, which selects
+the row and arms the mark-read timer. The drill-down must not mark a message
+read that the user only passed through, and the existing single-click path is
+what arms it, so the double-click handler has to cancel it the way the
+multi-row branch of `onThreadSelected()` does.
+
+**Outcome, 2026-08-15. Shipped, hand-confirmed green by the user.**
+
+`onRowDoubleClicked()` resolves the row to a thread id and a message id and
+hands both to `recoverStaleThread()`, which already did the entire job for the
+stale-thread notice: run `thread:<id>`, expand the thread when the row arrives,
+select the target message once the replies land, fall back to the root when the
+message has gone. All three of the user's cases are existing paths through it,
+so the feature is a connection and a resolver rather than new machinery.
+
+Two details that were nearly built wrong:
+
+- **A reply drills to its THREAD, not to itself.** The obvious reading of "open
+ it by itself" is `id:<reply>`, and one of the three tests exists specifically
+ to fail against that.
+- **The row is reached through the INDEX, never `index.row()`.** A tree numbers
+ rows per parent, so `threadAt(row)` on a reply answers about an unrelated
+ thread. This is item 88's trap, avoided here by construction.
+
+**The first click of a double-click arms the mark-read timer**, and the handler
+cancels it: a gesture that navigates must not mutate mail. The timer is armed
+again for whichever row the recovery lands on, so this cancels only the arming
+for the row being left. Its test asserts the timer WAS active before the
+double-click, so it cannot pass by the timer never having been armed.
+
+**The expander keeps its own double-click.** `ThreadListView::mousePressEvent`
+accepts a press inside the expander rect and returns, so Qt never pairs one into
+a double-click there: double-clicking the reply count toggles expansion twice,
+double-clicking anywhere else on the card drills in. That is the right split and
+it falls out of the existing code rather than needing a guard.
+
+**Nothing was built for getting back**, per the user: the filter buttons already
+are that.
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 aefa6a5..7b44e7e 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
@@ -155,7 +155,7 @@ taking that too literally.
| 86 | A right-click search can replace or narrow, but never exclude | workflow | S | **done** 2026-08-14, unreleased; see `specs/2026-08-14-exclude-from-search-design.md`. Follows 85. The `extend` bool became a `SearchMode` enum across four signatures |
| 89 | A sync moves the list under the user's hands, and the auto-sync skips rather than retries | workflow | XS | **done** 2026-08-15, unreleased. The timer half only: a skipped auto-sync re-arms instead of giving up. The list-churn half is **dropped**, not built: the user resolved it as a mental-model question, an Unread view is SUPPOSED to be volatile |
| 90 | A saved-query button clears the account selection | workflow | S | **folded into 93** 2026-08-15. Not fixed in place: the button that misbehaves stops being a saved query at all. See `specs/2026-08-15-builtin-filters-design.md` |
-| 91 | Double-clicking a thread should open it on its own | workflow | S | open, fully specified 2026-08-15 and ready to build. The view is always the whole thread, EXPANDED; the pane shows whichever row was double-clicked, so a reply drills to its thread and not to itself. It is `recoverStaleThread()` triggered by a gesture |
+| 91 | Double-clicking a thread should open it on its own | workflow | S | **done** 2026-08-15, unreleased. The view is always the whole thread, EXPANDED; the pane shows whichever row was double-clicked, so a reply drills to its thread and not to itself. Reuses `recoverStaleThread()` outright |
| 92 | Nothing distinguishes a tag written by a rule from one the user applied | information | M | **postponed** 2026-08-15 at the user's request: "I don't see the utility, so I don't really know how to answer." Needs per-MESSAGE provenance nothing records, a two-repo format change blank on all existing mail. Reopen only if the need appears in use |
| 93 | The query buttons are whatever the user pinned, not a designed set of filters | workflow | M | **done** 2026-08-15, unreleased; see `specs/2026-08-15-builtin-filters-design.md`. Absorbs item 90. Four built-in filters composing with the account dropdown; the user's own queries unpinned, never deleted |
| 95 | A query in the overflow menu cannot be run | defect | XS | **done** 2026-08-15, unreleased. Pre-existing and not caused by 93: the entry's action owned a submenu, and Qt emits no `triggered` for those, so the connection had never fired. Surfaced because 93 moved every query into the menu |
@@ -493,72 +493,6 @@ bump either way: an ignored optional field is not a breaking change.
**Size: S.** Removing a field, two UI affordances and their tests.
-## 91. Double-clicking a thread should open it on its own
-
-**Observed (user, notes):** "double clicking a message/thread should open it in
-its own view (?) UX not sure."
-
-**Specified by the user, 2026-08-15**, in three cases. The view is always the
-whole THREAD; what changes is which message the pane shows:
-
-- Double-click a single message: that message in a view by itself, loaded in
- the right pane.
-- Double-click a thread: the whole thread, expanded, in a view by itself, with
- the FIRST message in the right pane.
-- Double-click a reply inside a thread: the whole thread, expanded, in a view by
- itself, with THAT REPLY in the right pane.
-
-So a reply does not drill down to itself alone. It drills to its thread, with
-itself selected. That is the part an implementation is most likely to get wrong,
-because the obvious reading of "open it by itself" is an `id:` query on the
-reply, and it is not what was asked for.
-
-**It is NOT a second window.** The user ruled that out explicitly: "I didn't
-mean open in a new window." No second `QWebEngineView`, no extra Chromium render
-process, nothing reparented.
-
-**Cause (verified in code):** nothing is connected to `doubleClicked` on
-`ThreadListView`, so a double-click today is two single-clicks and selects the
-row twice.
-
-**Approach: this is `recoverStaleThread()` triggered by a gesture.** That
-function already does the entire job for the stale-thread notice: it sets
-`m_recoverThreadId` and `m_recoverMessageId`, runs a `thread:` query, expands
-the thread when the row arrives, selects the target reply once the replies land,
-and falls back to selecting the thread when the message has gone. Every case
-above maps onto it, with the recovery target set to the double-clicked row's own
-message. Reuse it rather than writing a second selection-after-query path.
-
-Two properties of that code are exactly what this item needs and are already
-commented in place: the root card IS the thread's first message and is not among
-the children, so a thread double-click is the empty-target case; and a reply
-cannot be selected until the replies exist, so the first pass selects the thread
-provisionally and refines on the next.
-
-**Expansion is not automatic and must be asked for.** Nothing in the tree
-auto-expands: `expandAll` and `setExpanded` appear nowhere in `mainwindow.cpp`,
-so a plain `thread:` query lands on ONE COLLAPSED CARD and the user would still
-have to click the expander. `recoverStaleThread()` calls `expand()` on the
-thread, which is also what asks the worker for the replies.
-
-**How the user gets back: nothing is built for it.** Answered by the user:
-"I didn't think of a back action, usually I'd go back to a known list like
-unread or inbox at that point." The filter buttons already are that and are one
-click away in every view. A Back action, a history stack and a
-restore-the-previous-query scheme were all drafted and are all unnecessary.
-
-**The undo-stack cost is inherited and acceptable.** `runQuery()` clears the
-undo stack because its rows are about to be discarded, and a drill-down discards
-them exactly as a typed query does. The user already expects that from the query
-bar, so the gesture inherits consistent behaviour rather than a surprise. No
-filter-over-the-model alternative is needed, which is what would have made this
-item 40's machinery and M rather than S.
-
-**Constraint.** A double-click also delivers a single click first, which selects
-the row and arms the mark-read timer. The drill-down must not mark a message
-read that the user only passed through, and the existing single-click path is
-what arms it, so the double-click handler has to cancel it the way the
-multi-row branch of `onThreadSelected()` does.
## Deferred, unsized, or split out
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index ba803bc..fcf97df 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -709,6 +709,9 @@ void MainWindow::buildUi()
&QItemSelectionModel::selectionChanged,
this, &MainWindow::onSelectionChanged);
+ connect(m_threadView, &QAbstractItemView::doubleClicked,
+ this, &MainWindow::onRowDoubleClicked);
+
m_messageView = new MessageView(central);
m_messageView->setTagColors(&m_tagColors);
connect(m_messageView, &MessageView::statusMessage,
@@ -3104,6 +3107,57 @@ void MainWindow::recoverStaleThread(const QString &threadId,
m_recoverMessageId = messageId;
}
+void MainWindow::onRowDoubleClicked(const QModelIndex &index)
+{
+ if (!index.isValid())
+ return;
+
+ // The whole thread in every case, and the double-clicked row's own message
+ // in the pane. A reply therefore drills to its THREAD with itself selected,
+ // never to itself alone: "double click on a reply in a thread should still
+ // load the whole thread expanded in a view by itself, with the reply I
+ // clicked on visible in the right pane" (item 91). An id: query on the
+ // reply is the obvious reading of "open it by itself" and is the wrong one.
+ //
+ // Reached through the INDEX rather than through index.row(): a tree numbers
+ // rows per parent, so a reply's row indexes its siblings and threadAt() on
+ // one answers about an unrelated thread.
+ QString threadId;
+ QString messageId;
+ if (m_model->isMessageRow(index)) {
+ const MessageNode node = m_model->messageAt(index);
+ threadId = node.threadId;
+ messageId = node.messageId;
+ } else {
+ threadId = m_model->data(index, ThreadListModel::ThreadIdRole).toString();
+ // The thread's first message, so the pane opens on it rather than on
+ // nothing. Empty is fine and means the same thing to the recovery: land
+ // on the root, which IS that message.
+ messageId = m_model->data(index, ThreadListModel::MessageIdRole).toString();
+ }
+
+ if (threadId.isEmpty())
+ return;
+
+ // The first click of the double-click already selected this row and armed
+ // the mark-read timer. The user is passing through on their way into the
+ // thread, and a gesture that navigates must not mutate mail, so the timer
+ // goes the same way it does for a multi-row selection.
+ //
+ // Not a correction of the single click's behaviour: the thread is about to
+ // be opened and its message read, which arms the timer again for the row
+ // the recovery selects. What is cancelled is the arming for a row the user
+ // is leaving.
+ m_markReadTimer->stop();
+ m_markReadThreadId.clear();
+
+ // Reuses the stale-thread recovery outright, which already runs thread:<id>,
+ // expands the thread when the row arrives, selects the target message once
+ // the replies land, and falls back to the root when the message has gone.
+ // Every one of item 91's three cases is one of those paths.
+ recoverStaleThread(threadId, messageId);
+}
+
void MainWindow::applyPendingRecovery()
{
if (m_recoverThreadId.isEmpty())
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 2a86d12..614430f 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -384,6 +384,10 @@ private slots:
/// point the button uses.
void recoverStaleThread(const QString &threadId, const QString &messageId);
+ /// Drills into the double-clicked row: the whole thread, expanded, alone in
+ /// the view, with that row's own message in the pane.
+ void onRowDoubleClicked(const QModelIndex &index);
+
/// Selects the remembered message once its thread's rows have loaded.
void applyPendingRecovery();
void onThreadsReady(const QVector<ThreadSummary> &threads, quint64 generation);
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 253d3e6..f4357c6 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -233,6 +233,9 @@ private slots:
void recoveringAStaleThreadQueriesTheWholeThread();
void recoveryReselectsTheMessageThatWasBeingRead();
void recoveryOnTheFirstMessageSelectsTheThreadRow();
+ void doubleClickingAThreadOpensThatThreadAlone();
+ void doubleClickingAReplyOpensItsThreadNotTheReplyAlone();
+ void doubleClickingDoesNotLeaveTheMarkReadTimerArmed();
void aUserQueryAbandonsAPendingRecovery();
void blankingThePaneAlsoDropsTheStaleNotice();
void aNewQueryDropsTheStaleNotice();
@@ -3163,6 +3166,145 @@ void TestMainWindow::recoveryOnTheFirstMessageSelectsTheThreadRow()
QCOMPARE(model->threadAt(current.row()).threadId, QStringLiteral("T1"));
}
+void TestMainWindow::doubleClickingAThreadOpensThatThreadAlone()
+{
+ // Item 91, the thread case: "double click on a thread loads the whole
+ // thread expanded in a view by itself and load the first message in the
+ // right pane."
+ //
+ // The view becomes thread:<id>. Asserted on the query text because that is
+ // what makes it a view "by itself"; a gesture that only expanded the row in
+ // place would leave every other thread on screen and pass any assertion
+ // about the expansion alone.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ ThreadSummary first = makeThread(QStringLiteral("T1"), {});
+ first.totalCount = 3;
+ model->appendBatch({ first, makeThread(QStringLiteral("T2"), {}) });
+ QCOMPARE(model->rowCount(QModelIndex()), 2);
+
+ const QModelIndex thread = model->index(0, 0, QModelIndex());
+ QVERIFY(thread.isValid());
+
+ emit view->doubleClicked(thread);
+
+ QCOMPARE(queryEdit->text(), QStringLiteral("thread:T1"));
+
+ // The recovery target is what carries the expansion and the selection
+ // across the two round-trips this takes. Without it the query would run and
+ // land on a collapsed card with a blank pane.
+ QVERIFY2(window.hasPendingRecoveryForTesting(),
+ "the double-click ran a query but asked for nothing to be "
+ "expanded or selected in the result");
+}
+
+void TestMainWindow::doubleClickingAReplyOpensItsThreadNotTheReplyAlone()
+{
+ // Item 91, the case most likely to be built wrong: "double click on a reply
+ // in a thread should still load the whole thread expanded in a view by
+ // itself, with the reply I clicked on visible in the right pane."
+ //
+ // So NOT id:<reply>. The obvious reading of "open it by itself" is a query
+ // for that one message, and it is not what was asked for: the view is the
+ // thread, the pane is the reply.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ ThreadSummary summary = makeThread(QStringLiteral("T1"), {});
+ summary.totalCount = 3;
+ model->appendBatch({ summary });
+
+ const QModelIndex thread = model->index(0, 0, QModelIndex());
+ QVERIFY(thread.isValid());
+
+ // Replies exist only once the tree has loaded, which is what gives this
+ // test a child row to double-click at all.
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("T1");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("T1"), { root, reply });
+
+ const QModelIndex replyRow = model->index(0, 0, thread);
+ QVERIFY2(replyRow.isValid(), "the fixture built no reply row");
+ QVERIFY2(model->isMessageRow(replyRow), "that row is not a message row");
+
+ emit view->doubleClicked(replyRow);
+
+ // The THREAD, not the reply. A query of id:m1@example.org here would be the
+ // defect this test exists for.
+ QCOMPARE(queryEdit->text(), QStringLiteral("thread:T1"));
+ QVERIFY2(window.hasPendingRecoveryForTesting(),
+ "nothing was remembered to select the reply once it comes back");
+}
+
+void TestMainWindow::doubleClickingDoesNotLeaveTheMarkReadTimerArmed()
+{
+ // A double-click delivers a single click FIRST, which selects the row and
+ // arms the mark-read timer. The user is passing through on their way to
+ // opening the thread, so the message must not be marked read behind the
+ // drill-down: the timer that the first click armed has to be cancelled.
+ //
+ // This is the same reasoning the multi-row branch of onThreadSelected()
+ // uses, where a selection gesture must not mutate mail.
+ const Config config;
+ MainWindow window(config);
+
+ auto *queryEdit = window.findChild<QLineEdit *>();
+ QVERIFY(queryEdit);
+ queryEdit->setText(QStringLiteral("tag:unread"));
+ queryEdit->returnPressed();
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
+ QVERIFY(timer);
+
+ model->appendBatch({ makeThread(QStringLiteral("T1"),
+ { QStringLiteral("unread") }) });
+ const QModelIndex thread = model->index(0, 0, QModelIndex());
+ QVERIFY(thread.isValid());
+
+ // The single click a real double-click delivers first. Asserted, so this
+ // test cannot pass by the timer never having been armed at all.
+ selectThreadRow(view, 0);
+ QVERIFY2(timer->isActive(),
+ "the selection did not arm the timer, so this proves nothing");
+
+ emit view->doubleClicked(thread);
+
+ QVERIFY2(!timer->isActive(),
+ "the drill-down left a mark-read armed for a message the user "
+ "only passed through");
+}
+
void TestMainWindow::aUserQueryAbandonsAPendingRecovery()
{
// A recovery spans two round-trips, so the user can type a query in the