diff options
| -rw-r--r-- | CHANGELOG.md | 40 | ||||
| -rw-r--r-- | CLAUDE.md | 120 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 30 |
3 files changed, 130 insertions, 60 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 1444c3a..5bf9256 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,29 @@ point at which they are stable. ## [Unreleased] +### Changed + +- **The thread list is now a list of cards rather than a table of columns.** + Each thread shows its sender and date, its subject with the flag, attachment + and reply-count marks, and its tags, on three lines at one uniform height. + Expanding a thread shows its replies indented under a continuous spine, + carrying only the tags the thread itself does not have, and without the `Re:` + prefix every reply used to repeat. +- **Threads can be listed newest or oldest first**, from a new control beside + the query bar. The choice is remembered between sessions. +- **An account's colour now runs down the left edge of its threads**, and down + the spine of their replies, replacing the account chip that used to sit in + front of every subject. The account dropdown shows the same colours, so which + colour means which account is readable in one place. +- **Alt+Up and Alt+Down step between threads**, alongside the existing Ctrl+J + and Ctrl+K. Plain Up and Down now step message by message through an expanded + thread, which is the view's own behaviour rather than a binding. +- **An out-of-range `message_zoom` now says so.** The documented 0.5 to 3.0 + range was already enforced on the way to the web view, so a `message_zoom` of + 500 rendered at 3.0 rather than unusably, but nothing reported that the value + in the file was not the value on screen. It is now listed with the other + configuration problems at startup. + ### Fixed - **The message pane no longer comes back as a sliver.** A splitter position is @@ -19,13 +42,18 @@ point at which they are stable. left, in one real case 29px. The pane now has a minimum width and cannot be collapsed, which covers the restore and the equivalent drag. -### Changed +- **Clicking a thread no longer scrolls the list sideways.** A card is exactly + the width of the pane, so there is nowhere to scroll to. +- **Next and previous thread no longer step onto a reply** when a thread is + expanded. They skip message rows, so they keep meaning thread-to-thread. -- **An out-of-range `message_zoom` now says so.** The documented 0.5 to 3.0 - range was already enforced on the way to the web view, so a `message_zoom` of - 500 rendered at 3.0 rather than unusably, but nothing reported that the value - in the file was not the value on screen. It is now listed with the other - configuration problems at startup. +### Upgrading + +- **Saved thread-list column widths are ignored.** There is one column now, so + the `threadlist/header` and `threadlist/columns` entries in + `~/.local/state/qtmaildir/uistate.conf` no longer do anything. Nothing needs + to be done: they are read past and can be left in place or deleted. Window + geometry, the splitter position and the message zoom are unaffected. ## [0.12.1] - 2026-08-09 @@ -43,10 +43,10 @@ MainWindow NotmuchWorker ├ query row: QComboBox, QLineEdit, └ owns the only notmuch_database_t* │ saved-query QPushButtons ├ ThreadListView (QTreeView) ── ThreadListModel (QAbstractItemModel) - │ thread rows, expanding to message rows; RowStyleDelegate every - │ column, SubjectDelegate on Subject (chip, subject, expander) + │ ONE column of cards; CardDelegate paints each whole, from CardLayout └ MessageView (header QLabel, QWebEngineView, attachment bar, TagStrip) +CardLayout (pure geometry, no painting) Config (INI) KeyMap MailSync (QProcess) MimeParser (GMime) SyncMonitor (/proc/locks) TagColors QueryCompleter ThreadCidMap ``` @@ -56,62 +56,92 @@ The query row and the message-pane header are **built inline in `MainWindow` and listed `QueryBar`, `SavedQueryBar`, `HeaderWidget` and `AttachmentBar`; none of those types have ever existed, and looking for them wastes a search. The widget classes that do exist are `MessageView`, `ThreadListView`, `TagStrip`, -`TagDialog`, `RowStyleDelegate` and `SubjectDelegate`; `TagChip` is a namespace -of painting helpers, not a widget, and `ThreadCidMap` is a struct. - -**`ThreadListView` exists because a delegate cannot paint outside its column.** -The tag chips under each row are one strip spanning the whole width, so they -are drawn in the view's `paintEvent` after the cells. Consequences that are -easy to undo by accident: `SubjectDelegate` reads `AccountLabelRole`, which -belongs to the ROW, so installing it view-wide draws the account chip into -every column (a `Q_ASSERT` catches this); and because alternating colours, the -selection and the model's `BackgroundRole` are all painted per cell, the view -has to fill the strip's band itself, honouring all three or a deleted row is -cut in half and every other row shows a bare stripe. +`TagDialog`, `RowStyleDelegate` and `CardDelegate`; `TagChip` is a namespace of +painting helpers, not a widget, and `ThreadCidMap` and `CardLayout` are structs. +`SubjectDelegate` existed until item 53 and is gone. + +**`ThreadListView` survives only for the expander hit-test.** `CardDelegate` +draws the reply count, and a delegate gets no click of its own without an +editor, so the view owns the click and asks the delegate for the rect rather +than recomputing it. + +Until item 53 it also painted a row-wide strip of tag chips after the cells, +because a delegate cannot paint outside its column and the strip spanned all +five. That is why the class exists at all, and the history is worth keeping: +the arithmetic it needed produced a deleted row cut in half and every other row +showing a bare stripe, both because the view had to re-honour alternating +colours, the selection and `BackgroundRole` across cells it did not own. With +one column there is nothing to span, so the `paintEvent` and its band +arithmetic are deleted and none of that applies any more. + +**A card layout must be testable without a painter.** `CardLayout` computes +every rect on a card and touches no `QPainter` and no widget, so the geometry +has tests that a blank render cannot defeat. When changing what a card shows, +change `CardLayout` and assert there; a test that renders the delegate and +counts pixels proves nothing, for the reasons under "Rendering probes lie". +Two traps it already handles: `QRect::right()` is inclusive, so the right edge +is carried as an exclusive one, and `QFont::pointSizeF()` returns -1 for a font +set in pixels, which qt6ct does. + +**`QTreeView`'s Up/Down already walk into an expanded thread's replies**, and +that is where message-to-message navigation comes from. Do not bind arrow keys +as `QAction` shortcuts to get it: a shortcut is dispatched before the focused +widget sees the key and Qt withholds only plain LETTERS from editable widgets, +so a bare `Up` would break the query bar, the tag dialog and the web view at +once. `Alt+Up`/`Alt+Down` are chords and therefore safe; `Shift+Up`/`Down` is +the built-in extend-selection and must be left alone. Binding two sequences to +one action needs `setShortcuts`, not `setShortcut`, which keeps only the last. + +**`Q_ENUM` is not enough to send an enum across a queued connection.** It gives +the type a meta-object entry, not a metatype registered under the name +`invokeMethod` resolves, so a `Q_ARG` carrying it is dropped at runtime with a +warning and the slot runs with a default. `NotmuchWorker::SortOrder` is +registered beside the type for this reason, not in `MainWindow`, so a caller +that never constructs one still gets it. **It is a `QTreeView` over a `QAbstractItemModel` since item 20**, because a thread's replies are child rows and a table can neither indent nor expand. What did NOT survive that port is anything keyed on a row NUMBER: a tree numbers rows -per parent, so row 0 exists once per expanded thread and a flat `0..N` walk -paints the first thread's strip over every one of them. The strip walk goes by -index, alternating colour follows visual position rather than `index.row()`, and -`QTableView::isRowSelected(int)` has no equivalent — use -`selectionModel()->isSelected(index)`. Row height comes from -`setUniformRowHeights` plus the delegate's `sizeHint`, since a tree has no -vertical header to carry a default section size. - -**Four traps in the expander, all of which shipped a plausible-looking broken +per parent, so `row 0` exists once per expanded thread and `current.row() + 1` +names a sibling rather than the next thread. Navigation walks with +`indexBelow`/`indexAbove`; `QTableView::isRowSelected(int)` has no equivalent — +use `selectionModel()->isSelected(index)`. Row height comes from +`setUniformRowHeights` plus `CardDelegate::sizeHint`, since a tree has no +vertical header to carry a default section size. Indentation is +`setIndentation(0)`: `CardLayout` draws the indent inside the card's own rect, +so `visualRect` reports the SAME left edge for a thread and its reply and a +geometry probe sees no nesting in a correctly nested list. + +**Three traps in the expander, all of which shipped a plausible-looking broken build before being caught.** `QTreeView::drawBranches` is the documented hook and does not work when the expander sits on a content column: it runs BEFORE the row's cells, so the delegate's background paints over it (a 60-pixel triangle -survived as 8). `SubjectDelegate` draws it instead, from BOTH of its branches — -calling it only from the no-chip branch leaves every real row without one, since -every real row has an account chip. `setRootIsDecorated(false)`, needed to stop -the style drawing its own indicator underneath, also removes the style's HIT -AREA, so the glyph renders perfectly and is inert; `ThreadListView::mousePressEvent` -handles the click. And `isExpanded`/`setExpanded` are keyed on **column 0**, so -asking them about the subject-column index always answers false and every click -expands again instead of toggling. +survived as 8). `CardDelegate` draws it instead, as the reply count on the +card's second line. `setRootIsDecorated(false)`, needed to stop the style +drawing its own indicator underneath, also removes the style's HIT AREA, so the +glyph renders perfectly and is inert; `ThreadListView::mousePressEvent` handles +the click, asking `CardDelegate::expanderRectFor` for the target so the drawn +and clickable rects cannot drift. A fourth trap died with the grid: `isExpanded` +is keyed on column 0, which used to disagree with the subject-column index. **Visible, clickable and toggling are three separate properties.** A test for one passes against the other two being broken, which happened twice in one session: a pixel test proved the triangle was drawn while nothing could click it, and a click test proved it opened while it could never close. -**A reply row's indent must beat the account chip's width.** A thread row draws -a chip before its subject and a reply row does not, so a reply's text starts -roughly a chip-width to the LEFT of its thread's before any indent applies. -Qt's 20px default is swallowed entirely by that difference and the replies read -as flush or outdented. `SubjectDelegate::kReplyIndent` is 72px for this reason. -Note that `visualRect` reports the indent correctly the whole time, so a -geometry probe endorses a layout with no visible nesting: assert on where the -TEXT lands. - -**`paintEvent` runs AFTER the cells.** Anything it fills across a row covers the -text the delegate just drew: the reply tint filled the full row height in its -first version and erased every sender and subject, measured at zero surviving -text pixels. The fill and the thread-line stub stay in the band below the text, -where the tag strip lives on thread rows. +**Assert a reply's indent on where the TEXT lands, never on `visualRect`.** The +reason has inverted twice and the rule has not. Under item 20 the geometry was +indented while the text was not, because the delegate laid text out from its own +left edge; now `setIndentation(0)` means `visualRect` reports no indent at all +while the text is indented, because `CardLayout` draws it inside the card's rect. +A probe on `visualRect` therefore endorsed a broken layout then and would fail a +correct one now. Assert on `CardLayout::contentLeft`. + +**`paintEvent` ran AFTER the cells**, which is why anything the view filled +across a row covered the text the delegate had just drawn: the reply tint filled +the full row height in one version and erased every sender and subject, measured +at zero surviving text pixels. Recorded because it is the class of bug a view +that paints invites. `ThreadListView` no longer paints at all. **No `notmuch_*` pointer ever crosses the thread boundary.** Data crosses as the plain value structs in `src/types.h` (`ThreadSummary`, `MessageRef`, `MessageNode`, 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 17cf434..ad6dbc0 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 @@ -77,7 +77,7 @@ taking that too literally. | 17 | No completion for tags in the query bar | workflow | M | **done** | | 18 | No visual cue that there are unsynced edits | feedback | S | **done** | | 19 | No prompt to sync on exit when edits are pending | behavior | S | **done** | -| 20 | Thread view does not match the user's mental model | presentation | L | built on branch `item-20-message-rows`, **not merged**; see 53 | +| 20 | Thread view does not match the user's mental model | presentation | L | rebuilt as cards on branch `card-list`, **not merged**; awaiting the user's verdict | | 21 | Default shortcuts are not sensible enough | discoverability | S | open | | 22 | Translatability audit and i18n wiring | correctness | M | open | | 23 | No way to save a search query from the UI | workflow | M | open | @@ -108,16 +108,16 @@ taking that too literally. | 48 | Removing a tag suggests every tag, not the thread's own | workflow | XS | **done** | | 49 | Sync runs every account regardless of what changed | workflow | M | **done** | | 50 | Esc blanks the pane but leaves the row selected | workflow | XS | **done** | -| 51 | Clicking a subject scrolls the list sideways | presentation | XS | open; resolved as a side effect of 53, do not work separately | +| 51 | Clicking a subject scrolls the list sideways | presentation | XS | built on branch `card-list`, **not merged**; a card is viewport width | | 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | **done** | -| 53 | Message rows still read as a table, not as a conversation | presentation | M | open, specified 2026-08-09; see the card-list spec | +| 53 | Message rows still read as a table, not as a conversation | presentation | M | built 2026-08-10 on branch `card-list`, **not merged**; awaiting the user's verdict | | 54 | A cron sync carries the edits but the count still says pending | correctness | S | **done** | | 55 | In a narrow window the message pane is invisible | presentation | XS | **done** | | 56 | No action carries an icon, so the toolbar reserves space for nothing | presentation | S | **done** | | 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 | +| 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | **done** on the branch; already fixed by 5487d58, see below | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -3683,11 +3683,23 @@ navigation actions against the same rule. 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. +**The cause above is wrong, and was corrected on 2026-08-10.** It was read off +`master`, where the arithmetic really is `current.row() + 1`. The branch does +not do that: `5487d58` added `MainWindow::threadRowOf()`, which walks up to the +containing thread BEFORE the arithmetic, so from the last reply of an expanded +thread `next_thread` already reached the next thread. The defect was fixed in +the same commit that could have introduced it, one commit before this entry was +written. Verified by writing both failing tests first, on the branch, and +watching them pass against unchanged code. + +The entry is kept rather than deleted, because the reasoning it records is +sound and the tests it demanded now exist. It is a reminder that a cause +"verified in code" is only verified against the branch it was read on. + +**Superseded by the card-list work all the same.** Both actions now walk with +`indexBelow`/`indexAbove` (`card-list`, 2026-08-10), so nothing in that path is +keyed on a row number, which is the rule a deeper tree would break next. +Alt+Up/Down were added alongside Ctrl+J/K there. **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 |