diff options
| -rw-r--r-- | CHANGELOG.md | 6 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 42 | ||||
| -rw-r--r-- | src/composewindow.cpp | 117 | ||||
| -rw-r--r-- | src/composewindow.h | 10 | ||||
| -rw-r--r-- | tests/test_composewindow.cpp | 165 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 36 |
6 files changed, 371 insertions, 5 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 61a93c7..3dcc0a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,12 @@ point at which they are stable. own modified marker for the same state, so it is visible while the composer sits behind another window. Autosaving already worked; it was silent on success, and the only feedback was the banner that reports a FAILED save. +- **A menu bar on the composer**, with File, Edit and Format. **Save draft + (`Ctrl+S`) is new**: autosave, sending and closing were the only things that + could write a draft, so there was no way to ask for one. Everything else was + already there and is now findable: Send and Close under File, the editor's + undo and clipboard actions under Edit, and the formatting buttons, Attach, + Send as HTML and the signature switch under Format. ## [0.27.0] - 2026-08-25 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 41a7a1b..2e941ec 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 @@ -7709,3 +7709,45 @@ because the user looked at it. severity is the established vocabulary here. - `saveDraftNow()` returns false on failure and the banner takes over, so the status line must not claim a save the write did not make. + + + +## 161. The composer has no menu bar + +**Observed (user, 2026-08-25):** "the compose window should have a menu bar on +top", with "save draft (ctrl+s)" and "duplicate the other actions like we do +on the main window". + +**Cause (verified in code):** the composer has no `QMenuBar`, and **Save draft +does not exist as an action at all.** `saveDraftNow()` is reachable only from +the autosave timer, from the send path, and from `closeEvent`; there is no way +for the user to ask for a save, and no `Ctrl+S` anywhere in the composer or in +`KeyMap`. + +The actions the composer does have are ad-hoc `QAction`s parented to the +window: `m_sendAction` and a Close action (`composewindow.cpp:583-608`), plus +the formatting toolbar's own. **None is registered in `KeyMap`**, which is +deliberate and documented for `Ctrl+W` in item 148: they are WindowShortcuts +dispatched to the active composer only, so they never touch the main window's +namespace. A consequence worth stating before this item is built: +`everyActionIsReachableFromAMenu()` walks the MAIN window's menu bar, so it +does not currently constrain these, and adding a composer menu bar does not +automatically bring them under item 132's rules. + +**Approach.** A `QMenuBar` on the composer, with Save draft (`Ctrl+S`) as the +new action and the existing ones gathered under it rather than duplicated. + +**Constraints.** + +- **"Duplicate the other actions" means the main window's MESSAGE actions, and + most of them are meaningless here.** A composer has no thread, no selection + and no tag surface. Ask which the user actually wants before building a menu + that offers Archive or Mark all read over a message being written. +- Keep the composer's actions out of `KeyMap`, per item 148's reasoning. A + composer menu bar is not a reason to move them. +- Save draft must go through `saveDraftNow()`, which already handles the + failure banner and emits `draftSaved` for the indexing item 158 added. A + second write path would reintroduce the ghost-file problem that fixed. +- **Item 160 unblocked this** on 2026-08-25: the status bar exists, so a + manual save reports through `refreshDraftStatus()` like an autosave. Route + `Ctrl+S` through `saveDraftNow()` and the reporting is already done. diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 2b0b9d8..879a9d1 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -42,6 +42,7 @@ #include <QLineEdit> #include <QListWidget> #include <QMenu> +#include <QMenuBar> #include <QMessageBox> #include <QPlainTextEdit> #include <QPushButton> @@ -132,6 +133,9 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, buildUi(); buildFormatToolbar(); + // AFTER buildFormatToolbar(): the menus show its actions, so they must + // exist before a menu can hold them. + buildMenuBar(); seedFields(); seedBody(); seedSignature(); @@ -606,11 +610,114 @@ void ComposeWindow::buildFormatToolbar() // close() rather than anything of its own: closeEvent() already decides // whether the draft is saved or discarded, and a second route out that // skipped it would lose the message. - auto *closeAction = new QAction(tr("Close"), this); - closeAction->setObjectName(QStringLiteral("compose_close")); - closeAction->setShortcut(QKeySequence(QStringLiteral("Ctrl+W"))); - connect(closeAction, &QAction::triggered, this, &ComposeWindow::close); - addAction(closeAction); + m_closeAction = new QAction(tr("Close"), this); + m_closeAction->setObjectName(QStringLiteral("compose_close")); + m_closeAction->setShortcut(QKeySequence(QStringLiteral("Ctrl+W"))); + connect(m_closeAction, &QAction::triggered, this, &ComposeWindow::close); + addAction(m_closeAction); + + // Save draft, the one action item 161 adds rather than gathers. It goes + // through saveDraftNow() like every other write: that is what emits + // draftSaved for item 158's indexing, reports through item 160's status + // bar, and raises the failure banner. A second write path would have to + // repeat all three and would reintroduce the ghost file 158 removed. + m_saveAction = new QAction(tr("Save draft"), this); + m_saveAction->setObjectName(QStringLiteral("compose_save")); + m_saveAction->setShortcut(QKeySequence(QStringLiteral("Ctrl+S"))); + const QIcon saveIcon = QIcon::fromTheme(QStringLiteral("document-save")); + if (!saveIcon.isNull()) + m_saveAction->setIcon(saveIcon); + connect(m_saveAction, &QAction::triggered, this, + [this]() { saveDraftNow(); }); + addAction(m_saveAction); + + // The menu twin of the HTML tool button. m_sendHtml is a QToolButton, so + // it cannot go in a menu; this mirrors it in BOTH directions, since a + // menu entry that only follows the button is half a control. + m_sendHtmlAction = new QAction(tr("Send as HTML"), this); + m_sendHtmlAction->setObjectName(QStringLiteral("compose_send_html")); + m_sendHtmlAction->setCheckable(true); + m_sendHtmlAction->setChecked(m_sendHtml->isChecked()); + connect(m_sendHtml, &QToolButton::toggled, m_sendHtmlAction, + &QAction::setChecked); + connect(m_sendHtmlAction, &QAction::toggled, m_sendHtml, + &QToolButton::setChecked); +} + +void ComposeWindow::buildMenuBar() +{ + // The menus SHOW the toolbar's own QActions rather than owning copies, + // exactly as item 140 required for the message pane's bar: a copy drifts, + // so an enablement change or a new shortcut would reach one surface and + // not the other. + QMenu *file = menuBar()->addMenu(tr("&File")); + file->addAction(m_saveAction); + file->addAction(m_sendAction); + file->addSeparator(); + file->addAction(m_closeAction); + + // The editor's own undo stack, which had no menu presence at all. These + // are QPlainTextEdit's actions, so they follow its state for free: undo + // greys out when there is nothing to undo, and the clipboard entries + // follow the selection. + QMenu *edit = menuBar()->addMenu(tr("&Edit")); + const auto addEdit = [this, edit](const QString &name, const QString &text, + const QKeySequence &shortcut, + void (QPlainTextEdit::*slot)()) { + QAction *action = edit->addAction(text); + action->setObjectName(name); + action->setShortcut(shortcut); + connect(action, &QAction::triggered, m_body, slot); + return action; + }; + QAction *undo = addEdit(QStringLiteral("compose_undo"), tr("Undo"), + QKeySequence::Undo, &QPlainTextEdit::undo); + QAction *redo = addEdit(QStringLiteral("compose_redo"), tr("Redo"), + QKeySequence::Redo, &QPlainTextEdit::redo); + edit->addSeparator(); + QAction *cut = addEdit(QStringLiteral("compose_cut"), tr("Cut"), + QKeySequence::Cut, &QPlainTextEdit::cut); + QAction *copy = addEdit(QStringLiteral("compose_copy"), tr("Copy"), + QKeySequence::Copy, &QPlainTextEdit::copy); + addEdit(QStringLiteral("compose_paste"), tr("Paste"), QKeySequence::Paste, + &QPlainTextEdit::paste); + + // Enablement follows the editor, so a greyed entry tells the truth about + // what pressing it would do. + undo->setEnabled(false); + redo->setEnabled(false); + cut->setEnabled(false); + copy->setEnabled(false); + connect(m_body, &QPlainTextEdit::undoAvailable, undo, &QAction::setEnabled); + connect(m_body, &QPlainTextEdit::redoAvailable, redo, &QAction::setEnabled); + connect(m_body, &QPlainTextEdit::copyAvailable, cut, &QAction::setEnabled); + connect(m_body, &QPlainTextEdit::copyAvailable, copy, &QAction::setEnabled); + + QMenu *format = menuBar()->addMenu(tr("F&ormat")); + const auto byName = [this](const char *name) { + return findChild<QAction *>(QString::fromLatin1(name)); + }; + for (const char *name : { "format_bold", "format_italic", "format_code", + "format_strike" }) { + if (QAction *action = byName(name)) + format->addAction(action); + } + format->addSeparator(); + for (const char *name : { "format_link", "format_quote" }) { + if (QAction *action = byName(name)) + format->addAction(action); + } + format->addSeparator(); + format->addAction(m_attachAction); + format->addAction(m_detachAction); + format->addSeparator(); + format->addAction(m_sendHtmlAction); + + // The switch's own menu, shown a second time. It is rebuilt whenever the + // signatures change, so taking the QMenu itself keeps both in step; a + // copy of its entries would go stale on the next rebuild. + QAction *signature = format->addMenu(m_signatureSwitch->menu()); + signature->setText(tr("Signature")); } void ComposeWindow::seedFields() diff --git a/src/composewindow.h b/src/composewindow.h index dc7f3c0..2eaeeea 100644 --- a/src/composewindow.h +++ b/src/composewindow.h @@ -228,6 +228,10 @@ private: void applyEdit(const MarkdownFormat::Edit &edit); void markDirty(); + /// Builds the menu bar (item 161). Called after buildFormatToolbar(), + /// since the menus SHOW its actions rather than owning copies. + void buildMenuBar(); + /// Builds the status bar carrying the unsaved cue and the age line. void buildDraftStatusBar(); @@ -310,6 +314,12 @@ private: QPlainTextEdit *m_sendLog = nullptr; QToolBar *m_formatToolbar = nullptr; QAction *m_sendAction = nullptr; + QAction *m_saveAction = nullptr; + QAction *m_closeAction = nullptr; + + /// The menu twin of the m_sendHtml tool button, which is a QToolButton + /// and cannot be put in a menu. Kept in step both ways. + QAction *m_sendHtmlAction = nullptr; QAction *m_attachAction = nullptr; QAction *m_detachAction = nullptr; diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 68ec4d4..779d48c 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -24,6 +24,10 @@ #include <QPlainTextEdit> #include <QSignalSpy> #include <QLabel> +#include <QMenuBar> +#include <QToolBar> +#include <QSet> +#include <functional> #include <QRegularExpression> #include <QTemporaryDir> #include <QTimer> @@ -56,6 +60,10 @@ private slots: void aSavedDraftReportsItAndClearsTheCue(); void aSentMessageLeavesNoUnsavedCue(); void onlyTheSetterWritesTheDirtyFlag(); + void theMenuBarReachesEveryComposerAction(); + void saveDraftWritesAndReports(); + void theMenusReuseTheToolbarActions(); + void theHtmlMenuItemTracksTheToolbarButton(); void theAgeLineFollowsTheClock(); private: @@ -643,5 +651,162 @@ void TestComposeWindow::theAgeLineFollowsTheClock() "the age line must move as the clock does"); } +/// Item 132's rule for the main window, applied to the composer: an action +/// nobody can find in a menu is reachable only by a chord the user has to +/// know. Walks the real menu bar rather than a list, so an action added to +/// the toolbar and forgotten in the menus fails here. +void TestComposeWindow::theMenuBarReachesEveryComposerAction() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + + QMenuBar *bar = window.menuBar(); + QVERIFY2(bar, "the composer has no menu bar"); + + // Every action the menus reach, submenus included. + QSet<QString> reachable; + std::function<void(QMenu *)> walk = [&](QMenu *menu) { + const QList<QAction *> actions = menu->actions(); + for (QAction *action : actions) { + if (QMenu *sub = action->menu()) { + // An action owning a submenu is not itself reachable: Qt + // emits no triggered for it, as CLAUDE.md records. + walk(sub); + continue; + } + if (action->isSeparator()) + continue; + if (!action->objectName().isEmpty()) + reachable.insert(action->objectName()); + } + }; + const QList<QAction *> top = bar->actions(); + for (QAction *action : top) { + QVERIFY2(action->menu(), "a top-level menu bar entry with no menu"); + walk(action->menu()); + } + + // Every named action the composer owns. findChildren, so an action added + // later is picked up without touching this list. + const QList<QAction *> owned = window.findChildren<QAction *>(); + QStringList missing; + for (QAction *action : owned) { + const QString name = action->objectName(); + if (name.isEmpty()) + continue; + // The signature entries are built from the files on disk and named + // per signature; the switch itself is what a menu offers. + if (name.startsWith(QStringLiteral("signature_choice"))) + continue; + if (!reachable.contains(name)) + missing.append(name); + } + + QVERIFY2(missing.isEmpty(), + qPrintable(QStringLiteral("not reachable from any menu: %1") + .arg(missing.join(QStringLiteral(", "))))); +} + +/// The action item 161 adds. Save draft did not exist at all: saveDraftNow() +/// was reachable only from the timer, the send path and closeEvent, so the +/// user could not ask for a save. +void TestComposeWindow::saveDraftWritesAndReports() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("Saved by hand.")); + + auto *save = window.findChild<QAction *>(QStringLiteral("compose_save")); + QVERIFY2(save, "there is no Save draft action"); + QCOMPARE(save->shortcut(), QKeySequence(QStringLiteral("Ctrl+S"))); + + QSignalSpy saved(&window, &ComposeWindow::draftSaved); + save->trigger(); + + QCOMPARE(saved.size(), 1); + + // Routed through saveDraftNow(), so item 160's reporting comes free. A + // second write path would have to repeat it, and would be the ghost-file + // bug item 158 fixed. + auto *age = window.findChild<QLabel *>(QStringLiteral("draftAge")); + QVERIFY(age); + QVERIFY2(!age->text().isEmpty(), "a manual save must report like an autosave"); + QVERIFY2(!window.isWindowModified(), "a manual save must clear the marker"); +} + +/// The same QAction objects, shown twice over, exactly as item 140 required +/// for the message pane's bar. A copy would drift: an enablement change or a +/// new shortcut would reach one surface and not the other. +void TestComposeWindow::theMenusReuseTheToolbarActions() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + + auto *bold = window.findChild<QAction *>(QStringLiteral("format_bold")); + auto *toolbar = window.findChild<QToolBar *>(QStringLiteral("formatToolbar")); + QVERIFY(bold); + QVERIFY(toolbar); + QVERIFY2(toolbar->actions().contains(bold), + "Bold left the formatting toolbar"); + + QMenu *format = nullptr; + const QList<QAction *> top = window.menuBar()->actions(); + for (QAction *action : top) { + if (action->menu() && action->menu()->actions().contains(bold)) + format = action->menu(); + } + QVERIFY2(format, "Bold is not in any menu"); + + // The pointer itself, not a namesake. + QVERIFY2(format->actions().contains(bold), + "the menu holds a copy of Bold rather than the action itself"); +} + +/// The HTML toggle is a QToolButton, not a QAction, so a menu entry for it +/// has to be built and kept in step by hand. Both directions: a menu that +/// only follows the button is half a control. +void TestComposeWindow::theHtmlMenuItemTracksTheToolbarButton() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + + auto *button = window.findChild<QToolButton *>(QStringLiteral("sendHtml")); + auto *item = window.findChild<QAction *>(QStringLiteral("compose_send_html")); + QVERIFY(button); + QVERIFY2(item, "there is no menu entry for the HTML toggle"); + QVERIFY(item->isCheckable()); + + const bool initial = button->isChecked(); + QCOMPARE(item->isChecked(), initial); + + button->setChecked(!initial); + QCOMPARE(item->isChecked(), !initial); + + item->setChecked(initial); + QCOMPARE(button->isChecked(), initial); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 5773461..913d151 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -111,6 +111,42 @@ <translation>Invia</translation> </message> <message> + <source>Save draft</source> + <translation>Salva bozza</translation> + </message> + <message> + <source>&File</source> + <translation>&File</translation> + </message> + <message> + <source>&Edit</source> + <translation>&Modifica</translation> + </message> + <message> + <source>Undo</source> + <translation>Annulla</translation> + </message> + <message> + <source>Redo</source> + <translation>Ripeti</translation> + </message> + <message> + <source>Cut</source> + <translation>Taglia</translation> + </message> + <message> + <source>Copy</source> + <translation>Copia</translation> + </message> + <message> + <source>Paste</source> + <translation>Incolla</translation> + </message> + <message> + <source>F&ormat</source> + <translation>F&ormato</translation> + </message> + <message> <source>None</source> <translation>Nessuna</translation> </message> |