From a8e8ca13a88f99d4f6495cbb7fd28a63d93c93c8 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 29 Aug 2026 13:05:22 +0200 Subject: feat: put Star and Archive on the message bar too Follows items 185 and 186, which established the pane's bar as where actions on the displayed message live. Star and Archive are both selection-scoped and fit that rule with nothing to decide; Archive leaves the main toolbar the way Delete did, since the same icon in two places reads as two controls when the toolbar is icon-only. Ordered by what they do rather than by where they came from: answering the message, then filing it, then destroying it, so the destructive button is not between two that are not. Mark all read deliberately stays on the main toolbar, at the user's decision. It is the one action in this window that ignores the selection and acts on every row in the view, so a bar whose every other entry acts on the one displayed message is exactly where it must not be. Item 140's toolbar test named archive as an example of a list-wide action. That was never true of it, only untested, and this item reclassifies it: the test now asserts archive LEFT the toolbar and keeps its guard on mark_all_read, which is the action that genuinely is list-wide. Closes item 189. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NY6poqw199LfFaXe5BHKNe --- CHANGELOG.md | 8 ++- .../2026-08-03-post-0.1.0-usability-closed.md | 65 +++++++++++++++++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 67 +--------------------- src/mainwindow.cpp | 34 ++++++++--- tests/test_mainwindow.cpp | 47 ++++++++++++--- 5 files changed, 135 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df73931..a985541 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,11 @@ point at which they are stable. ### Changed -- **Delete moved from the main toolbar to the message pane's bar**, beside - Reply and Forward, where it belongs with the other actions that operate on - the message being read. It is still in the Message menu and the context menu. +- **Delete and Archive moved from the main toolbar to the message pane's + bar**, beside Reply and Forward, and Star joined them there. All five act on + the message being read, which is what that bar is for. They are still in the + Message menu and the context menu. Mark all read stays on the main toolbar: + it acts on every row in the view rather than on the message you are reading. ### 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 33bba8b..26f90fa 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 @@ -9742,3 +9742,68 @@ scoping and its trash-aware visibility all stay exactly as they are. **Closed 2026-08-29**, unreleased, the two built together. See the status table in the open file for what shipped. +## 189. The message bar carries only Reply, Forward and Delete + +**Observed.** The user asks for more of the standard message actions on the +pane's own bar, now that items 185 and 186 established it as where actions on +the displayed message live: + +- **Star (Important)**, per message or selection +- **Archive**, per message or selection, possibly moved off the main toolbar + the way Delete was +- **Mark all read**, raised as a question rather than a request + +**Cause.** Nothing is broken. The bar was filled with the reply pair when it +was built (items 139 to 141) and has gained a branch at a time since: Edit +draft for a draft (157), the trash trio and Delete (185, 186). Its rule has +never been written down, which is why the third of these does not obviously +fit. + +**The rule the bar has actually followed, stated.** An action belongs on the +bar when it acts on **what the pane is displaying**. That is why Compose is on +the main toolbar and Reply is not (item 140), and it is what made Delete's move +correct in item 186. + +**So two of the three fit and one does not.** + +- **`flag`** is selection-scoped and is a toggle over the displayed message. + It fits with no decision needed. +- **`archive`** is selection-scoped and fits the same way. The open part is + only whether it LEAVES the main toolbar, as Delete did, or appears on both. + Delete left because a duplicate reads as two different controls when the + toolbar is icon-only; the same argument applies here. +- **`mark_all_read` does NOT fit.** It is the one action in the window that + deliberately ignores the selection (`markAllRead()`, "Every row, not the + selection"), and it is gated on the query being COMPLETE rather than on + anything being displayed. Putting a view-wide action on a bar whose every + other entry acts on one message is how a user marks a thousand threads read + meaning to mark one. This is the user's own `#discuss` and the answer from + here is: leave it on the main toolbar. + +**Approach.** Add `flag` and `archive` to the ordinary branch of +`refreshMessageBarActions()` beside Reply, Forward and Delete. Decide with the +user whether `archive` also comes off the main toolbar. Leave `mark_all_read` +where it is unless the user overrules the reasoning above. + +**Constraints.** + +- **The trash branch must not grow with it**, though the reason is narrower + than it first looks. Delete already strips `inbox` alongside `unread` (the + `sendMove()` call in `trashMessages()`, since 2026-08-26), so on mail THIS + application deleted, Archive is genuinely a no-op, as the user observed. + The gap is mail trashed by ANOTHER client: nothing rewrote its tags, so it + can sit in a trash folder still carrying `inbox`. That is the same reason + the trash view is path-based rather than `tag:deleted`. On such a message + Archive is a real write that strips `inbox`, counts an unsynced change and + rewrites the Maildir filename under `synchronize_flags`, all with no visible + effect, which is item 168's shape exactly. Star is harmless there but adds a + fifth button to a bar that is already three. Whatever is added goes on the + ORDINARY branch only. +- **Five buttons is the practical ceiling** on an icon-only bar next to the + view controls, so this is close to full. Anything after this needs the + overflow question answering rather than one more icon. +- **Every action added must carry a distinct icon**, which the existing test + enforces for anything reachable from a toolbar. `flag` and `archive` already + have their own. + +**Closed 2026-08-29**, unreleased. Star and Archive added to the ordinary branch; Archive left the main toolbar; `mark_all_read` stays there by the user's decision, and the assertion that it does is now in the suite. 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 8370e35..442e0cf 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 @@ -262,7 +262,7 @@ taking that too literally. | 186 | Delete sits on the main toolbar rather than beside Reply and Forward | presentation | XS | **done 2026-08-29**, unreleased, with 185. Moved to the message bar's ordinary branch; still in the Message and context menus. Section in the closed file. Original entry: `toolBar->addAction(... "delete")` at `mainwindow.cpp:2251`. The user places it with the message actions, so this rides with item 185 rather than being done alone: moving it before the bar is trash-aware leaves Delete in a bar that still offers Reply on trashed mail | | 187 | There is no Spam view beside Trash | workflow | S | open, 2026-08-29, from the notes. `kQueryGenerators` (`config.cpp:62`) holds six generators and no `spam`, while the `spam` ACTION has existed since 0.2.x and writes the tag. So mail can be marked spam and never listed. A tag generator like `unread`, not a folder one like `trash`: nothing in the config names a spam folder, and adding one would make it a per-account mandatory key like `trash` | | 188 | Does Empty trash respect the account selector? | question | XS | **answered 2026-08-29** by reading the code, no work needed. It does: `MainWindow::emptyTrash()` (`mainwindow.cpp:6567`) reads `m_accountBox->currentData()` and uses `allTrashQuery()` only for All accounts, and the confirmation names which. Recorded so the notes' question has an answer rather than sitting open | -| 189 | The message bar carries only Reply, Forward and Delete | presentation | S | open, 2026-08-29, from the notes, and a follow-on from 185 and 186 rather than a defect. Asks for Star (`flag`) and Archive on the bar, and raises Mark all read as a question. Two of the three are selection-scoped and fit the bar's rule as it stands; **`mark_all_read` does not**, since it deliberately ignores the selection and acts on every row in the view, which is the one action in the window that does. Needs a decision from the user on that one and on whether Archive LEAVES the main toolbar the way Delete did | +| 189 | The message bar carries only Reply, Forward and Delete | presentation | S | **done 2026-08-29**, unreleased. Star and Archive joined the bar's ordinary branch, Archive leaving the main toolbar as Delete did. `mark_all_read` deliberately did NOT move, at the user's decision: it is the one action that ignores the selection. Item 140's toolbar test listed `archive` as a list-wide action and had to be corrected, which is the classification this item changed. Section in the closed file. Original entry: Asks for Star (`flag`) and Archive on the bar, and raises Mark all read as a question. Two of the three are selection-scoped and fit the bar's rule as it stands; **`mark_all_read` does not**, since it deliberately ignores the selection and acts on every row in the view, which is the one action in the window that does. Needs a decision from the user on that one and on whether Archive LEAVES the main toolbar the way Delete did | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1462,68 +1462,3 @@ dialog names the scope ("every account" or the account's display name). Recorded rather than dropped so the notes' question has an answer here, which is where the user will look for it. - - -## 189. The message bar carries only Reply, Forward and Delete - -**Observed.** The user asks for more of the standard message actions on the -pane's own bar, now that items 185 and 186 established it as where actions on -the displayed message live: - -- **Star (Important)**, per message or selection -- **Archive**, per message or selection, possibly moved off the main toolbar - the way Delete was -- **Mark all read**, raised as a question rather than a request - -**Cause.** Nothing is broken. The bar was filled with the reply pair when it -was built (items 139 to 141) and has gained a branch at a time since: Edit -draft for a draft (157), the trash trio and Delete (185, 186). Its rule has -never been written down, which is why the third of these does not obviously -fit. - -**The rule the bar has actually followed, stated.** An action belongs on the -bar when it acts on **what the pane is displaying**. That is why Compose is on -the main toolbar and Reply is not (item 140), and it is what made Delete's move -correct in item 186. - -**So two of the three fit and one does not.** - -- **`flag`** is selection-scoped and is a toggle over the displayed message. - It fits with no decision needed. -- **`archive`** is selection-scoped and fits the same way. The open part is - only whether it LEAVES the main toolbar, as Delete did, or appears on both. - Delete left because a duplicate reads as two different controls when the - toolbar is icon-only; the same argument applies here. -- **`mark_all_read` does NOT fit.** It is the one action in the window that - deliberately ignores the selection (`markAllRead()`, "Every row, not the - selection"), and it is gated on the query being COMPLETE rather than on - anything being displayed. Putting a view-wide action on a bar whose every - other entry acts on one message is how a user marks a thousand threads read - meaning to mark one. This is the user's own `#discuss` and the answer from - here is: leave it on the main toolbar. - -**Approach.** Add `flag` and `archive` to the ordinary branch of -`refreshMessageBarActions()` beside Reply, Forward and Delete. Decide with the -user whether `archive` also comes off the main toolbar. Leave `mark_all_read` -where it is unless the user overrules the reasoning above. - -**Constraints.** - -- **The trash branch must not grow with it**, though the reason is narrower - than it first looks. Delete already strips `inbox` alongside `unread` (the - `sendMove()` call in `trashMessages()`, since 2026-08-26), so on mail THIS - application deleted, Archive is genuinely a no-op, as the user observed. - The gap is mail trashed by ANOTHER client: nothing rewrote its tags, so it - can sit in a trash folder still carrying `inbox`. That is the same reason - the trash view is path-based rather than `tag:deleted`. On such a message - Archive is a real write that strips `inbox`, counts an unsynced change and - rewrites the Maildir filename under `synchronize_flags`, all with no visible - effect, which is item 168's shape exactly. Star is harmless there but adds a - fifth button to a bar that is already three. Whatever is added goes on the - ORDINARY branch only. -- **Five buttons is the practical ceiling** on an icon-only bar next to the - view controls, so this is close to full. Anything after this needs the - overflow question answering rather than one more icon. -- **Every action added must carry a distinct icon**, which the existing test - enforces for anything reachable from a toolbar. `flag` and `archive` already - have their own. diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 44c7250..504545d 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2265,11 +2265,18 @@ void MainWindow::buildMenus() } toolBar->addAction(syncAction); toolBar->addSeparator(); - toolBar->addAction(m_actions.value(QStringLiteral("archive"))); - // Delete is NOT here (item 186). It acts on the displayed message, like - // Reply and Forward, so it lives on the pane's own bar by the same rule - // items 139 to 141 settled for those two. It stays in the Message menu - // and the context menu, so nothing became unreachable. + // Neither Archive nor Delete is here (items 186 and 189). Both act on the + // displayed message, like Reply and Forward, so they live on the pane's + // own bar by the same rule items 139 to 141 settled for those two. Both + // stay in the Message menu and the context menu, so nothing became + // unreachable. + // + // mark_all_read STAYS, and that is a decision rather than an oversight + // (item 189, the user's own call). It is the one action in this window + // that deliberately ignores the selection and acts on every row in the + // view, so a bar whose every other entry acts on the one displayed + // message is exactly where it must not be: that is how a user marks a + // thousand threads read meaning to mark one. toolBar->addAction(m_actions.value(QStringLiteral("mark_all_read"))); toolBar->addSeparator(); toolBar->addAction(m_actions.value(QStringLiteral("undo"))); @@ -2356,12 +2363,23 @@ void MainWindow::populateMessageBar() } else if (currentMessageIsADraft()) { messageActions = { m_actions.value(QStringLiteral("edit_draft")) }; } else { - // Delete joins the pair here (item 186), from the main toolbar. It is - // hidden on a reply row and outside its scope by + // Delete joins the pair here (item 186), and Star and Archive with it + // (item 189), all three from the main toolbar. Every one of them acts + // on the displayed message, which is the rule this bar follows. + // + // Ordered by what they do rather than by where they came from: + // answering the message, then filing it, then destroying it. Delete + // stays last so the destructive button is not between two that are + // not. + // + // Delete is hidden on a reply row and outside its scope by // refreshTrashActions(), which the bar inherits by showing the - // window's own QActions rather than copies. + // window's own QActions rather than copies. Star and Archive need no + // such guard: both are ordinary tag writes with an undo behind them. messageActions = { m_actions.value(QStringLiteral("reply")), m_actions.value(QStringLiteral("forward")), + m_actions.value(QStringLiteral("flag")), + m_actions.value(QStringLiteral("archive")), m_actions.value(QStringLiteral("delete")) }; } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 90418e3..5817ac4 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -8416,8 +8416,14 @@ void TestMainWindow::theMainToolbarKeepsOnlyListWideActions() window.findChild(QStringLiteral("main_toolbar")); QVERIFY(toolBar); + // delete joined them under item 186, and archive under 189. Both act on + // the displayed message, which is the rule this test encodes; archive sat + // in the guard list below until 189 reclassified it, since "list-wide" was + // never true of it, only untested. for (const QString &name : { QStringLiteral("reply"), - QStringLiteral("forward") }) { + QStringLiteral("forward"), + QStringLiteral("delete"), + QStringLiteral("archive") }) { auto *action = window.findChild(name); QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name))); QVERIFY2(!toolBar->actions().contains(action), @@ -8426,10 +8432,12 @@ void TestMainWindow::theMainToolbarKeepsOnlyListWideActions() } // The guard: without it, a change emptying the toolbar entirely would pass - // every assertion above while deleting the feature. + // every assertion above while deleting the feature. mark_all_read is the + // one item 189 deliberately kept, being the only action here that ignores + // the selection outright. for (const QString &name : { QStringLiteral("compose"), QStringLiteral("sync"), - QStringLiteral("archive"), + QStringLiteral("mark_all_read"), QStringLiteral("undo") }) { auto *action = window.findChild(name); QVERIFY2(action && toolBar->actions().contains(action), @@ -13892,6 +13900,18 @@ void TestMainWindow::theMessageBarSwapsToTheTrashActionsOnTrashedMail() "the message bar lost Forward on ordinary mail"); QVERIFY2(barHolds(QStringLiteral("delete")), "Delete did not arrive on the message bar (item 186)"); + // Item 189, the same move for the two that followed it. + QVERIFY2(barHolds(QStringLiteral("flag")), + "Star did not arrive on the message bar (item 189)"); + QVERIFY2(barHolds(QStringLiteral("archive")), + "Archive did not arrive on the message bar (item 189)"); + // And the one that deliberately did NOT move. mark_all_read ignores the + // selection and acts on every row in the view, so a bar whose every other + // entry acts on one message is where it must not be. The user's call, and + // asserted so it cannot drift in later. + QVERIFY2(!barHolds(QStringLiteral("mark_all_read")), + "Mark all read is on the message bar, where a view-wide action " + "sits among per-message ones (item 189)"); QVERIFY2(!barHolds(QStringLiteral("restore")), "Restore is offered on mail that was never deleted"); QVERIFY2(!barHolds(QStringLiteral("purge")), @@ -13902,13 +13922,22 @@ void TestMainWindow::theMessageBarSwapsToTheTrashActionsOnTrashedMail() auto *mainBar = window.findChild(QStringLiteral("main_toolbar")); QVERIFY(mainBar); const auto mainActions = mainBar->actions(); - QVERIFY2(std::none_of(mainActions.cbegin(), mainActions.cend(), - [](const QAction *action) { - return action - && action->objectName() - == QStringLiteral("delete"); - }), + const auto mainBarHolds = [&](const QString &name) { + return std::any_of(mainActions.cbegin(), mainActions.cend(), + [&](const QAction *action) { + return action && action->objectName() == name; + }); + }; + QVERIFY2(!mainBarHolds(QStringLiteral("delete")), "Delete is still on the main toolbar as well as the message bar"); + QVERIFY2(!mainBarHolds(QStringLiteral("archive")), + "Archive is still on the main toolbar as well as the message bar"); + + // The guard, and the half that matters: with no positive assertion here a + // main toolbar that had lost EVERYTHING would pass the three checks + // above. mark_all_read is the action item 189 decided must stay. + QVERIFY2(mainBarHolds(QStringLiteral("mark_all_read")), + "Mark all read left the main toolbar, where item 189 kept it"); // And the trash, which is the whole point. QVERIFY2(selectById(QStringLiteral("trashed1@example.org")), -- cgit v1.2.3