aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-26 14:36:46 +0200
committerDanilo M. <danix@danix.xyz>2026-08-26 14:36:46 +0200
commitd38bbff34e0c0d1282251bd9412423aa3d182727 (patch)
treeb39eec66ce4e79aa22dd9250a603ed9c8fee744b /docs/superpowers/plans
parent0ca4624195cdd8c78ff614e3912af5b914458497 (diff)
downloadqtmaildir-d38bbff34e0c0d1282251bd9412423aa3d182727.tar.gz
qtmaildir-d38bbff34e0c0d1282251bd9412423aa3d182727.zip
docs: reconcile the backlog, and specify card avatars
Reconciliation against the user's notes found two unrecorded lines, and corrected the cause of one entry that was recorded wrongly. - 169, new: a card shows the account only as a bar, with no fade and no avatar. Half of it shipped as the accent bar. - 170, new: a row that stops matching the view only leaves it on the Delete path. Filed from a note that reads as a stale request for optimistic updates; it is not. removeThreadsWithoutTag() has exactly one caller, so marking a message read in the Unread view repaints the row and leaves it in a list it no longer belongs to. - 65, narrowed: the notes now name it as a dead-code and duplication sweep rather than a performance or security pass, so it produces a list to decide on rather than a diff. The spec for 169 covers the avatar geometry, the fade, the two hash-generated fills and the sender list that chooses between them. It also records the one structural change the feature needs and the measurement that forced it: ThreadSummary::authors carries display names only, with no address anywhere, so the identicon has nothing stable to hash and the sender list has nothing to match. ThreadSummary gains firstMessageSender, filled by the walk that already fills firstMessageId. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXCZFLXbAii5n5wtovpdhh
Diffstat (limited to 'docs/superpowers/plans')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md132
1 files changed, 124 insertions, 8 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 ffb41ae..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,7 +131,7 @@ 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 |
@@ -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,14 +367,35 @@ 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.
+**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.
-**Size: `?`, unspecified.** Do not propose a design for this; ask.
+**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
@@ -1291,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.