diff options
| -rw-r--r-- | CHANGELOG.md | 9 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 66 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 72 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 28 | ||||
| -rw-r--r-- | src/mainwindow.h | 5 | ||||
| -rw-r--r-- | src/messageview.cpp | 43 | ||||
| -rw-r--r-- | src/messageview.h | 11 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 111 |
8 files changed, 266 insertions, 79 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index abd0ba7..cd3c419 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ point at which they are stable. reply follows what the message being answered used. - Drafts autosave to the account's `drafts` folder as ordinary Maildir files, so mbsync carries them to the server and another client can pick one up. +- **A button bar over the message pane.** Compose, Reply and Forward sit + above the message, with Toggle HTML at the right end. Forward had been + reachable only from the Message menu. - A reply opens with the cursor on a blank line above the quote, and with the body focused, so typing can start immediately. `[compose] quote_position` defaults to `below` (your reply first, the quote under it); `above` puts the @@ -43,6 +46,12 @@ point at which they are stable. under `assets/hooks/`. They moved from the companion `mailctl` project, which is being retired. +### Changed + +- The main toolbar keeps only the actions that act on the list or the + selection (Sync, Archive, Delete, Mark all read, Undo). Compose and Reply + move to the new message-pane bar, where Forward joins them. + ### Fixed - Sent mail and drafts no longer appear in the inbox. notmuch tags every newly diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index a4aae42..d1daa57 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -7123,3 +7123,69 @@ real context-menu event and the offscreen platform cannot deliver one. The honest options the entry named were an assertion on the production menu or a hand test; the first is impossible here, so it is the second, recorded in the test so nobody adds an assertion that appears to cover it. + +## 139. Forward is reachable only from the Message menu + +**Observed.** There is no Forward button anywhere in the interface. The action +exists and works; it is in the Message menu and nowhere else. + +**Cause, verified 2026-08-23.** `mainwindow.cpp:1714` registers the action and +`1763` adds it to `messageMenu`. The toolbar block at `1975-1994` adds Compose +and Reply but never Forward, so two thirds of the message-action set are +visible and the third is not. + +**Approach.** One line, if the toolbar is where it belongs. It probably is not: +item 140 records the user's own view that all three belong over the message +pane instead, which makes this item the cheap half of that one. Build 140 and +this closes with it; build this alone only if 140 is deferred. + +**Constraints.** The no-duplicate-icons rule covers any action that can reach +the toolbar, so Forward needs an icon distinct from Reply's rather than a +variant of it. + +--- + +## 140. Compose, Reply and Forward belong over the message pane + +**Observed.** The user's note: "'Write new message' and 'reply' live next to +the other icon only buttons, but they belong in a new bar on top of the message +pane, together with 'Forward'." + +**Cause.** Not a defect. The toolbar grew by accretion and now mixes two +different scopes: Sync, Archive, Delete, Mark all read and Undo act on the LIST +or on the selection, while Compose, Reply and Forward are about a message. The +main toolbar reads as the place for everything, so the distinction is invisible. + +**Approach.** A bar above the message pane carrying the three message actions, +and the main toolbar keeping the list-wide ones. Compose is arguably neither, +since it needs no message at all; the user grouped it with the other two, and +that grouping is theirs to make. It shares the container item 141 introduces. + +**Constraints.** The actions themselves do not move: they stay in +`m_actions`, keep their shortcuts, and keep their menu entries, which is what +`everyActionIsReachableFromAMenu()` asserts on. This is a second presentation +of the same `QAction`s. Absorbs item 139. + +--- + +## 141. The message pane has no button bar of its own + +**Observed.** The user asks for "a button bar in the message pane area", and +names `toggle_html` as a control that would fit it. + +**Cause.** Nothing exists to hang such a control on. The pane is a header +label, the web view, the attachment bar and the tag strip; a per-message +control has no home, which is why `toggle_html` lives in a menu. + +**Approach.** The container item 140 needs. Whether it holds only the three +message actions, only view controls like `toggle_html`, or both is the design +question, and it should be settled with the user before building: a bar that +mixes "act on this message" with "change how I am looking at it" is the same +confusion item 140 exists to remove, one level down. + +**Constraints.** `MessageView` is built inline in its own class rather than +from named widget classes, per CLAUDE.md, and this should not become the +exception. Size assumes 140 and 141 are built together; separately they are +each S and the seam between them is wasted work. + +--- 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 0b1195e..7f9c606 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 @@ -208,9 +208,9 @@ taking that too literally. | 136 | `undoMovesTheMessageBack` fails when run ALONE, passes in the full suite | defect | ? | open, 2026-08-21, re-measured 2026-08-24 and it is not what the row said. Filed as an intermittent race (1 in 6); it is in fact **deterministic on the selection**: 6 failures in 6 when named on the command line, and 0 failures in the full 258-test run, on a clean tree with the day's work stashed out. All three of its 15s `QTRY` timeouts expire, giving 45s against a 25s whole-suite run, so undo never moves the file rather than losing a race. A test that needs its predecessors is the likely shape (the `init()` lock-table fixture of item 61 is one candidate), which makes it a TEST defect until shown otherwise. Not caused by item 149 | | 137 | A reply to a message that arrived at two accounts can come from the wrong one | defect | S | open, 2026-08-22, found while building item 123 task 12. `ComposeContextBuilder::accountForReply()` takes `messagePaths` PLURAL to disambiguate, and nothing upstream ever gives it more than one path, so the disambiguation is inert | | 138 | No Drafts filter beside Sent and Trash | workflow | S | open, 2026-08-23, from the notes. Verified: `kQueryGenerators` has no `drafts` entry, though every account already configures a `drafts` folder. Follows the `sent` generator exactly, which composes per-account folders rather than matching a tag | -| 139 | Forward is reachable only from the Message menu | discoverability | XS | open, 2026-08-23, from the notes. Verified: `forward` is added to `messageMenu` and to no toolbar. Compose and Reply are on the toolbar, so the third member of the set is the only one hidden | -| 140 | Compose, Reply and Forward belong over the message pane, not on the main toolbar | presentation | M | open, 2026-08-23, from the notes. The user's design: a bar of its own above the message pane carrying the three message actions, leaving the main toolbar for list-wide operations. Absorbs 139, which is the same three buttons in a worse place. See also 141 | -| 141 | The message pane has no button bar of its own | presentation | M | open, 2026-08-23, from the notes. The container 140 needs, and the home the user names for a `toggle_html` control. Sized as one item with 140 if built together | +| 139 | Forward is reachable only from the Message menu | discoverability | XS | **done** 2026-08-24, unreleased, inside 140/141 as that entry said it would be. Forward is on the message pane's own bar with Compose and Reply | +| 140 | Compose, Reply and Forward belong over the message pane, not on the main toolbar | presentation | M | **done** 2026-08-24, unreleased, with 139 and 141. The three actions LEAVE the main toolbar rather than gaining a second home, which is what makes the toolbar's remaining contents mean one thing (list-wide operations). Same `QAction` objects shown twice over, never copies, so enablement and the menu entries stay single-sourced | +| 141 | The message pane has no button bar of its own | presentation | M | **done** 2026-08-24, unreleased, with 139 and 140. The design question the entry flagged was settled with the user: message actions left, view controls right, separated by an expanding spacer, with `toggle_html` the first of the latter. `MessageView::setBarActions()` is the seam, so the pane still knows nothing about `MainWindow`'s action map. Two traps: a toolbar has no `addStretch()`, and `noTwoActionsShareAnIcon` took an UNNAMED `findChild<QToolBar*>` which now has two candidates, so it is pinned to `main_toolbar` or it would assert against the wrong bar and pass while the rule went unchecked | | 142 | The composer's formatting buttons share a toolbar with Send and Attach | presentation | S | open, 2026-08-23, from the notes. Verified: one `addToolBar` carries Bold through Quote, then Attach, Remove attachment and Send. The user reads the row as a menu bar that is not one. Move the formatting half down to sit directly above the editor, beside the HTML checkbox | | 143 | The formatting buttons are text, where every editor uses icons | presentation | XS | open, 2026-08-23, from the notes. Follows 142, and cheap once the row moves. `QIcon::fromTheme` per CLAUDE.md's chrome rule, with the text kept as the tooltip | | 144 | "Also send a formatted copy" is prominent and does not say what it does | presentation | XS | open, 2026-08-23, from the notes. It means "send an HTML part as well as plain text", which the label never says. Secondary to writing the message, so it should read as such | @@ -1375,72 +1375,6 @@ importing the other. --- -## 139. Forward is reachable only from the Message menu - -**Observed.** There is no Forward button anywhere in the interface. The action -exists and works; it is in the Message menu and nowhere else. - -**Cause, verified 2026-08-23.** `mainwindow.cpp:1714` registers the action and -`1763` adds it to `messageMenu`. The toolbar block at `1975-1994` adds Compose -and Reply but never Forward, so two thirds of the message-action set are -visible and the third is not. - -**Approach.** One line, if the toolbar is where it belongs. It probably is not: -item 140 records the user's own view that all three belong over the message -pane instead, which makes this item the cheap half of that one. Build 140 and -this closes with it; build this alone only if 140 is deferred. - -**Constraints.** The no-duplicate-icons rule covers any action that can reach -the toolbar, so Forward needs an icon distinct from Reply's rather than a -variant of it. - ---- - -## 140. Compose, Reply and Forward belong over the message pane - -**Observed.** The user's note: "'Write new message' and 'reply' live next to -the other icon only buttons, but they belong in a new bar on top of the message -pane, together with 'Forward'." - -**Cause.** Not a defect. The toolbar grew by accretion and now mixes two -different scopes: Sync, Archive, Delete, Mark all read and Undo act on the LIST -or on the selection, while Compose, Reply and Forward are about a message. The -main toolbar reads as the place for everything, so the distinction is invisible. - -**Approach.** A bar above the message pane carrying the three message actions, -and the main toolbar keeping the list-wide ones. Compose is arguably neither, -since it needs no message at all; the user grouped it with the other two, and -that grouping is theirs to make. It shares the container item 141 introduces. - -**Constraints.** The actions themselves do not move: they stay in -`m_actions`, keep their shortcuts, and keep their menu entries, which is what -`everyActionIsReachableFromAMenu()` asserts on. This is a second presentation -of the same `QAction`s. Absorbs item 139. - ---- - -## 141. The message pane has no button bar of its own - -**Observed.** The user asks for "a button bar in the message pane area", and -names `toggle_html` as a control that would fit it. - -**Cause.** Nothing exists to hang such a control on. The pane is a header -label, the web view, the attachment bar and the tag strip; a per-message -control has no home, which is why `toggle_html` lives in a menu. - -**Approach.** The container item 140 needs. Whether it holds only the three -message actions, only view controls like `toggle_html`, or both is the design -question, and it should be settled with the user before building: a bar that -mixes "act on this message" with "change how I am looking at it" is the same -confusion item 140 exists to remove, one level down. - -**Constraints.** `MessageView` is built inline in its own class rather than -from named widget classes, per CLAUDE.md, and this should not become the -exception. Size assumes 140 and 141 are built together; separately they are -each S and the seam between them is wasted work. - ---- - ## 142. The composer's formatting buttons share a toolbar with Send and Attach **Observed.** The user reads the composer's top row as a menu bar that is not diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 9a94a2e..fd59f47 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -555,6 +555,9 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) } buildMenus(); + // After buildMenus(), which registers the toolbar and menu entries these + // actions already carry: the bar shows the same objects a second time. + populateMessageBar(); // After buildMenus(): QMainWindow::restoreState() matches toolbars by // object name, so they must already exist or their position is dropped. restoreUiState(); @@ -1967,15 +1970,10 @@ void MainWindow::buildMenus() const int iconSize = m_config.toolbarIconSize(); toolBar->setIconSize(QSize(iconSize, iconSize)); - // First, because composing and replying are what a user reaches for most - // (item 123). These TWO only: the other four are menu-and-key, which is - // what keeps the no-duplicate-icons rule satisfiable, since reply_no_quote - // shares reply's icon and an icon-only toolbar would make the two buttons - // indistinguishable. - toolBar->addAction(m_actions.value(QStringLiteral("compose"))); - toolBar->addAction(m_actions.value(QStringLiteral("reply"))); - toolBar->addSeparator(); - + // Compose, Reply and Forward are NOT here (item 140). They act on a + // message, where everything below acts on the list or on the selection, + // and mixing the two is what made this toolbar read as the place for + // everything. They live on the message pane's own bar instead. QAction *syncAction = m_actions.value(QStringLiteral("sync")); // Carried over from the QPushButton this replaced: with no command // configured the control is disabled, and the tooltip is the only thing @@ -1994,6 +1992,18 @@ void MainWindow::buildMenus() toolBar->addAction(m_actions.value(QStringLiteral("undo"))); } +void MainWindow::populateMessageBar() +{ + // The window's own QActions, shown a second time rather than copied: a + // duplicate QAction would need its own enablement and would drift from the + // menu entry that updateComposeActions() keeps in step. + m_messageView->setBarActions( + { m_actions.value(QStringLiteral("compose")), + m_actions.value(QStringLiteral("reply")), + m_actions.value(QStringLiteral("forward")) }, + { m_actions.value(QStringLiteral("toggle_html")) }); +} + void MainWindow::showShortcutReference() { // Generated from the actions, so it cannot disagree with what the keys diff --git a/src/mainwindow.h b/src/mainwindow.h index ea3ba61..909b04b 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -637,6 +637,11 @@ private: void registerActions(); void buildMenus(); + + /// Fills the message pane's own bar with the three message actions and + /// toggle_html (items 139 to 141). Called after buildMenus(), which is + /// what creates the actions it hands over. + void populateMessageBar(); void wireWorker(); /// Asks the worker to re-enumerate the database tags for the completer. diff --git a/src/messageview.cpp b/src/messageview.cpp index 22df6ce..c8395f1 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -37,6 +37,9 @@ #include <QTimer> #include <QTreeWidget> #include <QVBoxLayout> +#include <QStyle> +#include <QSizePolicy> +#include <QToolBar> #include <QWheelEvent> #include <QWebEnginePage> #include <QWebEngineProfile> @@ -456,7 +459,20 @@ MessageView::MessageView(QWidget *parent) menu.exec(globalPos); }); + // The pane's own action bar (items 139 to 141). Empty until MainWindow + // fills it: the actions belong to the window, and MessageView deliberately + // knows nothing about the action map. + m_messageBar = new QToolBar(this); + m_messageBar->setObjectName(QStringLiteral("message_toolbar")); + // The desktop's own button style, for the reason the main toolbar records: + // a hardcoded setToolButtonStyle() overrides the user's "Icon only". + m_messageBar->setToolButtonStyle(static_cast<Qt::ToolButtonStyle>( + style()->styleHint(QStyle::SH_ToolButtonStyle, nullptr, m_messageBar))); + m_messageBar->setMovable(false); + m_messageBar->hide(); + auto *layout = new QVBoxLayout(this); + layout->addWidget(m_messageBar); layout->addLayout(headerRow); layout->addWidget(m_blockedBar); layout->addWidget(m_receiveOnlyRibbon); @@ -566,6 +582,33 @@ void MessageView::applyNoticeBarStyles() } } +void MessageView::setBarActions(const QList<QAction *> &messageActions, + const QList<QAction *> &viewControls) +{ + m_messageBar->clear(); + + for (QAction *action : messageActions) { + if (action) + m_messageBar->addAction(action); + } + + // The stretch is what separates the two scopes, so the view controls end + // up at the right edge. A QToolBar has no addStretch(), so it takes an + // expanding spacer widget. + if (!viewControls.isEmpty()) { + auto *spacer = new QWidget(m_messageBar); + spacer->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); + m_messageBar->addWidget(spacer); + + for (QAction *action : viewControls) { + if (action) + m_messageBar->addAction(action); + } + } + + m_messageBar->setVisible(!m_messageBar->actions().isEmpty()); +} + void MessageView::clear() { m_items.clear(); diff --git a/src/messageview.h b/src/messageview.h index 29101d6..b28dcdc 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -32,6 +32,7 @@ class QLabel; class QMenu; +class QToolBar; class QWebEnginePage; class QPushButton; class QWebEngineView; @@ -245,6 +246,15 @@ public: /// after three wrong theories. Keep the two questions separate. static void addPaneActions(QMenu *menu, QWebEnginePage *page); + /// Fills the pane's own action bar (items 139 to 141). The actions are + /// MainWindow's own, shown a second time rather than duplicated, so they + /// keep one enablement state and one menu entry. \p viewControls are + /// separated from \p messageActions by a stretch: acting on the message + /// and changing how it is displayed are different scopes, which is the + /// confusion the bar exists to remove one level up. + void setBarActions(const QList<QAction *> &messageActions, + const QList<QAction *> &viewControls); + /// Tells the pane whether the query bar currently holds anything. /// /// The menus need it to grey out "Exclude from search": excluding from an @@ -405,6 +415,7 @@ private: CidSchemeHandler *m_cidHandler = nullptr; QLabel *m_headerLabel = nullptr; + QToolBar *m_messageBar = nullptr; QWidget *m_blockedBar = nullptr; QLabel *m_blockedLabel = nullptr; QLabel *m_receiveOnlyRibbon = nullptr; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 84a6fd1..2825564 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -420,6 +420,8 @@ private slots: void placeholderCountsDropAnUncountableQuery(); void flatModeDoesNotSurviveTheNextQuery(); void noTwoActionsShareAnIcon(); + void theMessagePaneCarriesItsOwnActionBar(); + void theMainToolbarKeepsOnlyListWideActions(); void onlyPinnedQueriesBecomeButtons(); void unpinnedQueriesReachTheMenu(); @@ -7233,6 +7235,109 @@ void TestMainWindow::flatModeDoesNotSurviveTheNextQuery() QVERIFY(!model->flatMode()); } +void TestMainWindow::theMessagePaneCarriesItsOwnActionBar() +{ + // Items 139, 140 and 141. The main toolbar had grown to mix two scopes: + // Sync, Archive, Delete, Mark all read and Undo act on the LIST, while + // Compose, Reply and Forward are about a message. Forward was on no + // toolbar at all and reachable only from the Message menu, which is + // item 139. + const Config config; + MainWindow window(config); + + auto *bar = window.findChild<QToolBar *>(QStringLiteral("message_toolbar")); + QVERIFY2(bar, "the message pane has no action bar"); + + // Inside the message pane, not merely somewhere in the window: the point + // of the item is WHERE it sits. + auto *pane = window.findChild<MessageView *>(); + QVERIFY(pane); + QVERIFY2(pane->isAncestorOf(bar), + "the message bar is not inside the message pane"); + + // The three message actions, in the user's order, and the same QAction + // objects the menus use rather than copies: a second QAction would need + // its own enablement and would drift from the menu entry. + const QStringList expected = { QStringLiteral("compose"), + QStringLiteral("reply"), + QStringLiteral("forward") }; + for (const QString &name : expected) { + auto *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name))); + QVERIFY2(bar->actions().contains(action), + qPrintable(QStringLiteral("%1 is not on the message bar") + .arg(name))); + } + + // toggle_html is the view control the user named for this bar. It is a + // different scope from the three above ("change how I am looking at it", + // not "act on this"), so it sits apart from them, after a stretch. + auto *toggleHtml = + window.findChild<QAction *>(QStringLiteral("toggle_html")); + QVERIFY(toggleHtml); + QVERIFY2(bar->actions().contains(toggleHtml), + "toggle_html is not on the message bar"); + + const QList<QAction *> actions = bar->actions(); + const int lastMessageAction = + actions.indexOf(window.findChild<QAction *>(QStringLiteral("forward"))); + const int htmlIndex = actions.indexOf(toggleHtml); + QVERIFY2(lastMessageAction >= 0 && htmlIndex > lastMessageAction, + "toggle_html does not sit after the three message actions"); + + // Order alone is not the property: the two groups must be SEPARATED, which + // is an expanding spacer between them, and a test asserting only on the + // index passes with the spacer deleted (measured). Find the widget the + // toolbar made for it and check it expands and sits between the groups. + int spacerIndex = -1; + for (int i = 0; i < actions.size(); ++i) { + auto *widget = bar->widgetForAction(actions.at(i)); + if (widget + && widget->sizePolicy().horizontalPolicy() == QSizePolicy::Expanding) { + spacerIndex = i; + break; + } + } + QVERIFY2(spacerIndex > lastMessageAction && spacerIndex < htmlIndex, + "no expanding spacer separates the message actions from the view " + "controls, so they read as one group"); +} + +void TestMainWindow::theMainToolbarKeepsOnlyListWideActions() +{ + // The other half of item 140: the actions do not merely gain a second + // home, they LEAVE the main toolbar, which is what makes its remaining + // contents mean one thing. + const Config config; + MainWindow window(config); + + auto *toolBar = + window.findChild<QToolBar *>(QStringLiteral("main_toolbar")); + QVERIFY(toolBar); + + for (const QString &name : { QStringLiteral("compose"), + QStringLiteral("reply"), + QStringLiteral("forward") }) { + auto *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name))); + QVERIFY2(!toolBar->actions().contains(action), + qPrintable(QStringLiteral("%1 is still on the main toolbar") + .arg(name))); + } + + // The guard: without it, a change emptying the toolbar entirely would pass + // every assertion above while deleting the feature. + for (const QString &name : { QStringLiteral("sync"), + QStringLiteral("archive"), + QStringLiteral("undo") }) { + auto *action = window.findChild<QAction *>(name); + QVERIFY2(action && toolBar->actions().contains(action), + qPrintable(QStringLiteral("%1 left the main toolbar, which " + "should keep the list-wide actions") + .arg(name))); + } +} + void TestMainWindow::noTwoActionsShareAnIcon() { // Reported by the user against the icons shipped in 0.12.0: Archive and @@ -7279,7 +7384,11 @@ void TestMainWindow::noTwoActionsShareAnIcon() // The exception must not become a hiding place: every one of them still // has to carry an icon, which everyActionCarriesAnIcon asserts, and none // may sit on the toolbar. - auto *toolBar = window.findChild<QToolBar *>(); + // BY NAME. There are two toolbars since item 140, and an unnamed + // findChild returns whichever comes first: pointed at the message pane's + // bar, this loop would assert that a thread action is absent from a bar + // that never holds any, and pass while the rule went unchecked. + auto *toolBar = window.findChild<QToolBar *>(QStringLiteral("main_toolbar")); QVERIFY(toolBar); for (const QString &name : menuOnlySharedIconActions) { auto *action = window.findChild<QAction *>(name); |