diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 18:22:45 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 18:22:45 +0200 |
| commit | 48c243d7aee3382713f7a4653be2179d313b432c (patch) | |
| tree | b5eb63907e3f7cff18815844baccce71f4492dc1 /docs/superpowers/plans | |
| parent | d62a9eb9694beee1674f9c63f9a77029a81627ee (diff) | |
| download | qtmaildir-48c243d7aee3382713f7a4653be2179d313b432c.tar.gz qtmaildir-48c243d7aee3382713f7a4653be2179d313b432c.zip | |
feat(ui): make Escape deselect the row as well as blank the pane
Blanking the pane while leaving the row highlighted reads as half an
action, and deselecting is what Escape means nearly everywhere else. The
user asked for two actions rather than a changed one, so clear_pane keeps
its behaviour and moves to Shift+Esc; clear_selection takes Escape and
does both. Shift+Esc rather than unbound because every action carries a
default and a test enforces it.
Clearing the selection re-adopts the thread it just cleared, unless done
in exactly the right way. clearSelection() leaves currentIndex() valid,
so onSelectionChanged takes its "one or fewer rows" branch, sees a
current row whose id differs from m_currentThreadId, and calls
onThreadSelected for it. Clearing the selection before blanking lets that
run while the id still matches, so nothing reloads, and clearing current
stops a later collapse-to-one-row reaching the same row. All four
arrangements were tried; only this one passes.
The first version of the test could not distinguish any of them. It
asserted showingPlaceholder(), which passes regardless because this
fixture has no worker, so loadThread never replies and the pane is never
repainted. currentThreadId() and currentIndex() are observable without
one, and asserting those is what made the test discriminate.
Diffstat (limited to 'docs/superpowers/plans')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 68 |
1 files changed, 67 insertions, 1 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 6405e75..2e0930c 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 @@ -98,7 +98,7 @@ taking that too literally. | 47 | The query bar looks unfinished, and cannot be cleared by mouse | presentation | XS | **done** | | 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 | open | -| 50 | Esc blanks the pane but leaves the row selected | workflow | XS | open | +| 50 | Esc blanks the pane but leaves the row selected | workflow | XS | **done** | | 51 | Clicking a subject scrolls the list sideways | presentation | XS | open | | 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | **done** | @@ -1969,6 +1969,38 @@ worker call. it somewhere that opens on every launch; a dialog the user asks for is the right shape, a status-bar field refreshed continuously is not. +### Revisited 2026-08-07, after item 30 shipped + +The placeholder pane did not exist when this was written, and it now displays +database-level counts, so "where does this live" was worth asking again. **The +answer is unchanged: a dialog.** Recorded because the reasoning is not obvious +and would otherwise be re-litigated. + +**Not on the placeholder**, even though the counts machinery is right there. +Item 30's own decision was that the pane carries counts the user ACTS on, each +one a link to the query it names, plus a sync line that appears only when +something needs attention, specifically so the pane cannot become wallpaper. +Total messages, account count and tag count are reference material read once, +not things to click. Putting them there dilutes exactly what that decision +protects. + +**What did change is the cost, not the location.** This item claims a total +message count "needs a new worker call". It no longer does, quite: +`NotmuchWorker::requestCounts(QStringList, quint64)` already exists, crosses on +a queued connection and takes an arbitrary list of queries, so the dialog can +ask for whatever it wants in one round trip. + +**One trap in reusing it.** `requestCounts` counts **threads** +(`notmuch_query_count_threads`), because item 30's pane says "N in inbox" beside +a list that shows threads. This item wants **messages**. Those differ by roughly +the reply depth of the database and the difference is not small. Reusing the +call as it stands would report a confidently wrong number under the right +label. Either add a second signal, or give the existing request a mode; do not +quietly reinterpret what it returns. + +**Still true from the constraints above:** the account count comes from config, +never from notmuch, and the tag list is already fetched for the completer. + ## 35. No refresh of the thread list after a sync **Observed (user, 2026-08-04):** "auto refresh list after sync." @@ -2864,6 +2896,40 @@ matter of adding `clearSelection()` to the existing one: - Both actions need entries in the shortcut reference, which is generated, so the descriptions must distinguish them in one line. +### Outcome (done 2026-08-07) + +Built as two actions, per the user's "two actions instead of one". +`clear_selection` takes `Esc` and does both; `clear_pane` keeps its existing +behaviour on `Shift+Esc`. It needed a default rather than being left unbound: +every action carries one, and `everyActionHasAShortcut` enforces it. + +**The reload hazard is real, and both guards against it are load-bearing.** +`clearSelection()` leaves `currentIndex()` VALID, so `onSelectionChanged()` +takes its "one or fewer rows" branch, finds a current row whose id differs from +`m_currentThreadId`, and calls `onThreadSelected` for it: the thread is +re-adopted and a `loadThread` sent for the row being cleared. Clearing the +selection BEFORE blanking means that runs while `m_currentThreadId` still names +the displayed thread, so the ids match and nothing reloads; `setCurrentIndex()` +then stops a later collapse-to-one-row reaching the same row. + +All four arrangements were tried, and **only one passes**: dropping +`setCurrentIndex()` fails, and moving either line after the blanking fails. + +**The first version of the test could not tell any of them apart.** It asserted +`showingPlaceholder()`, which passes whatever the code does, because +`test_mainwindow` has no worker: `loadThread` never replies, so nothing ever +repaints the pane. That is the standing limitation `CLAUDE.md` records, met +head-on. What IS observable without a worker is `currentThreadId()`, the id the +window sets on its way to sending the request, and `currentIndex()`. Asserting +those two is what made the test discriminate. + +**A caution about mutation testing, learned the hard way here.** Two of these +conclusions were reached and reversed before the four-way comparison settled it, +once because a mutation crashed the build and the crash was read as a test +failure. A mutation that does not compile, or that dies before the assertion, +proves nothing. Check the run actually reached the assertion before believing +what it says. + ## 51. Clicking a subject scrolls the list sideways **Observed (user, 2026-08-07):** "when selecting a row by clicking on the subject, |
