From 6d35f7ec87590fd484ca7640d4d06c06a6d97970 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 15 Aug 2026 16:35:56 +0200 Subject: 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:, 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 --- .../2026-08-03-post-0.1.0-usability-closed.md | 100 +++++++++++++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 68 +------------- 2 files changed, 101 insertions(+), 67 deletions(-) (limited to 'docs/superpowers/plans') 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:`, 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:`, 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 -- cgit v1.2.3