diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 194 |
1 files changed, 127 insertions, 67 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 437ceda..5e95804 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 @@ -131,12 +131,12 @@ taking that too literally. | 62 | No config option for the date format on a card | presentation | XS | **done** 2026-08-11 | | 63 | No way to see sent mail, and no filter for it | workflow | M | **done** 2026-08-11; see `specs/2026-08-11-sent-mail-design.md` | | 64 | The Sync button carries a mailbox icon, not a refresh one | presentation | XS | **done** 2026-08-11 | -| 65 | No full code review and optimization pass | correctness | ? | open, unspecified | +| 65 | No full code review and optimization pass | correctness | ? | open, **narrowed 2026-08-26**: the notes now name it as a dead-code and duplication sweep, not a performance or security pass. Produces a LIST for the user to decide on, not a diff. See the entry | | 66 | Selecting a thread root leaves the message pane blank until a reply has been selected | defect | S | **done** 2026-08-14, unreleased. Not the blank pane it was filed as: the root rendered the CONVERSATION until the thread had been expanded once, then one message. Now always one message, and the conversation view is removed at the user's request. **One case unverified by hand:** the notes also report a single-message `id:` query whose card would not open, which is the same empty-`MessageIdRole` failure and should be gone; confirmed 2026-08-15 as a SEPARATE defect with a different cause, see item 96 | | 87 | Auto mark-read marks a whole thread, including replies never displayed | defect | S | **done** 2026-08-16, unreleased. Built on 108, which is why it stayed small: the timer tracks a MESSAGE id now, and arms for a reply too, which it never did before | | 88 | `threadAt(current.row())` answers about the wrong thread for a reply row | defect | M | **done** 2026-08-16, unreleased. The audit found FOUR live sites, not one. `ThreadListModel::threadFor(index)` resolves a reply through its parent; every caller holding a selected index converted, and no `.row()` on a selected index remains in `mainwindow.cpp`. Unblocks 87 | | 67 | The placeholder pane counts unread, flagged and inbox, but not sent or drafts | information | XS | **done** 2026-08-11, shipped in 0.15.0 | -| 68 | A forwarded subject gets no `passed` tag | workflow | S | open; no subject rule exists, measured 2026-08-11. Decision needed: display mark (XS) or write the flag (S, syncs out) | +| 68 | A forwarded subject gets no `passed` tag | workflow | S | **done 2026-08-26**, unreleased, as THREE things once the premise was measured away. The note asked to expand a subject rule to `Fw:`; there was no subject rule, and the correlation it rested on did not exist. What did exist was a gap nobody had reported: qtmaildir has never written `R` or `P`, so a reply and a forward now flag their source (off the undo stack, per the auto-mark-read precedent), and `subjectIsForwarded()` drives a SEPARATE received-forward mark, display only, extendable through `[general] forward_prefixes`. The user chose all three | | 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | **done** 2026-08-11, inside item 70 | | 70 | Pane icons are a private set where the main window uses the system theme | presentation | M | **done** 2026-08-11; six shipped SVGs | | 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | **done** 2026-08-11; 2s default, `auto_sync_delay_ms` | @@ -242,6 +242,8 @@ taking that too literally. | 166 | Mail you send to your own other account loses `inbox` | defect | S | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected | | 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use | | 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together | +| 169 | A card shows the account only as a bar, with no fade and no avatar | presentation | M | open, 2026-08-26, from the notes. The accent bar exists (`CardLayout::accentRect`, `CardDelegate::accentLineColour()`); the gradient fade and the sender avatar do not. The avatar's initials source is decided, the vCard half is blocked on item 72 | +| 170 | A row that stops matching the view only leaves it on the Delete path | defect | S | open, 2026-08-26, from the notes, **cause found the same day and the premise is NOT stale**. The optimistic REPAINT is universal; the optimistic MEMBERSHIP is not. `removeThreadsWithoutTag()` has exactly one caller, on the move path, so marking a message read in the Unread view repaints the row and leaves it in a list it no longer belongs to | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -365,71 +367,36 @@ batches of 200 so a 10k-thread query paints immediately) already holds. An optimization pass with no measurement behind it is the kind of work that produces a large diff and no change a user can notice. -**What it needs before it can be sized.** The user saying which of these they -meant: a correctness/security review of a named area, a specific operation that -feels slow with the query that makes it slow, a dead-code and duplication sweep, -or the translatability audit that is already item 22. The first three are -different pieces of work with different sizes, and the fourth is already -recorded. - -**Size: `?`, unspecified.** Do not propose a design for this; ask. - -## 68. A forwarded subject gets no `passed` tag - -**Observed (user, from the notes):** "passed tag should appear when subject is -`Fwd:` and `Fw:`." Refined in session on 2026-08-11: the user had noticed -`passed` appearing on messages whose subject carried `Fwd:` and not on `Fw:`, -and asked to expand the rule to both. - -**Cause:** there is no rule to expand. `passed` is the Maildir `P` flag in the -message filename, translated into a tag by notmuch because -`maildir.synchronize_flags=true`. The flag is written by whichever client -forwarded the message, or by the server over IMAP; nothing reads a subject line -anywhere in the chain. qtmaildir only ever colours the tag -(`src/tagcolors.cpp:36-37`) and the database's `post-new` hook does not mention -it either. - -**Measured against the real database (2026-08-11):** - -| Query | Count | -|---|---| -| `tag:passed` | 6 | -| `tag:passed and subject:"Fwd:"` | 1 | -| `tag:passed and subject:"Fw:"` | 0 | -| `subject:"Fwd:" and not tag:passed` | 194 | -| `subject:"Fw:" and not tag:passed` | 28 | - -Six tagged messages in the whole database, and every one of them carries `P` in -its filename flags. The single overlap with `Fwd:` is a message that was -forwarded and whose subject was already a forward, not evidence of a rule: 194 -`Fwd:` subjects carry no tag at all. The correlation the observation rests on -does not exist. - -**Approach and the decision it needs first.** Two different features, and the -measurements above decide how far apart they are. - -*Display only.* The card shows a forwarded mark when the subject matches. Touches -no mail, changes no flag, reversible by deleting the rule. XS. - -*Write the tag.* qtmaildir sets `P` from a subject heuristic. With -`maildir.synchronize_flags=true` that flag is a filename change that mbsync -carries out to the server, on 222 existing messages, on a guess about a string. -Not cleanly undoable, and it asserts a meaning for a flag this application did -not define. Recommended against; recorded so the choice is deliberate rather than -forgotten. - -**Constraints:** localised clients use their own prefixes, and `Fwd:` can appear -inside a subject rather than at its head, so whatever matches must be anchored. -If the tag is ever written, it must not be re-applied on every sync in a way that -produces pending edits the user never made, item 28 is the record of a count -going wrong. The display-only route avoids that entirely, since it derives the -mark at paint time and stores nothing. - -**Size: S** as written, XS if it is display only. Most of it is the decision, not -the code. - -**Status:** left open deliberately on 2026-08-11. The cause is settled and the -options are costed; the user has not chosen, and no code was written. +**Narrowed by the user, 2026-08-26.** The notes now name two sub-bullets, and +they are the same piece of work rather than two: "deduplication of +functionalities" and "check for dead code (functionalities superseded by other +additions, rendering them useless now)". So this is a dead-code and duplication +sweep, NOT a performance pass and not a security review. Nothing slow has been +reported, and the translatability audit it might have meant is item 22, already +done. + +**What that makes it.** A read of the whole tree looking for a function with a +newer twin and for a path nothing reaches any more. The codebase has precedent +for both: `threadAt(int)` survives beside `threadFor(index)` for one legitimate +caller, `SubjectDelegate` was deleted outright at item 53, and item 132 deleted +a whole test rule that had stopped serving. The output is a LIST first, one +entry per candidate with the evidence that it is dead or duplicated, not a +diff; the user decides what goes. + +**Constraints.** + +- "Unreachable from the UI" is not the same as dead. Item 16's + double-press-to-undelete branch reads as dead and is not, because stranded + mail reaches it. Every candidate needs the reachability argument written out + before it is cut. +- A test is a caller. Deleting production code with only test callers is + usually right; deleting the test with it needs saying so explicitly. +- The sweep is worth nothing if it is not run against a green suite before and + after, since the whole value is that nothing observable changed. + +**Size: still `?` until the list exists.** The sweep that produces the list is +S to M; what it finds is the work. + ## 72. No khard/khal integration @@ -1347,3 +1314,96 @@ id and a draft of a reply carries both. - Item 163's fix stands on its own and this does not block it: the file is replaced correctly now, so the fork this would have mitigated no longer happens by that route. + +## 169. A card shows the account only as a bar, with no fade and no avatar + +**Observed (user, from the notes):** "the left border of a card expresses the +account the mail belongs to. the background color of the card should fade left +to right from the account color to the current background color we are using (or +to transparent to work both in light and dark themes). On the left we should +leave room for an account avatar (a squircle), for now it could be extracted +from the sender name "From: john doe" becomes "JD" in the avatar. As soon as we +include khard (or some other vcard provider/manager) we will switch to images if +the corresponding vCard has one." + +**Cause (verified in the code):** not a defect. Half of it shipped. The account +colour is drawn as a solid bar down the left edge, `CardLayout::accentRect` +placed by `CardLayout`, filled by `CardDelegate::paint()` with +`CardDelegate::accentLineColour()`. There is no gradient anywhere on a card, and +nothing draws an avatar: `CardLayout` reserves no rect for one, so the geometry +would have to grow before the painting could. + +**Approach.** Two separable pieces, and the avatar is the one that changes the +layout. + +- The fade is a `QLinearGradient` fill over the card rect, from the accent + colour to the pane's background. `accentLineColour()` already records why + blending toward the background is wrong for a CHIP; a card's background is + exactly where such a blend belongs, so the constraint does not carry over. + Both themes come free if the far stop is the palette's own base rather than + a literal. +- The avatar needs a rect in `CardLayout`, which is where it becomes testable + without a painter, and it shifts `contentLeft` for every card. The initials + come from the display name already carried on the summary; a sender with no + display name (an address only) needs an answer before this is built. + +**Constraints.** + +- The vCard half is blocked on item 72, which is itself unspecified. Build the + initials only; do not design the image path in advance. +- A gradient behind the text has to keep the text readable at the left edge in + both themes, which is the same failure mode `accentLineColour()` guards + against on a dark palette. +- This is a looks question, so it is settled by the user looking at it rather + than by a test: assert the geometry in `CardLayout`, and hand the appearance + over per `tests-only-for-measurable-things`. + +## 170. A row that stops matching the view only leaves it on the Delete path + +**Observed (user, from the notes):** "should we refactor the list UI to be +responsive so changes are applied immediately instead of waiting for a view +change to repaint?" + +**Cause (verified in the code, 2026-08-26).** Two different properties were +being called "responsive", and only one of them was built. + +The optimistic **repaint** is universal. `ThreadListModel::applyTagChange()` +covers a thread-scoped write, `applyMessageTagChange()` a message-scoped one +(items 105 to 111), and `revertPendingTagChange()` undoes either if the write +is rejected. A chip, a bold row and a dimmed row all move the moment the user +acts. + +The optimistic **membership** is not. `ThreadListModel::removeThreadsWithoutTag()` +has exactly ONE caller, in `trashMessages()`, added last session because Delete +strips `inbox` and a deleted message sat in the Inbox view across restarts. The +ordinary tag path never calls it: neither `sendMessageTagChange()` nor +`sendThreadTagChange()` asks whether the row still belongs in the view. + +So in the Unread view, marking a message read repaints the row and leaves it in +a list defined by `tag:unread`, which it no longer matches. Un-flagging in the +Flagged view is the same, and so is removing `inbox` by hand from the Inbox +view. It corrects itself at the next query or sync, which is exactly the "waits +for a view change" the note describes. + +**Approach.** Not a refactor. `viewFilterTag()` already resolves the view's own +tag from the query, and `removeThreadsWithoutTag()` already does the removal. +The gap is that the guard sits in `trashMessages()` rather than at the funnel +every tag write passes. Move it, or call it from both send paths. + +**Constraints.** + +- The guard's existing reasoning is what makes this safe and must be kept: only + a plain `tag:<x>` view has a membership one tag decides. A path query (Trash, + Sent, Drafts) is unaffected by a tag going away, and a hand-typed query cannot + be reasoned about. Both are left alone. Without that, marking read in an `id:` + view would empty the list. +- A row leaving is not revertible by `revertPendingTagChange()`, which repaints + rather than reinserts. A REJECTED write would leave the row gone until the + next query. The move path already carries that exposure; check whether it is + acceptable at the tag path's much higher frequency, or make the removal wait + for confirmation there. +- The inverse case is deliberately out of scope: a row that starts matching + cannot be inserted optimistically, since the model has no summary for a + thread the query never returned. +- Undo goes back through the same funnel, so a removal must not make an undone + mark-read invisible in the view it was undone in. |