summaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
diff options
context:
space:
mode:
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.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