diff options
| -rw-r--r-- | CHANGELOG.md | 15 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 72 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 48 | ||||
| -rw-r--r-- | src/keymap.cpp | 4 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 147 | ||||
| -rw-r--r-- | src/mainwindow.h | 18 | ||||
| -rw-r--r-- | src/messageview.cpp | 36 | ||||
| -rw-r--r-- | src/messageview.h | 9 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 122 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 19 |
10 files changed, 470 insertions, 20 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2040fba..df73931 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,15 @@ point at which they are stable. ### Added +- **The message pane's bar knows about the trash.** On a message that is + already in the trash it now carries Restore, Delete permanently and Empty + trash in place of Reply and Forward, which were the two least useful things + to offer on mail that was thrown away. Restore is tinted so the one action + that gives mail back is not the same shape as the two that destroy it. +- **Delete permanently.** The selection-scoped counterpart to Empty trash: + the same act, on the messages you picked rather than on the whole trash. Like + Empty trash it asks first, names the count, and carries no default shortcut, + because it cannot be undone. - **The sync script reports what it did.** `mailsync.sh` now writes a small status file at the end of every run, naming the channels it synced, whether each of mbsync and notmuch succeeded, and whether the run was skipped because @@ -20,6 +29,12 @@ point at which they are stable. outcome from a line in the log. Set `status` under `[sync]` to move the file; the default matches what the script writes and no config change is needed. +### 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. + ### Fixed - **A background sync now clears only the accounts it actually carried.** A 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 0972168..33bba8b 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 @@ -9670,3 +9670,75 @@ The test drives `onSyncFinished()` with the skip code after recording a real pending edit, since `runAutoSync()` correctly declines when there is nothing to carry and a fixture without one would arm nothing for a legitimate reason. Mutation-checked by removing the call. + +## 185. The message-pane bar offers Reply and Forward on a trashed message + +**Observed.** In the Trash view the bar above the message pane reads Reply and +Forward. Neither is a thing to do with a message the user threw away, and the +two actions that are, Restore and Delete permanently, are reachable only from +the Message menu and the context menu. + +**Cause.** `MainWindow::refreshMessageBarActions()` (`mainwindow.cpp:2311`) +picks the bar's message half from exactly one question, whether the displayed +message is a draft: + +- a draft gets `edit_draft` +- everything else gets `reply` and `forward` + +The trash is not a third case, so a trashed message takes the `else` branch. +The knowledge needed is already computed elsewhere: +`refreshTrashActions()` (`mainwindow.cpp:3735`) already decides `inTrash` and +already toggles `delete` and `restore` on it, for the menus. + +**Approach.** Give the bar a third branch that asks the same `inTrash` +question, and fill it with `restore` and `empty_trash`, or with `restore` and a +per-selection permanent delete if the user wants one that is not the whole +trash. The notes ask for both to grey out when nothing is selected, which is +what the existing visibility rules already do for the menu entries; the bar +should read the same actions rather than duplicating the predicate. + +**Constraints.** + +- **One source for the predicate.** `inTrash` is computed in + `refreshTrashActions()`; the bar must read that rather than deriving the + trash-ness a second way, or the two surfaces will disagree the first time a + view changes. +- **"Delete permanently" for a selection does not exist yet.** Item 118 built + `empty_trash` for the whole trash and deliberately gave it the project's one + confirmation. A per-selection permanent delete is a SECOND irreversible + action and inherits that rule, including the confirmation and the absence of + a default shortcut. Decide with the user whether the bar offers the existing + whole-trash action or a new selection-scoped one; they are different items of + work. +- **Item 186 rides with this**, since moving Delete into a bar that still + offers Reply on trashed mail improves nothing. + +**Closed 2026-08-29**, unreleased, the two built together. See the status table in the open file for what shipped. + +## 186. Delete sits on the main toolbar rather than beside Reply and Forward + +**Observed.** The user reads Delete as a message action and expects it in the +message pane's own bar, with Reply and Forward, not on the window's main +toolbar. + +**Cause.** Placement, nothing more: `toolBar->addAction(m_actions.value("delete"))` +at `mainwindow.cpp:2251`, beside `archive`, `mark_all_read` and `undo`. + +**Approach.** Remove it from the main toolbar's list and add it to the +`messageActions` list in `refreshMessageBarActions()`. The action itself, its +scoping and its trash-aware visibility all stay exactly as they are. + +**Constraints.** + +- **Do it with item 185, not before it.** The bar is not trash-aware yet, so + Delete would land next to a Reply button offered on a trashed message. +- **The bar hides over an empty pane** (`messageview.cpp:710`), so Delete + disappears when no message is displayed, where the toolbar kept it visible + and disabled. That is a behaviour change worth confirming rather than + discovering. +- **The no-duplicate-icons test covers the toolbar**, so check + `test_mainwindow`'s icon and menu-reachability assertions still hold after + the move; the action stays in the Message menu either way. + +**Closed 2026-08-29**, unreleased, the two built together. See the status table in the open file for what shipped. + 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 aa56727..3ace9d2 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 @@ -258,6 +258,10 @@ taking that too literally. | 182 | An edit made during a sync is announced twice and never says it is waiting | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`, found by hand. The hold branches set a deliberately NON-transient label; all three callers overwrote it a line later with the bare action, so the user was told the write had landed and then told again when it really did. `announceAction()` adds the wait to the action rather than replacing it, since that announcement is what stands in for the confirmation dialog this project rules out. Section in the closed file | | 183 | `undoingAMarkReadRestoresOnlyWhatWasUnread` fails about 1 run in 9 under the full suite | testing | ? | open, 2026-08-29, measured. Item 176's regression test, which guards the undo that rewrote 44 messages of real mail. Nine runs on master: 4 standalone, 3 under `ctest -R mainwindow`, 3 under the FULL parallel suite, and the single failure was in the last group. Not a regression, the base commit behaves the same. Probably the same root cause as item 136 and worth solving with it | | 184 | New mail waits up to ten minutes, because sync is a fixed cron tick | workflow | ? | open, 2026-08-29, from the user: the 10 minute tick "has always bothered me", and it is already a compromise down from 30. Outgoing edits are immediate (`auto_sync_delay_ms`), so this is the INCOMING half only. Polling faster is not the answer; IMAP IDLE is, and it lives in a watcher that triggers `mailsync.sh`, NOT in qtmaildir, which does no network protocol work. Needs decisions first: which watcher, whether it packages on Slackware, and what the server supports. **Blocked on 174**, whose status file is the reporting channel this needs anyway | +| 185 | The message-pane bar offers Reply and Forward on a trashed message | presentation | S | **done 2026-08-29**, unreleased, with 186. The bar has a third branch keyed on the SELECTION being in a trash folder, the same predicate the menus use: Restore, Delete permanently and Empty trash replace the reply pair, and Restore alone is tinted. Added `purge`, the selection-scoped sibling of `empty_trash`, which inherits both its safeguards. Refilled from the digest as well as from the selection, since a conversation's trash-ness is not known until every path is reported. Section in the closed file. Original entry: `MainWindow::refreshMessageBarActions()` (`mainwindow.cpp:2311`) swaps the bar's message half for a DRAFT and for nothing else, so the trash view shows the two actions that make least sense there. The notes ask for Restore and Delete permanently in their place, and for Delete to move here from the main toolbar (item 186). The visibility rules already exist in `refreshTrashActions()`; what is missing is the bar consulting them | +| 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 | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1413,3 +1417,47 @@ repository.** 174 or 125 better than a file does, and it adds a process that can wedge and take mail delivery with it. What the user wants is a watcher, which is a different thing in a different place. + + +## 187. There is no Spam view beside Trash + +**Observed.** The user asks for a Spam view next to Trash. Mail can be marked +spam today and there is no filter that lists it. + +**Cause.** `kQueryGenerators` (`config.cpp:62`) is a closed set of six: +`unread`, `inbox`, `flagged`, `sent`, `drafts`, `trash`. There is no `spam`. +The `spam` action has existed since the first toolbar and writes the tag +(`mainwindow.cpp:1760`, adds `spam`, removes `inbox`), so the write half is +built and the read half is missing. + +**Approach.** A TAG generator, like `unread` and `flagged`, not a folder one +like `trash`: add `spam` to `kQueryGenerators`, return `spam` from +`generatorTag()`, and give it a label in `builtinFilter()`. Threaded rather +than flat, matching Trash. That is the whole change; `Config::resolvedQuery()` +already composes a tag generator with the account selector. + +**Constraints.** + +- **Do NOT make it a folder generator.** No account config names a spam + folder, and adding one would be a new mandatory per-account key with the + item 103 hazard attached: a folder name that does not exist is CREATED and + propagates to the server. +- **The label is translated, the generator is not.** `spam` is stored in + `queries.json` and matched against a closed set, so it is wire format; see + the `flagged`/"Important" note in `builtinFilter()`. +- **Adding a generator changes queries.json's readable set**, so an older + build reading a file that names `spam` reports an unknown generator and + KEEPS the row. That is the existing behaviour and needs no version bump. +- **The button row is fixed and left to right.** Decide where Spam sits with + the user; the obvious place is beside Trash, which puts it last. + +## 188. Does Empty trash respect the account selector? + +**Answered on 2026-08-29 by reading the code; no work follows from it.** It +does. `MainWindow::emptyTrash()` (`mainwindow.cpp:6567`) reads +`m_accountBox->currentData()`, uses `Config::allTrashQuery()` only when that is +empty and the account's own `trashQuery()` otherwise, and the confirmation +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. diff --git a/src/keymap.cpp b/src/keymap.cpp index 17f8241..b7cf8f5 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -37,6 +37,10 @@ QStringList KeyMap::knownActions() // that destroys mail with no undo, and a chord is how it would be run // by accident. Menu only, which item 132 made a legitimate choice. QStringLiteral("empty_trash"), + // Item 185. Empty trash's sibling, scoped to the selection, and it + // carries no default binding for exactly the same reason: the act is + // identical and so is the hazard. + QStringLiteral("purge"), QStringLiteral("spam"), QStringLiteral("toggle_unread"), QStringLiteral("mark_all_read"), diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4859b1f..44c7250 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1757,6 +1757,14 @@ void MainWindow::registerActions() tr("Permanently delete every message in the trash"), [this]() { emptyTrash(); }); + // The same act as empty_trash, scoped to the selection: the user's own + // words, "they are the same action, scoped differently". So it inherits + // both safeguards rather than being reasoned about afresh, the + // confirmation and the absent default shortcut. + addAction(QStringLiteral("purge"), tr("Delete per&manently..."), + tr("Permanently delete the selected messages"), [this]() { + purgeSelected(); + }); addAction(QStringLiteral("spam"), tr("Mark &spam"), tr("Add spam and remove inbox"), [this]() { tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, @@ -2046,6 +2054,11 @@ void MainWindow::buildMenus() // disabled entry with its shortcut beside it says both that it exists and // where it applies. messageMenu->addAction(m_actions.value(QStringLiteral("restore"))); + // Beside Restore, the other thing a trashed message affords. Hidden + // outside the trash rather than greyed, unlike Restore above: this one + // destroys, so offering it where it does not apply is worse than teaching + // that it exists. + messageMenu->addAction(m_actions.value(QStringLiteral("purge"))); messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); @@ -2122,6 +2135,11 @@ void MainWindow::buildMenus() // thing it deliberately does not do. { QStringLiteral("cleanup_stranded"), QStringLiteral("system-search") }, { QStringLiteral("empty_trash"), QStringLiteral("edit-delete-shred") }, + // Item 185. Distinct from empty_trash's, because both reach the trash + // bar and there the icon IS the control: two buttons that destroy + // different amounts of mail must not look identical. `user-trash` is + // the theme's own wastebasket, which reads as "this one, gone". + { QStringLiteral("purge"), QStringLiteral("user-trash") }, { QStringLiteral("undo"), QStringLiteral("edit-undo") }, { QStringLiteral("spam"), QStringLiteral("mail-mark-junk") }, { QStringLiteral("flag"), QStringLiteral("mail-mark-important") }, @@ -2248,7 +2266,10 @@ void MainWindow::buildMenus() toolBar->addAction(syncAction); toolBar->addSeparator(); toolBar->addAction(m_actions.value(QStringLiteral("archive"))); - toolBar->addAction(m_actions.value(QStringLiteral("delete"))); + // 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. toolBar->addAction(m_actions.value(QStringLiteral("mark_all_read"))); toolBar->addSeparator(); toolBar->addAction(m_actions.value(QStringLiteral("undo"))); @@ -2308,17 +2329,45 @@ void MainWindow::populateMessageBar() // changed the answer. The guard that IS load-bearing sits one level up in // updateComposeActions(), where accountForCurrentMessage() would otherwise // answer about a row this query is discarding. + // A THIRD branch since item 185, and the order matters: a draft that has + // been deleted is in the trash, where Edit draft is no more use than + // Reply. Trash is asked first for that reason. + // + // Keyed on the SELECTION being in a trash folder, the same predicate + // refreshTrashActions() uses, rather than on the trash VIEW: the two + // disagree on mail reached from a search, where a message can sit in the + // trash while the query was never the trash filter. Reading the view + // there would offer Reply on a trashed message, which is the whole + // complaint. QList<QAction *> messageActions; - if (currentMessageIsADraft()) { + QList<QAction *> tinted; + if (everySelectedRowIsInATrashFolder() + && !m_threadView->selectionModel()->selectedRows().isEmpty()) { + // Restore first, then the two purges, which are one act at two + // scopes: this message, and the whole trash. Delete is absent because + // refreshTrashActions() hides it on mail already in the trash, where + // it reported success and did nothing (item 168). + messageActions = { m_actions.value(QStringLiteral("restore")), + m_actions.value(QStringLiteral("purge")), + m_actions.value(QStringLiteral("empty_trash")) }; + // Restore alone. The two purges are the same act at two scopes and + // need no colour to tell them from each other, only from this one. + tinted = { m_actions.value(QStringLiteral("restore")) }; + } 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 + // refreshTrashActions(), which the bar inherits by showing the + // window's own QActions rather than copies. messageActions = { m_actions.value(QStringLiteral("reply")), - m_actions.value(QStringLiteral("forward")) }; + m_actions.value(QStringLiteral("forward")), + m_actions.value(QStringLiteral("delete")) }; } m_messageView->setBarActions( messageActions, { m_actions.value(QStringLiteral("toggle_html")) }, - iconSize); + iconSize, tinted); } void MainWindow::showShortcutReference() @@ -3743,6 +3792,15 @@ void MainWindow::refreshTrashActions() restore->setVisible((!haveSelection || inTrash) && !m_replySelectionHidesDelete); } + + // Item 185. Follows Restore exactly: both are what a message ALREADY in + // the trash affords, and neither means anything outside it. Unlike the + // pair above, it is not hidden on a reply row: Delete is a + // conversation-level act because removing one reply from a live + // conversation is not offered, but a reply that is already in the trash + // is just mail, and destroying only that one is a coherent thing to ask. + if (auto *purge = m_actions.value(QStringLiteral("purge"))) + purge->setVisible(haveSelection && inTrash); } void MainWindow::refreshUnreadAction() @@ -4263,6 +4321,12 @@ void MainWindow::onThreadDigestLoaded(const ThreadDigest &digest, m_conversationPathsThreadId = digest.threadId; m_conversationPaths = digest.messagePaths; refreshTrashActions(); + // And the bar, for the same reason and on the same line of reasoning + // (item 185). A conversation's trash-ness is not known until the digest + // reports every path, so the bar filled at selection time answered from + // the summary's single path; refilling here is what makes it agree with + // the menu entries refreshTrashActions() just corrected. + populateMessageBar(); m_messageView->showDashboard(digest); } @@ -6282,7 +6346,17 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, } if (requestTag == QStringLiteral("empty_trash")) { - confirmAndPurge(messageIds); + const QString accountKey = m_accountBox->currentData().toString(); + confirmAndPurge(messageIds, accountKey.isEmpty() + ? tr("every account") + : m_accountBox->currentText()); + return; + } + + if (requestTag == QStringLiteral("purge_selection")) { + // No scope named: the prompt says "the selection", which is what the + // user pointed at, rather than an account they did not. + confirmAndPurge(messageIds, QString()); return; } @@ -6597,25 +6671,66 @@ void MainWindow::emptyTrash() Q_ARG(QString, QStringLiteral("empty_trash"))); } -void MainWindow::confirmAndPurge(const QStringList &messageIds) +void MainWindow::purgeSelected() { - if (messageIds.isEmpty()) { - showTransientStatus(tr("The trash is already empty")); + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) { + showTransientStatus(tr("Nothing selected to delete")); return; } - const QString accountKey = m_accountBox->currentData().toString(); - const QString where = accountKey.isEmpty() - ? tr("every account") - : m_accountBox->currentText(); + if (!m_worker) { + showTransientStatus(tr("Not connected to the mail index")); + return; + } + + const ActionScope scope = m_model->scopeForSelection(rows); + + // One combined query rather than a resolve per scope, since two round + // trips would reach confirmAndPurge() twice and ask the user twice for + // one gesture. `thread:` and `id:` compose in one notmuch query, which is + // what lets a mixed selection stay a single confirmation. + QStringList terms; + terms.reserve(scope.threadIds.size() + scope.messageIds.size()); + for (const QString &id : scope.threadIds) + terms.append(QStringLiteral("thread:%1").arg(id)); + for (const QString &id : scope.messageIds) + terms.append(QStringLiteral("id:%1").arg(id)); + + if (terms.isEmpty()) + return; + + // Resolved by the WORKER, like every other count that precedes a + // destructive act: the dialog must name what will actually be destroyed, + // and the model holds whatever the view last painted. + QMetaObject::invokeMethod( + m_worker, "resolveQueryMessages", Qt::QueuedConnection, + Q_ARG(QString, terms.join(QStringLiteral(" or "))), + Q_ARG(QString, QStringLiteral("purge_selection"))); +} + +void MainWindow::confirmAndPurge(const QStringList &messageIds, + const QString &where) +{ + if (messageIds.isEmpty()) { + showTransientStatus(where.isEmpty() + ? tr("Nothing selected to delete") + : tr("The trash is already empty")); + return; + } QMessageBox box(this); box.setObjectName(QStringLiteral("emptyTrashConfirmation")); box.setIcon(QMessageBox::Warning); - box.setWindowTitle(tr("Empty trash")); - box.setText(tr("Permanently delete %n message(s) from the trash of %1?", - "", messageIds.size()) - .arg(where)); + box.setWindowTitle(where.isEmpty() ? tr("Delete permanently") + : tr("Empty trash")); + box.setText(where.isEmpty() + ? tr("Permanently delete %n selected message(s)?", "", + messageIds.size()) + : tr("Permanently delete %n message(s) from the trash " + "of %1?", "", messageIds.size()) + .arg(where)); // Said plainly, because it is the only place in this application where it // is true. box.setInformativeText(tr("This cannot be undone.")); diff --git a/src/mainwindow.h b/src/mainwindow.h index 9c93f4f..3829ae2 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1272,9 +1272,23 @@ private: /// which holds whatever the current view happens to show. void emptyTrash(); + /// Empty trash's sibling, scoped to the SELECTION rather than to the + /// account's whole trash. The act is identical, `purgeMessages()` in both + /// cases, and so are its safeguards: it confirms, and it carries no + /// default shortcut. Only where the ids come from differs, which is why + /// there are two entry points rather than a parameter. + /// + /// Asynchronous for the same reason emptyTrash() is: the count in the + /// dialog must be what will be destroyed, so a conversation row's + /// messages are resolved against the database rather than counted from + /// the model. The answer arrives at onThreadMessagesResolved() tagged + /// `purge_selection`. + void purgeSelected(); + /// The confirmation, and the only one in this application. Destroys - /// nothing if the user declines. - void confirmAndPurge(const QStringList &messageIds); + /// nothing if the user declines. \p where names the scope in the prompt, + /// since the two callers destroy very different amounts of mail. + void confirmAndPurge(const QStringList &messageIds, const QString &where); /// Moves each resolved message home, using the tags and paths the WORKER /// reported rather than anything the model holds. diff --git a/src/messageview.cpp b/src/messageview.cpp index 7106ab5..77d79dd 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -673,7 +673,8 @@ void MessageView::applyNoticeBarStyles() void MessageView::setBarActions(const QList<QAction *> &messageActions, const QList<QAction *> &viewControls, - int iconSize) + int iconSize, + const QList<QAction *> &tinted) { m_messageBar->clear(); if (iconSize > 0) @@ -684,6 +685,39 @@ void MessageView::setBarActions(const QList<QAction *> &messageActions, m_messageBar->addAction(action); } + // Applied after the actions are added, because a QToolBar creates the + // button for an action when it takes it: widgetForAction() returns + // nothing before that, so tinting in the loop above would silently do + // nothing at all. + // + // Per-button rather than a bar-wide sheet keyed on the object name, since + // the bar refills on every selection change and a sheet naming actions + // would have to be rewritten each time anyway. The ground follows the + // palette by the same rule applyNoticeBarStyles() uses: QPalette::Base + // decides which way round the theme is, so the tint cannot come out + // near-white on near-white. + if (!tinted.isEmpty()) { + const bool dark = palette().color(QPalette::Base).lightnessF() < 0.5; + // Green, not the blue the notice bars use: those are informational, + // and this marks the one button on a destructive bar that gives mail + // back. Each set carries its own ground and border rather than being + // the other dimmed, for the reason recorded beside the notice bars. + const QString ground = dark ? QStringLiteral("#12301c") + : QStringLiteral("#e2f4e6"); + const QString border = dark ? QStringLiteral("#2b5c39") + : QStringLiteral("#a9d5b5"); + const QString sheet = QStringLiteral( + "QToolButton { background: %1; border: 1px solid %2; " + "border-radius: 4px; padding: 2px; }").arg(ground, border); + + for (QAction *action : tinted) { + if (!action) + continue; + if (QWidget *button = m_messageBar->widgetForAction(action)) + button->setStyleSheet(sheet); + } + } + // 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. diff --git a/src/messageview.h b/src/messageview.h index 10b9430..54dd6e7 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -278,9 +278,16 @@ public: /// 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. + /// \p tinted names the actions whose buttons take a coloured ground. The + /// trash bar is icons-only like the rest, so the tint is what separates + /// the one action that gives mail back from the two that destroy it. Only + /// the positive one is tinted, at the user's decision: colouring all + /// three would make the bar a warning strip and the tint would stop + /// meaning anything. void setBarActions(const QList<QAction *> &messageActions, const QList<QAction *> &viewControls, - int iconSize = 0); + int iconSize = 0, + const QList<QAction *> &tinted = {}); /// Tells the pane whether the query bar currently holds anything. /// diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 48ea95b..90418e3 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -514,6 +514,7 @@ private slots: void aDraftReopensWithItsOwnContent(); void editDraftIsOfferedOnlyForADraft(); void theMessageBarOffersEditOnADraft(); + void theMessageBarSwapsToTheTrashActionsOnTrashedMail(); void doubleClickingADraftOpensTheComposer(); void aResumedDraftReplacesItsFileRatherThanAddingOne(); void aResumedDraftKeepsItsBlindRecipients(); @@ -13817,6 +13818,127 @@ void TestMainWindow::editDraftIsOfferedOnlyForADraft() "Edit draft is not offered on a message in the drafts folder"); } +void TestMainWindow::theMessageBarSwapsToTheTrashActionsOnTrashedMail() +{ + // Items 185 and 186. The bar offered Reply and Forward on a message the + // user had thrown away, which are the two things a trashed message is + // least likely to want, while Restore and the two purges lived only in + // menus. Delete moved here from the main toolbar in the same change. + // + // Asserted on WHICH ACTIONS the bar carries, which has a right answer. + // The tint on Restore does not and is not tested: a probe counting + // coloured pixels passes whatever the stylesheet says, for the reasons + // CLAUDE.md records under rendering probes. + WorkerComposeFixture fixture; + QVERIFY(fixture.backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("trashed1@example.org"), + QStringLiteral("Thrown away"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body."))); + QVERIFY2(fixture.seed({ { QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash"), + QStringLiteral("/bin/true"), + QStringLiteral("you@example.org"), + QStringLiteral("Drafts") } }, + QStringLiteral("acct/inbox")), + qPrintable(fixture.backed.error())); + + MainWindow window(fixture.backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *bar = window.findChild<QToolBar *>(QStringLiteral("message_toolbar")); + QVERIFY(model && view && queryEdit && bar); + + const auto selectById = [&](const QString &id) { + queryEdit->setText(QStringLiteral("id:") + id); + queryEdit->returnPressed(); + bool ready = false; + for (int attempt = 0; attempt < 150 && !ready; ++attempt) { + ready = model->rowCount(QModelIndex()) == 1 + && !window.mailRootForTesting().isEmpty(); + if (!ready) + QTest::qWait(100); + } + if (!ready) + return false; + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + // The bar's trash branch reads the same predicate the menus do, and + // for a conversation row that predicate is only right once the digest + // has reported every path. Give the round trip a moment. + QTest::qWait(300); + return true; + }; + + const auto barHolds = [&](const QString &name) { + const auto actions = bar->actions(); + return std::any_of(actions.cbegin(), actions.cend(), + [&](const QAction *action) { + return action && action->objectName() == name + && action->isVisible(); + }); + }; + + // Ordinary mail first, so the trash assertions below mean something: a bar + // that never holds Reply passes the "no Reply in the trash" check by + // accident. This is also item 186's assertion, since Delete is on the bar + // here only because it was moved off the main toolbar. + QVERIFY2(selectById(QStringLiteral("compose1@example.org")), + "the inbox message was not found"); + QVERIFY2(barHolds(QStringLiteral("reply")), + "the message bar lost Reply on ordinary mail"); + QVERIFY2(barHolds(QStringLiteral("forward")), + "the message bar lost Forward on ordinary mail"); + QVERIFY2(barHolds(QStringLiteral("delete")), + "Delete did not arrive on the message bar (item 186)"); + QVERIFY2(!barHolds(QStringLiteral("restore")), + "Restore is offered on mail that was never deleted"); + QVERIFY2(!barHolds(QStringLiteral("purge")), + "Delete permanently is offered outside the trash"); + + // The main toolbar must have LOST it, or item 186 moved nothing and the + // action simply appears twice. + auto *mainBar = window.findChild<QToolBar *>(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"); + }), + "Delete is still on the main toolbar as well as the message bar"); + + // And the trash, which is the whole point. + QVERIFY2(selectById(QStringLiteral("trashed1@example.org")), + "the trashed message was not found"); + QVERIFY2(barHolds(QStringLiteral("restore")), + "Restore is missing from the bar on a trashed message"); + QVERIFY2(barHolds(QStringLiteral("purge")), + "Delete permanently is missing from the bar in the trash"); + QVERIFY2(barHolds(QStringLiteral("empty_trash")), + "Empty trash is missing from the bar in the trash"); + QVERIFY2(!barHolds(QStringLiteral("reply")), + "Reply is still offered on a trashed message, which is the " + "complaint item 185 exists to fix"); + QVERIFY2(!barHolds(QStringLiteral("forward")), + "Forward is still offered on a trashed message"); + QVERIFY2(barHolds(QStringLiteral("toggle_html")), + "the view controls were lost when the bar swapped to the trash"); + + // And back, because a one-way swap is the plausible defect: the bar is + // refilled on every selection change, so returning to ordinary mail has to + // restore the reply pair rather than leaving the purges behind on mail + // they must not destroy. + QVERIFY2(selectById(QStringLiteral("compose1@example.org")), + "the inbox message was not found on the way back"); + QVERIFY2(barHolds(QStringLiteral("reply")), + "Reply did not come back after leaving the trash"); + QVERIFY2(!barHolds(QStringLiteral("purge")), + "Delete permanently stayed on the bar after leaving the trash"); +} + void TestMainWindow::theMessageBarOffersEditOnADraft() { // Item 157, and the half item 153 did not close. A draft was editable by diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 5d59b44..4daea21 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -603,6 +603,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Non connesso all'indice della posta</translation> </message> <message> + <source>Nothing selected to delete</source> + <translation>Nessun messaggio selezionato da eliminare</translation> + </message> + <message> <source>The trash is already empty</source> <translation>Il cestino è già vuoto</translation> </message> @@ -615,6 +619,13 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Svuota cestino</translation> </message> <message numerus="yes"> + <source>Permanently delete %n selected message(s)?</source> + <translation> + <numerusform>Eliminare definitivamente %n messaggio selezionato?</numerusform> + <numerusform>Eliminare definitivamente %n messaggi selezionati?</numerusform> + </translation> + </message> + <message numerus="yes"> <source>Permanently delete %n message(s) from the trash of %1?</source> <translation> <numerusform>Eliminare definitivamente %n messaggio dal cestino di %1?</numerusform> @@ -850,6 +861,14 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>&Regole di etichettatura...</translation> </message> <message> + <source>Delete per&manently...</source> + <translation>Elimina definitiva&mente...</translation> + </message> + <message> + <source>Permanently delete the selected messages</source> + <translation>Elimina definitivamente i messaggi selezionati</translation> + </message> + <message> <source>Edit the rules that tag mail as it arrives</source> <translation>Modifica le regole che etichettano la posta in arrivo</translation> </message> |