summaryrefslogtreecommitdiffstats
path: root/docs
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-09 12:22:11 +0200
committerDanilo M. <danix@danix.xyz>2026-08-09 12:22:11 +0200
commit14218dc6e6ba8d8648b9b574457214b298bcf75f (patch)
tree8a0440036234bc180e71da616612a79eb862140e /docs
parent0a79470292bc2baf6bec94117095c6cd21b4c849 (diff)
downloadqtmaildir-14218dc6e6ba8d8648b9b574457214b298bcf75f.tar.gz
qtmaildir-14218dc6e6ba8d8648b9b574457214b298bcf75f.zip
docs: close the card list's open questions, and record a navigation defect
Both items the spec left open are settled, and one of them turned out to be a defect rather than the addition it was filed as. Item 20 deferred "move between messages without returning to the list" as an addition on top. It is a repair: next_thread and prev_thread are selectRow(current.row() + 1), and a tree numbers rows per parent, so from the last reply of an expanded thread row+1 names a sibling that does not exist and the action silently does nothing. rowCount() with no argument counts top-level threads, compounding it. Recorded as item 60, since it exists on the branch whether or not the card list is built, and it is a fresh instance of the exact rule that branch's own commit message states. The user asked for arrow keys to skip within a thread and Shift+arrows to skip between threads, scoped so the keys still work normally in the message pane, menus and entry bars. Two findings changed the shape of that: - Up/Down need no binding and no code. QTreeView's built-in navigation walks VISIBLE rows, so it already steps into an expanded thread's replies, and being the view's own key handling rather than a shortcut it is inert whenever focus is elsewhere. The requested focus behaviour is automatic. - Arrow keys must never become keymap actions. Every action is a QAction with WindowShortcut, dispatched before the focused widget sees the key, and Qt withholds only plain LETTERS from editable widgets. Return already proved this by breaking the query bar and needing a per-widget ShortcutOverride filter scoped to one widget and one key. Up as a window shortcut would break the query bar, the tag dialog and the web view at once. Shift+Up/Down was rejected for thread-jumping: it is the built-in extend-selection that multi-row tagging and item 20's action scope depend on. Alt+Up/Down instead, verified free across the whole keymap. The way back to a whole thread after clicking a reply is the root card, which is always visible above its replies. Escape is deliberately not overloaded, since it already means clear-selection with clear-pane on Shift+Escape, and a third meaning is the half-an-action problem item 50 exists to fix. The remaining open item is only whether the message pane should change to match, which stays out of scope: this spec is the left pane.
Diffstat (limited to 'docs')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md51
-rw-r--r--docs/superpowers/specs/2026-08-09-card-list-design.md78
2 files changed, 126 insertions, 3 deletions
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 c799e23..17cf434 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
@@ -117,6 +117,7 @@ taking that too literally.
| 57 | "Flag" would read better as "Important" or "Starred" | presentation | XS | **done** |
| 58 | `message_zoom` documents a 0.5 to 3.0 range and enforces none of it | correctness | XS | **done** |
| 59 | Archive and Mark all read shipped with the same icon | presentation | XS | **done** |
+| 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | open; branch only, fix as part of 53 |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -1201,6 +1202,13 @@ screenshots alone did not settle:**
within a thread without returning to the list. An addition on top, not part of
this item.
+**No longer deferred, 2026-08-09.** Folded into the card-list spec, because it
+turned out to be a defect repair rather than an addition: `next_thread` is
+`selectRow(current.row() + 1)`, and in a tree that names a sibling, so from the
+last reply of an expanded thread the action does nothing at all. Up/Down get the
+behaviour for free from `QTreeView`'s own navigation, and Alt+Up/Down keep the
+thread-to-thread jump.
+
**What exists today**, as the starting point: a thread is
one HTML document in one web view, messages stacked in chronological order,
each with a small grey `.msg-header` carrying From and Date
@@ -3643,6 +3651,49 @@ distinguishable are separate properties, and only the first was tested. The
candidate replacement was therefore checked by rendering both icons at 24px and
comparing the images, not by asking whether the name existed.
+## 60. Next thread dead-ends on the last reply of an expanded thread
+
+**Observed (found while specifying 53, 2026-08-09), not user-reported.** On the
+`item-20-message-rows` branch, with a thread expanded and the last reply
+selected, `next_thread` (Ctrl+J) does nothing. It should move to the next
+thread.
+
+**Cause (verified in code).** `mainwindow.cpp:644-655` implements both actions
+as arithmetic on a row NUMBER:
+
+```
+const int row = current.isValid() ? current.row() + 1 : 0;
+if (row < m_model->rowCount())
+ m_threadView->selectRow(row);
+```
+
+A `QTableView` numbers rows once for the whole view, so this was correct before
+item 20. A tree numbers them **per parent**: the last reply of a thread is row
+N of that thread, `row + 1` names a sibling that does not exist, and
+`m_model->rowCount()` with no argument counts top-level threads rather than the
+current parent's children. `prev_thread` fails the mirror case, moving from the
+first reply to nowhere instead of to the thread root.
+
+This is a fresh instance of the rule the branch's own commit message states:
+**nothing may be keyed on a row NUMBER**, because a tree numbers rows per
+parent. That commit lists it for the tag strip's paint walk. Nobody checked the
+navigation actions against the same rule.
+
+**Approach.** Walk with `QTreeView::indexBelow()` / `indexAbove()` from the
+current index, which follow visible rows across parent boundaries. For
+thread-to-thread jumping, skip any index whose `IsMessageRole` is true.
+
+**Specified as part of the card-list spec**
+(`docs/superpowers/specs/2026-08-09-card-list-design.md`), which also adds
+Alt+Up/Down for these actions and relies on `QTreeView`'s built-in Up/Down for
+stepping through replies. Fix it there rather than separately, unless the card
+list is dropped.
+
+**Constraint.** The test that would catch this must start from the **last reply
+of an expanded thread**. A test that arrows down a collapsed list passes against
+the bug, since with nothing expanded every row is top-level and the arithmetic
+is accidentally correct.
+
## Deferred, unsized, or split out
Items noted while triaging but not part of the original list. Same numbering
diff --git a/docs/superpowers/specs/2026-08-09-card-list-design.md b/docs/superpowers/specs/2026-08-09-card-list-design.md
index 3c717eb..f231577 100644
--- a/docs/superpowers/specs/2026-08-09-card-list-design.md
+++ b/docs/superpowers/specs/2026-08-09-card-list-design.md
@@ -215,15 +215,87 @@ whole session was lost to probes that lied:
- **Item 51 gets a regression test**: with cards, the view reports no horizontal
scroll range, and clicking a card does not change `horizontalScrollBar()`'s
value.
+- **`next_thread` gets the test that would have caught its current defect**:
+ from the LAST reply of an expanded thread it lands on the next thread, not on
+ nothing. The old `selectRow(row + 1)` fails this; a row-0-of-a-collapsed-list
+ test passes against the bug and is worthless.
+- **Alt+Up/Down skip replies**: from a thread root with its replies expanded,
+ one Alt+Down lands on the next thread root rather than on the first reply.
+- `everyActionHasAShortcut` must still pass once `setShortcut` becomes
+ `setShortcuts`, since it is the invariant that every action carries a default.
+
+Arrow-key navigation is `QTreeView`'s own and is not re-tested here, but the
+claim that it steps into replies should be confirmed by hand once before the
+spec is trusted on it. `QTest::keyClick` is weak evidence about key reachability
+per `CLAUDE.md`, and this design leans on the built-in behaviour rather than
+implementing it.
Two constraints on writing these, from `CLAUDE.md`: nothing may be keyed on a
row **number**, because a tree numbers rows per parent; and the offscreen
platform chooses the window width itself and has been seen to choose
differently between runs, so a test must not depend on a particular width.
+## Keyboard navigation
+
+Item 20 deferred "moving between messages in a thread without returning to the
+list" as an addition on top. It is folded in here instead, because the card list
+makes it a **defect repair** rather than a feature: `next_thread` and
+`prev_thread` are implemented as `selectRow(current.row() + 1)`
+(`mainwindow.cpp:644-655`), and a tree numbers rows per parent, so on the last
+reply of an expanded thread `row + 1` names a sibling that does not exist and
+the action silently does nothing. This is the same trap the branch's own commit
+message records: nothing may be keyed on a row NUMBER.
+
+**Up / Down step through everything, replies included.** This needs no code and
+no binding at all. `QTreeView`'s built-in navigation walks *visible* rows, so it
+already steps into an expanded thread's replies and past its end into the next
+thread. It is the view's own key handling rather than a shortcut, so it is
+inert whenever focus is elsewhere: arrows scroll the message pane when the web
+view has focus, move the cursor in the query bar, and move through menus, with
+nothing to configure.
+
+**Alt+Up / Alt+Down jump thread to thread**, skipping replies even when a thread
+is expanded. Bound to the existing `prev_thread` / `next_thread` actions
+alongside their current Ctrl+K / Ctrl+J, which keep working. `MainWindow`'s
+`addAction` calls `setShortcut()` singular at `mainwindow.cpp:622` and must move
+to `setShortcuts()` with a list; `zoom_reset` at `mainwindow.cpp:773` is the
+existing precedent. Alt carries no binding anywhere in the keymap today, so
+nothing is displaced.
+
+Both actions are rewritten to walk with `QTreeView::indexBelow()` and
+`indexAbove()` from the current index, skipping any index whose `IsMessageRole`
+is true, rather than arithmetic on a row number.
+
+**Shift+Up / Shift+Down are left alone.** They are `QTreeView`'s built-in
+extend-selection, which multi-row tagging and item 20's action scope both depend
+on, and which every mail client and file manager binds the same way. They were
+considered for thread-jumping and rejected on that ground.
+
+**Arrow keys must never become keymap actions.** Every action is a `QAction`
+with `Qt::WindowShortcut` (`mainwindow.cpp:626`), and a shortcut is dispatched
+BEFORE the focused widget sees the key. Qt withholds a plain LETTER shortcut
+from an editable widget, which is why the existing letter bindings are safe, but
+arrows get no such protection, exactly like Return: binding Up as a window
+shortcut would break the arrow keys in the query bar, the tag dialog and the web
+view at once. The Return case needed a per-widget `ShortcutOverride` filter to
+claw the key back (`mainwindow.cpp:261-275`), scoped to one widget and one key
+precisely because the general case is unmanageable. Alt+Up is safe only because
+the modifier makes it a chord no text field wants.
+
+## Returning to the whole thread
+
+Clicking a reply card opens that one message in the pane; clicking a thread root
+card opens the whole thread. **The way back is the root card**, which is always
+visible directly above its replies whenever they are showing.
+
+No new affordance and no key. Escape is deliberately not overloaded for this: it
+already means clear-selection, with clear-pane on Shift+Escape (item 50), and a
+third meaning stacked on the same key would be the "half an action" problem that
+item 50 exists to fix.
+
## Open, deliberately not decided here
-- **Moving between messages in a thread without returning to the list.** Named
- by the user during item 20 and deferred there. Still deferred.
- Whether the message pane's own presentation should change to match. Out of
- scope: this spec is the left pane only.
+ scope: this spec is the left pane only, and the single-message rendering is
+ unchanged from the branch. The `<details>`-per-message design was offered on
+ 2026-08-08 and declined; it stays declined.