aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-24 11:22:10 +0200
committerDanilo M. <danix@danix.xyz>2026-08-24 11:22:10 +0200
commit55fe1bca09242220e514394acbddb53be796fe40 (patch)
tree217e3bb07cc63298ab32c30fce7ee625a84c03d7
parent2703d4c42710418b0f15a41fa896824e2e1a6170 (diff)
downloadqtmaildir-55fe1bca09242220e514394acbddb53be796fe40.tar.gz
qtmaildir-55fe1bca09242220e514394acbddb53be796fe40.zip
fix(ui): move Compose back, drop the bar below the header, size its icons
Three corrections from looking at the built bar. Compose returns to the main toolbar. The split this was built to, "about a message" against "about the list", does not survive contact: what matters is what the action NEEDS. Reply and Forward are meaningless without a message on display, while Compose needs none and is disabled only when no account can send. So the pane's bar holds exactly the two actions that depend on what it is showing, and Compose sits with the window-wide ones. The bar moves below the subject and details rows, directly above the web view. At the top of the pane it read as window chrome rather than as belonging to the message. The transient notice bars stay above it: they explain the message rather than offer an action on it. Its icons were the style's own default, 16px, which is tiny beside a 32px toolbar. They are now 7/8 of toolbar_icon_size, which is the 28 the user asked for at their 32, derived rather than hardcoded so the relation holds if that key changes. The test asserts the relation as well as the value, since a bare 28 would stop meaning anything the moment the key moved. m_headerLabel gains an object name so the placement test can find the row it must sit below.
-rw-r--r--CHANGELOG.md8
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md26
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md4
-rw-r--r--src/mainwindow.cpp23
-rw-r--r--src/messageview.cpp15
-rw-r--r--src/messageview.h3
-rw-r--r--tests/test_mainwindow.cpp98
7 files changed, 156 insertions, 21 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd3c419..da2156c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,7 +23,7 @@ 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
+- **A button bar over the message pane.** Reply and Forward sit directly
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
@@ -48,9 +48,9 @@ point at which they are stable.
### 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.
+- The main toolbar keeps the actions that need no particular message
+ (Compose, Sync, Archive, Delete, Mark all read, Undo). Reply moves to the
+ new message-pane bar, where Forward joins it.
### Fixed
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 d1daa57..3c1c8fd 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
@@ -7166,6 +7166,16 @@ that grouping is theirs to make. It shares the container item 141 introduces.
`everyActionIsReachableFromAMenu()` asserts on. This is a second presentation
of the same `QAction`s. Absorbs item 139.
+**Outcome, and the revision the user made on sight.** Built 2026-08-24 with
+Compose, Reply and Forward all on the pane's bar, as the note asked. The user
+then reconsidered: Compose belongs on the main toolbar after all. The reason
+is worth keeping, because it is a better rule than the one this entry was
+written to. The useful split is not "about a message" against "about the
+list", it is what the action NEEDS: Reply and Forward are meaningless without
+a message on display, while Compose needs none and is disabled only when no
+account can send. Compose therefore sits with the window-wide actions, and the
+pane's bar holds exactly the two actions that depend on what it is showing.
+
---
## 141. The message pane has no button bar of its own
@@ -7188,4 +7198,20 @@ 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.
+**Outcome.** Message actions left, view controls right, separated by an
+expanding spacer (`QToolBar` has no `addStretch()`), with `toggle_html` the
+first view control. `MessageView::setBarActions()` is the seam, so the pane
+still knows nothing about `MainWindow`'s action map.
+
+**Two things the user corrected on sight, both about placement rather than
+contents.** The bar was first put at the TOP of the pane, above the subject
+and details rows, where it read as window chrome rather than as belonging to
+the message; it now sits directly above the web view, below those rows and
+below the transient notice bars, which explain the message rather than offer
+an action on it. And its icons were the style's own default, which is 16px
+here and tiny beside a 32px toolbar: they are 7/8 of `toolbar_icon_size`,
+derived rather than hardcoded so the relation survives the user changing that
+key. A test asserting the resulting 28 alone would stop meaning anything the
+moment it did, so it asserts the relation as well.
+
---
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 7f9c606..5a42fb3 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
@@ -209,8 +209,8 @@ taking that too literally.
| 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 | **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 |
+| 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, then REVISED the same day after the user looked at it. Reply and Forward move; **Compose stays on the main toolbar**, because the split that survives contact is what the action NEEDS rather than what it is about, and composing needs no message at all. The moved actions leave the toolbar rather than gaining a second home. 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. It sits directly above the web view, BELOW the subject and details rows, which was the user's correction after seeing it at the top of the pane read as window chrome. Icons are 7/8 of `toolbar_icon_size` (28 against the user's 32), derived so the relation survives a change to that key. `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 |
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index fd59f47..5cb1ab8 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1970,10 +1970,13 @@ void MainWindow::buildMenus()
const int iconSize = m_config.toolbarIconSize();
toolBar->setIconSize(QSize(iconSize, iconSize));
- // 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.
+ // Compose stays here, and Reply and Forward do not (item 140). The split
+ // is what the action NEEDS: composing a new message requires no message at
+ // all, so it belongs with the window-wide operations, while Reply and
+ // Forward act on whatever the pane is showing and live on its own bar.
+ toolBar->addAction(m_actions.value(QStringLiteral("compose")));
+ toolBar->addSeparator();
+
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
@@ -1997,11 +2000,17 @@ 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.
+ // Reply and Forward only: Compose needs no message and sits on the main
+ // toolbar with the other window-wide actions.
+ //
+ // Slightly smaller than the main toolbar's icons, deriving from the
+ // configured size rather than hardcoding one, so the bar stays subordinate
+ // to the chrome above it however the user sets that key.
+ const int iconSize = qMax(16, (m_config.toolbarIconSize() * 7) / 8);
m_messageView->setBarActions(
- { m_actions.value(QStringLiteral("compose")),
- m_actions.value(QStringLiteral("reply")),
+ { m_actions.value(QStringLiteral("reply")),
m_actions.value(QStringLiteral("forward")) },
- { m_actions.value(QStringLiteral("toggle_html")) });
+ { m_actions.value(QStringLiteral("toggle_html")) }, iconSize);
}
void MainWindow::showShortcutReference()
diff --git a/src/messageview.cpp b/src/messageview.cpp
index c8395f1..4c2a9d0 100644
--- a/src/messageview.cpp
+++ b/src/messageview.cpp
@@ -342,6 +342,7 @@ MessageView::MessageView(QWidget *parent)
qApp->installEventFilter(this);
m_headerLabel = new QLabel(this);
+ m_headerLabel->setObjectName(QStringLiteral("messageHeader"));
m_headerLabel->setTextFormat(Qt::RichText);
m_headerLabel->setWordWrap(true);
m_headerLabel->setTextInteractionFlags(Qt::TextSelectableByMouse);
@@ -470,13 +471,20 @@ MessageView::MessageView(QWidget *parent)
style()->styleHint(QStyle::SH_ToolButtonStyle, nullptr, m_messageBar)));
m_messageBar->setMovable(false);
m_messageBar->hide();
+ // Set by MainWindow, which owns the configured size this is derived from.
+ // Left at the style's own default until then, which is what a MessageView
+ // built on its own in a test gets.
auto *layout = new QVBoxLayout(this);
- layout->addWidget(m_messageBar);
layout->addLayout(headerRow);
layout->addWidget(m_blockedBar);
layout->addWidget(m_receiveOnlyRibbon);
layout->addWidget(m_staleBar);
+ // Directly above the message it acts on, below the subject and details
+ // rows: the bar belongs to the body, not to the pane's heading. The
+ // transient notice bars stay above it, since they explain the message
+ // rather than offer an action on it.
+ layout->addWidget(m_messageBar);
layout->addWidget(m_view, 1);
layout->addWidget(m_attachmentBar);
layout->addWidget(m_tagStrip);
@@ -583,9 +591,12 @@ void MessageView::applyNoticeBarStyles()
}
void MessageView::setBarActions(const QList<QAction *> &messageActions,
- const QList<QAction *> &viewControls)
+ const QList<QAction *> &viewControls,
+ int iconSize)
{
m_messageBar->clear();
+ if (iconSize > 0)
+ m_messageBar->setIconSize(QSize(iconSize, iconSize));
for (QAction *action : messageActions) {
if (action)
diff --git a/src/messageview.h b/src/messageview.h
index b28dcdc..c81a4f6 100644
--- a/src/messageview.h
+++ b/src/messageview.h
@@ -253,7 +253,8 @@ public:
/// 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);
+ const QList<QAction *> &viewControls,
+ int iconSize = 0);
/// Tells the pane whether the query bar currently holds anything.
///
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 2825564..183e4fa 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -422,6 +422,8 @@ private slots:
void noTwoActionsShareAnIcon();
void theMessagePaneCarriesItsOwnActionBar();
void theMainToolbarKeepsOnlyListWideActions();
+ void theMessageBarSitsAboveTheBodyAndBelowTheHeader();
+ void theMessageBarIconsAreSmallerThanTheToolbars();
void onlyPinnedQueriesBecomeButtons();
void unpinnedQueriesReachTheMenu();
@@ -7258,8 +7260,11 @@ void TestMainWindow::theMessagePaneCarriesItsOwnActionBar()
// 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"),
+ // Compose is deliberately NOT here: it needs no message, so it stays on
+ // the main toolbar with the window-wide actions. The user reconsidered
+ // this after seeing the first version, and the split is now by what the
+ // action needs rather than by what it is about.
+ const QStringList expected = { QStringLiteral("reply"),
QStringLiteral("forward") };
for (const QString &name : expected) {
auto *action = window.findChild<QAction *>(name);
@@ -7269,6 +7274,11 @@ void TestMainWindow::theMessagePaneCarriesItsOwnActionBar()
.arg(name)));
}
+ auto *compose = window.findChild<QAction *>(QStringLiteral("compose"));
+ QVERIFY(compose);
+ QVERIFY2(!bar->actions().contains(compose),
+ "compose is on the message bar, where it needs no message");
+
// 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.
@@ -7303,6 +7313,84 @@ void TestMainWindow::theMessagePaneCarriesItsOwnActionBar()
"controls, so they read as one group");
}
+void TestMainWindow::theMessageBarSitsAboveTheBodyAndBelowTheHeader()
+{
+ // The user's correction after seeing the first version: the bar belongs
+ // immediately above the message it acts on, under the subject and details
+ // rows, rather than at the very top of the pane where it read as part of
+ // the window chrome.
+ const Config config;
+ MainWindow window(config);
+
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+ auto *bar = window.findChild<QToolBar *>(QStringLiteral("message_toolbar"));
+ auto *header = pane->findChild<QLabel *>(QStringLiteral("messageHeader"));
+ QVERIFY(bar);
+
+ auto *layout = qobject_cast<QVBoxLayout *>(pane->layout());
+ QVERIFY2(layout, "the message pane is not laid out vertically");
+
+ // Index in the pane's own column, which is what "above" and "below" mean
+ // here. Asserting on geometry instead would measure the offscreen
+ // platform's idea of an unshown widget, which is nothing.
+ int barIndex = -1;
+ int viewIndex = -1;
+ int headerIndex = -1;
+ for (int i = 0; i < layout->count(); ++i) {
+ QLayoutItem *item = layout->itemAt(i);
+ if (item->widget() == bar)
+ barIndex = i;
+ else if (item->widget()
+ && item->widget()->inherits("QWebEngineView"))
+ viewIndex = i;
+ else if (header && item->layout()
+ && item->layout()->indexOf(header) >= 0)
+ headerIndex = i;
+ }
+
+ QVERIFY2(barIndex >= 0 && viewIndex >= 0,
+ "the bar or the web view is not in the pane's column");
+ QVERIFY2(barIndex < viewIndex,
+ "the message bar sits below the message body");
+ if (headerIndex >= 0) {
+ QVERIFY2(headerIndex < barIndex,
+ "the message bar sits above the header row, where it reads as "
+ "window chrome rather than as belonging to the message");
+ }
+}
+
+void TestMainWindow::theMessageBarIconsAreSmallerThanTheToolbars()
+{
+ // The bar is subordinate to the main toolbar, so its icons are smaller.
+ // Derived from the configured size rather than hardcoded, so the relation
+ // survives the user changing toolbar_icon_size.
+ QTemporaryDir dir;
+ const QString path = dir.filePath(QStringLiteral("qtmaildir.conf"));
+ QFile file(path);
+ QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text));
+ file.write("[general]\ntoolbar_icon_size = 32\n");
+ file.close();
+
+ Config config;
+ config.load(path);
+ QCOMPARE(config.toolbarIconSize(), 32);
+
+ MainWindow window(config);
+ auto *toolBar =
+ window.findChild<QToolBar *>(QStringLiteral("main_toolbar"));
+ auto *bar = window.findChild<QToolBar *>(QStringLiteral("message_toolbar"));
+ QVERIFY(toolBar && bar);
+
+ QCOMPARE(toolBar->iconSize(), QSize(32, 32));
+ QCOMPARE(bar->iconSize(), QSize(28, 28));
+
+ // The RELATION, not the constant: a fixed 28 would satisfy the line above
+ // and stop meaning anything the moment the user set a different size.
+ QVERIFY2(bar->iconSize().width() < toolBar->iconSize().width(),
+ "the message bar's icons are not smaller than the toolbar's");
+}
+
void TestMainWindow::theMainToolbarKeepsOnlyListWideActions()
{
// The other half of item 140: the actions do not merely gain a second
@@ -7315,8 +7403,7 @@ void TestMainWindow::theMainToolbarKeepsOnlyListWideActions()
window.findChild<QToolBar *>(QStringLiteral("main_toolbar"));
QVERIFY(toolBar);
- for (const QString &name : { QStringLiteral("compose"),
- QStringLiteral("reply"),
+ for (const QString &name : { QStringLiteral("reply"),
QStringLiteral("forward") }) {
auto *action = window.findChild<QAction *>(name);
QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name)));
@@ -7327,7 +7414,8 @@ void TestMainWindow::theMainToolbarKeepsOnlyListWideActions()
// 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"),
+ for (const QString &name : { QStringLiteral("compose"),
+ QStringLiteral("sync"),
QStringLiteral("archive"),
QStringLiteral("undo") }) {
auto *action = window.findChild<QAction *>(name);