diff options
Diffstat (limited to 'docs/superpowers/plans')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 128 |
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 |
