aboutsummaryrefslogtreecommitdiffstats
path: root/docs
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-06 20:06:45 +0200
committerDanilo M. <danix@danix.xyz>2026-08-06 20:06:45 +0200
commitca89402d7e244e2616fdf13eb35c083afeb21826 (patch)
tree34a3278510205e783e023f2c3cbcb2622e501067 /docs
parenteef7f6cc1b2f845d2ca1eba53336f64597c49e62 (diff)
downloadqtmaildir-ca89402d7e244e2616fdf13eb35c083afeb21826.tar.gz
qtmaildir-ca89402d7e244e2616fdf13eb35c083afeb21826.zip
fix(ui): one Sync control, and a query bar that looks finished
Sync had two controls that behaved differently. The QPushButton beside the query bar cleared the log, opened the pane and disabled itself; the QAction behind the toolbar, menu and shortcut called start() and did nothing else, discarding its return value so a rejected start was silent. Worse, item 29's "disable Sync during a background sync" set the button only, so the toolbar entry stayed clickable through a cron sync and could only produce the script's EX_TEMPFAIL skip. startSync() is now the single handler behind every route in, and the enabled state lives on the QAction, which reaches the toolbar, the menu and the shortcut at once. It also reports when no sync command is configured rather than doing nothing. The QPushButton is gone. It read as a Search button given it sat beside a text field, which is the user's own observation and the reason the toolbar one survives instead. Its unavailable-command tooltip moved to the action, since that is the only thing that says why the control is dead. Removing it left the query field running flush to the window edge, so the saved-query buttons move from their own row onto the query row. The bar is now framed by the account dropdown on the left and the saved queries on the right, the empty row is gone, and the thread list gains the space. The field also gains setClearButtonEnabled, which is Qt's own themed clear icon rather than a hand-rolled button. A "Search" button was considered and rejected: Return already runs the query. No overflow handling for [queries], which is unbounded. Three entries fit; item 23 already specifies buttons-plus-menu and is where that belongs. CLAUDE.md's architecture diagram named four widget classes that have never existed, QueryBar, SavedQueryBar, HeaderWidget and AttachmentBar. The query row and the message header are built inline. Corrected, and the components that do exist but were missing from it added. Tests: the new action test was verified red first and load-bearing by mutation. The old button test is deleted rather than repointed, being an exact duplicate of it, and the unobservable-lock test now drives the action. The clear button and the row layout were confirmed by hand; no test clicks the icon, which is a mouse path. Backlog: 45 done and reclassified as a defect rather than a cosmetic redundancy, 47 added for the bar.
Diffstat (limited to 'docs')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md128
1 files changed, 124 insertions, 4 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 dd6a8ca..85e6ce8 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
@@ -92,8 +92,9 @@ taking that too literally.
| 42 | "Syncing..." says nothing about what is being synced | feedback | S | open |
| 43 | No "Mark all read" for the current view | workflow | S | open |
| 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified |
-| 45 | Two Sync buttons on the main window | discoverability | XS | open |
+| 45 | Two Sync buttons, and only one of them works properly | correctness | S | **done** |
| 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | **done** |
+| 47 | The query bar looks unfinished, and cannot be cleared by mouse | presentation | XS | **done** |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -2175,17 +2176,91 @@ passes.** `m_syncButton` is a `QPushButton` created in `buildUi()`
toolbar, from item 3's menu work. Nothing removed the original button when the
toolbar gained one, so the window shows both.
-**Approach.** Drop the standalone `QPushButton` and keep the toolbar action.
-The action carries its shortcut, its enabled state and its menu entry from one
+### Revised 2026-08-06: this is a defect, not a cosmetic cleanup
+
+**The two controls do different things**, which the original write-up assumed
+away by treating them as duplicates. Confirmed in code after the user reported
+that the toolbar one "doesn't perform a sync":
+
+- The `QPushButton` handler (`src/mainwindow.cpp:464`) starts the sync, clears
+ the log pane, shows it, disables the button, and reports "Sync already
+ running" when `start()` returns false.
+- The `sync` QAction handler (`src/mainwindow.cpp:721`) is
+ `if (m_sync->isAvailable()) m_sync->start();` and nothing else. No log pane,
+ no disable, and the return value is discarded, so a rejected start is silent.
+
+So the toolbar button most likely *does* start a sync; every piece of evidence
+that it did lives in the other handler. That is item 13's failure restated: the
+feedback exists where the user cannot see it.
+
+**Worse, item 29 shipped for one widget only.** `onExternalSyncStateChanged`
+disables `m_syncButton` during a background sync (`src/mainwindow.cpp:1606`) and
+never touches the action. During a cron sync the toolbar Sync stays clickable
+and can only produce the EX_TEMPFAIL skip, which is the exact behaviour item 29
+exists to prevent. The user's note about "2 sync buttons" is therefore sitting
+on top of a live defect rather than a redundancy.
+
+**The user's preference (2026-08-06):** keep the top-left one, next to Archive,
+Delete and Undo. The one beside the query bar reads instinctively as a Search
+button, which is a real misaffordance given what sits next to it.
+
+**Approach.** Not "pick a survivor". Move the button's handler onto the action,
+so the two behave identically, then drop the now-redundant `QPushButton`. The
+action carries its shortcut, its enabled state and its menu entry from one
place, which is the whole point of item 3's conversion; the loose button is the
last widget that predates it.
+Route the enabled state through the action too, so `setEnabled` has one target
+rather than two that can disagree. `QAction::setEnabled` propagates to every
+widget showing it, which is what makes this smaller than it looks.
+
**Constraints.**
- **The button is not just a button today.** It is disabled while a sync runs,
including one started externally (item 27/29), and `test_mainwindow` asserts on
it by name. Whatever replaces it has to carry that state, and the tests need
- pointing at the action rather than at the widget.
+ pointing at the action rather than at the widget. Three tests find it via
+ `findChild<QPushButton *>("syncButton")`, including the one item 38 just
+ fixed, so they move together with the widget.
+
+- **A test must cover the toolbar path specifically.** The whole defect is that
+ one of two controls was never given the behaviour, and a test that drives only
+ the surviving widget would have passed throughout. Assert on the action's
+ enabled state during a background sync, which is the half that silently never
+ worked.
+
+### Outcome (done)
+
+Built in the order the user asked for: make the toolbar control work, prove it,
+then remove the other one.
+
+`startSync()` is now the single handler behind every route in, the toolbar, the
+File menu, the shortcut and, until it was removed, the button. The old action
+handler was `if (m_sync->isAvailable()) m_sync->start();`, which cleared no log,
+opened no pane, disabled nothing and discarded `start()`'s return value, so a
+rejected start was silent. It now also reports when no sync command is
+configured rather than doing nothing at all.
+
+`setSyncBusy()` sets the enabled state on the QAction, which reaches the toolbar
+button, the menu entry and the shortcut at once. That was the actual defect:
+item 29 set a separate QPushButton and never touched the action.
+
+**Verified red first, then load-bearing.**
+`theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock` fails before the fix
+with the action still enabled during a background sync, and fails again when the
+action's `setEnabled` is removed afterwards. The pre-existing button test passed
+throughout, which is exactly why the defect survived item 29: it drove the half
+that worked.
+
+**Confirmed by hand before the widget was removed**, per the user's condition:
+reading mail grew the unsynced count, the toolbar Sync ran the sync, and the app
+refreshed and reported "Sync complete" at the end.
+
+**Then the QPushButton went.** Its unavailable-command tooltip moved to the
+action, since with no command configured the control is disabled and the tooltip
+is the only thing that says why. The old button test was deleted rather than
+repointed, being an exact duplicate of the new action test, and the
+unobservable-lock-table test now asserts on the action.
- Check for other loose widgets doing the same thing before touching this one, so
the fix is not repeated per widget later.
- Removing a visible control is the kind of change that looks like a regression.
@@ -2241,6 +2316,51 @@ Verified both ways round, since this one passed on Wayland throughout: 45 of 45
under offscreen where it previously failed, and still green on the real
platform.
+## 47. The query bar looks unfinished, and cannot be cleared by mouse
+
+**Observed (user, 2026-08-06), immediately after item 45 removed the Sync
+button:** the bar "having no button seems kind of incomplete", and the user
+asked whether a clear icon could be shown in it.
+
+**Cause:** the query field was the last stretching item in its row, so with the
+Sync button gone it ran flush to the window edge with nothing terminating it.
+Clearing it needed the keyboard; `QLineEdit` does not draw a clear button unless
+asked.
+
+**Approach, and what was deliberately NOT built.** The user's first instinct was
+a "🔎 Search" button. That was argued against and dropped: Return already runs
+the query, and a button beside a text field is exactly what read as Search and
+got removed in item 45. Adding one back would restate items 13 and 45 in a new
+spot.
+
+What was built instead:
+
+- `setClearButtonEnabled(true)` on the query field. Qt draws the ✕ inside the
+ field, shows it only when there is text, and themes it from the desktop. One
+ line, no icon asset, no new widget.
+- The saved-query buttons moved from their own row onto the query row, after the
+ field, so the bar is framed by the account dropdown on the left and the saved
+ queries on the right. The row they left is gone and the thread list gains that
+ vertical space. This was the user's own proposal and it addresses the
+ "incomplete" reading directly, without adding a control.
+
+**Constraints.**
+
+- **`[queries]` is unbounded.** Three entries fit comfortably; enough of them
+ would squeeze the field. No overflow handling was built, marked with a
+ `ponytail:` comment pointing at item 23, which already specifies
+ buttons-plus-menu and is where that belongs.
+- Button order follows `childKeys()`, which sorts alphabetically, so the buttons
+ read Flagged, Inbox, Unread regardless of the order written in the config.
+ Pre-existing, and its own item if it matters.
+
+**Known behaviour, accepted:** clicking ✕ focuses the field, and an empty field
+makes `QueryCompleter` offer everything, so the popup opens. Confirmed by the
+user as acceptable; suppressible if it becomes annoying in daily use.
+
+**Verification:** by hand. The tests do not click the ✕, which is a mouse path.
+The user confirmed the icon renders correctly, is themed, and clears the field.
+
## Deferred, unsized, or split out
Items noted while triaging but not part of the original list. Same numbering