From 188287f14f981ed3e8a08bc1514bb2ba6bb76809 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 14:08:59 +0200 Subject: fix: bind shortcuts users can actually press Typing a capital emits Shift+, but QKeySequence::fromString() folds the case of a bare letter away: "N" parsed to plain Key_N, a combination no keystroke produces. The N, F and G defaults (toggle_unread, flag and sync) therefore never fired, and neither would any hand-written capital in [keys]. normalizeSequence() rewrites a bare capital to Shift+ and is shared by the defaults and the override pass. As a side effect "y" and "Y" become distinct keys rather than a collision that dropped one. Defaults move to modifier shortcuts throughout. A single letter cannot be a QAction shortcut without stealing that letter from every text field in the window, and the menus in the next commit need real accelerators. defaultBindings() is now the one source of truth for them. --- src/keymap.cpp | 112 +++++++++++++++++++++++++++++++++++++++++++++------------ src/keymap.h | 27 ++++++++++++++ 2 files changed, 117 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/keymap.cpp b/src/keymap.cpp index 39991dc..e53c2bf 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -41,25 +41,95 @@ QStringList KeyMap::knownActions() }; } -void KeyMap::loadDefaults() +QList> KeyMap::defaultBindings() { - const QHash defaults = { - { QStringLiteral("j"), QStringLiteral("next_thread") }, - { QStringLiteral("k"), QStringLiteral("prev_thread") }, - { QStringLiteral("Return"), QStringLiteral("open_thread") }, - { QStringLiteral("a"), QStringLiteral("archive") }, - { QStringLiteral("d"), QStringLiteral("delete") }, - { QStringLiteral("N"), QStringLiteral("toggle_unread") }, - { QStringLiteral("F"), QStringLiteral("flag") }, - { QStringLiteral("/"), QStringLiteral("focus_query") }, - { QStringLiteral("h"), QStringLiteral("toggle_html") }, - { QStringLiteral("u"), QStringLiteral("undo") }, - { QStringLiteral("G"), QStringLiteral("sync") }, - { QStringLiteral("Ctrl+Q"), QStringLiteral("quit") }, + // Modifier shortcuts throughout, rather than the bare letters of 0.1.0. + // Two reasons. A bare capital never worked: "N" parses to plain Key_N + // while typing a capital emits Shift+N, so toggle_unread, flag and sync + // were dead keys. And a single letter cannot be a QAction shortcut in a + // menu without stealing that letter from every text field in the window. + // + // Ordered as the menus present them; a QList keeps that order, which a + // QHash would not. + return { + { QStringLiteral("Ctrl+J"), QStringLiteral("next_thread") }, + { QStringLiteral("Ctrl+K"), QStringLiteral("prev_thread") }, + { QStringLiteral("Return"), QStringLiteral("open_thread") }, + { QStringLiteral("Ctrl+E"), QStringLiteral("archive") }, + { QStringLiteral("Ctrl+D"), QStringLiteral("delete") }, + { QStringLiteral("Ctrl+Shift+S"), QStringLiteral("spam") }, + { QStringLiteral("Ctrl+U"), QStringLiteral("toggle_unread") }, + { QStringLiteral("Ctrl+I"), QStringLiteral("flag") }, + { QStringLiteral("Ctrl+L"), QStringLiteral("focus_query") }, + { QStringLiteral("Ctrl+H"), QStringLiteral("toggle_html") }, + { QStringLiteral("Ctrl+M"), QStringLiteral("load_remote") }, + { QStringLiteral("Ctrl+Z"), QStringLiteral("undo") }, + { QStringLiteral("Ctrl+G"), QStringLiteral("sync") }, + { QStringLiteral("Ctrl+Q"), QStringLiteral("quit") }, }; +} + +QStringList KeyMap::defaultActions() +{ + QStringList actions; + const auto bindings = defaultBindings(); + actions.reserve(bindings.size()); + for (const auto &binding : bindings) + actions.append(binding.second); + return actions; +} + +QKeySequence KeyMap::normalizeSequence(const QString &text) +{ + const QKeySequence sequence = QKeySequence::fromString(text); + + // fromString() does not return an empty sequence for unparseable input; + // it returns a non-empty one whose toString() is empty (verified on + // Qt 6.11). Both checks are needed to detect garbage. + if (sequence.isEmpty() || sequence.toString().isEmpty()) + return {}; + + // A bare uppercase letter, no modifiers: the user wrote "N" meaning the + // key they press to type a capital N, which is Shift+N. fromString() + // folded the case away, so put the Shift back. + if (text.size() == 1 && text.at(0).isUpper() && text.at(0).isLetter()) + return QKeySequence(sequence[0].key() | Qt::SHIFT); + + return sequence; +} - for (auto it = defaults.cbegin(); it != defaults.cend(); ++it) - m_bindings.insert(QKeySequence::fromString(it.key()), it.value()); +void KeyMap::loadDefaults() +{ + for (const auto &binding : defaultBindings()) + m_bindings.insert(normalizeSequence(binding.first), binding.second); +} + +QKeySequence KeyMap::sequenceFor(const QString &action) const +{ + // Several sequences can point at one action (a default the user did not + // remove, plus their own addition). QHash iteration order is unspecified, + // so pick deterministically rather than taking whichever comes first. + QKeySequence best; + for (auto it = m_bindings.cbegin(); it != m_bindings.cend(); ++it) { + if (it.value() != action) + continue; + const QString candidate = it.key().toString(); + if (best.isEmpty() || candidate.size() < best.toString().size() + || (candidate.size() == best.toString().size() + && candidate < best.toString())) { + best = it.key(); + } + } + return best; +} + +QKeySequence KeyMap::defaultSequenceFor(const QString &action) +{ + for (const auto &binding : defaultBindings()) { + if (binding.second == action) + return normalizeSequence(binding.first); + } + return {}; } void KeyMap::loadOverrides(QSettings &settings) @@ -79,12 +149,10 @@ void KeyMap::loadOverrides(QSettings &settings) for (const QString &key : keys) { const QString action = settings.value(key).toString(); - const QKeySequence sequence = QKeySequence::fromString(key); - // QKeySequence::fromString() does not return an empty sequence for - // unparseable input; it returns a non-empty sequence whose - // toString() is empty (verified on Qt 6.11). Use that to detect - // garbage input instead. - if (sequence.isEmpty() || sequence.toString().isEmpty()) { + // Shares the defaults' normalization, so a hand-written "N" binds the + // key the user actually presses rather than one nothing emits. + const QKeySequence sequence = normalizeSequence(key); + if (sequence.isEmpty()) { m_warnings.append( QStringLiteral("Unparseable key sequence '%1' in [keys]").arg(key)); continue; diff --git a/src/keymap.h b/src/keymap.h index 564eb10..1c7df5f 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -20,6 +20,8 @@ #include #include +#include +#include #include class QSettings; @@ -33,6 +35,11 @@ public: /// anything not in this set, so a typo in the config cannot bind silently. static QStringList knownActions(); + /// The built-in bindings, in menu order: {sequence, action}. The single + /// source of truth for the defaults, so the menus, the shortcut reference + /// and loadDefaults() cannot disagree about them. + static QList> defaultBindings(); + void loadDefaults(); /// Reads the [keys] group. Invalid sequences and unknown action names are @@ -42,6 +49,26 @@ public: /// Empty string when nothing is bound. QString actionFor(const QKeySequence &sequence) const; + /// The sequence currently bound to an action, empty if none. The reverse + /// of actionFor(): menus need a shortcut for an action they already know. + /// When several sequences are bound to one action, returns the shortest + /// text, so the menu shows a stable choice rather than a hash-order one. + QKeySequence sequenceFor(const QString &action) const; + + /// The built-in sequence for an action, ignoring any user override. + static QKeySequence defaultSequenceFor(const QString &action); + + /// Every action name carrying a built-in binding. + static QStringList defaultActions(); + + /// Normalizes a configured key string into the sequence a real keypress + /// produces. QKeySequence::fromString() discards the case of a bare + /// letter, so "N" parses to plain Key_N, which no keystroke ever emits: + /// typing a capital sends Shift+N. A bare uppercase letter is therefore + /// rewritten to Shift+. Returns an empty sequence for input + /// fromString() cannot parse. + static QKeySequence normalizeSequence(const QString &text); + QStringList warnings() const { return m_warnings; } private: -- cgit v1.2.3 From 1f2eddff6afcbf4c24f06e982e9169219429a2ed Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 14:17:09 +0200 Subject: feat: add menus, a toolbar and a shortcut reference Actions were a QHash of std::function dispatched by an event filter, which nothing could put in a menu. They are QActions now, bound from KeyMap so a [keys] override reaches the menus as well as the keyboard. Menu bar covers every action; the toolbar carries only Sync, Archive, Delete and Undo. Help > Keyboard shortcuts is generated from the actions, so it shows what the keys really do rather than a copy that drifts. spam and load_remote gained defaults, having been unreachable without a hand-written binding. The event filter is gone. Probing showed QAction shortcuts are dispatched before the focused widget sees the key, so they beat QAbstractItemView's type-to-search without one, and Qt already suppresses plain-letter shortcuts while an editable widget has focus. Dropping the filter's blanket guard also lets Ctrl+Q work while the query bar has focus. registeredActionNames() is derived from the actions rather than hand-maintained, so the two drift tests it needed are replaced by checks that a configured binding reaches its action. No confirmation dialogs: tag mutations still answer to undo. --- CHANGELOG.md | 32 ++- README.md | 55 +++-- .../plans/2026-08-03-post-0.1.0-usability.md | 35 ++- src/keymap.cpp | 30 ++- src/mainwindow.cpp | 260 ++++++++++++++------- src/mainwindow.h | 31 ++- tests/test_keymap.cpp | 33 +++ tests/test_mainwindow.cpp | 93 +++++++- 8 files changed, 439 insertions(+), 130 deletions(-) (limited to 'src') diff --git a/CHANGELOG.md b/CHANGELOG.md index 90853ce..ce2b744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,37 @@ point at which they are stable. ## [Unreleased] -Nothing yet. +### Added + +- Menu bar covering every action: File, Edit, Message, View and Help. +- Toolbar with the frequent subset, Sync, Archive, Delete and Undo. +- **Help > Keyboard shortcuts**, listing the current bindings. Generated from + the actions themselves, so it shows configured overrides rather than a + hand-written copy of the defaults. +- **Help > About**. +- Default bindings for `spam` and `load_remote`, which previously had none + and were unreachable until bound by hand. + +### Fixed + +- Three default bindings never fired. Typing a capital sends `Shift`+the key, + but `N`, `F` and `G` were stored as the unshifted key, which no keystroke + produces, leaving `toggle_unread`, `flag` and `sync` dead. A bare capital in + `[keys]` is now read as `Shift`+that letter. As a side effect `y` and `Y` + are two distinct keys rather than a collision that silently dropped one. +- Modifier shortcuts such as `Ctrl+Q` now work while the query bar has focus. + The old event filter suppressed every binding there, not only the plain + letters that would have interfered with typing. + +### Changed + +- Default bindings moved to modifier shortcuts (`Ctrl+E` archive, `Ctrl+D` + delete, and so on). Existing `[keys]` entries are unaffected, and single + letters are still safe to bind. See "Upgrading from 0.1.0" in the README. +- Actions are `QAction`s dispatched by shortcut rather than a hash of + callbacks behind an event filter, which is what lets them appear in menus. + The hand-maintained list of registered action names is now derived from the + actions, so it can no longer drift from them. ## [0.1.0] - 2026-08-03 diff --git a/README.md b/README.md index 4acd215..f7e2444 100644 --- a/README.md +++ b/README.md @@ -104,18 +104,10 @@ Unread = tag:unread Flagged = tag:flagged [keys] +Ctrl+E = archive +Ctrl+D = delete j = next_thread k = prev_thread -Return = open_thread -a = archive -d = delete -N = toggle_unread -F = flag -/ = focus_query -h = toggle_html -u = undo -G = sync -Ctrl+Q = quit ``` Saved-query buttons appear in alphabetical order rather than file order: @@ -128,27 +120,42 @@ Defaults, all rebindable through `[keys]`: | Key | Action | Does | |---|---|---| -| `j` | `next_thread` | Select the next thread | -| `k` | `prev_thread` | Select the previous thread | +| `Ctrl+J` | `next_thread` | Select the next thread | +| `Ctrl+K` | `prev_thread` | Select the previous thread | | `Return` | `open_thread` | Focus the thread list | -| `a` | `archive` | Remove `inbox` from every selected thread | -| `d` | `delete` | Add `deleted` | -| `N` | `toggle_unread` | Toggle `unread` | -| `F` | `flag` | Add `flagged` | -| `/` | `focus_query` | Focus and select the query bar | -| `h` | `toggle_html` | Switch the thread between HTML and plain text | -| `u` | `undo` | Undo the last tag change | -| `G` | `sync` | Run the configured sync command | +| `Ctrl+E` | `archive` | Remove `inbox` from every selected thread | +| `Ctrl+D` | `delete` | Add `deleted` | +| `Ctrl+Shift+S` | `spam` | Add `spam`, remove `inbox` | +| `Ctrl+U` | `toggle_unread` | Toggle `unread` | +| `Ctrl+I` | `flag` | Add `flagged` | +| `Ctrl+L` | `focus_query` | Focus and select the query bar | +| `Ctrl+H` | `toggle_html` | Switch the thread between HTML and plain text | +| `Ctrl+M` | `load_remote` | Load remote images for the current thread | +| `Ctrl+Z` | `undo` | Undo the last tag change | +| `Ctrl+G` | `sync` | Run the configured sync command | | `Ctrl+Q` | `quit` | Quit | -Two further actions exist but have **no default binding**, so they are -unreachable until you bind them: `spam` (adds `spam`, removes `inbox`) and -`load_remote` (the keyboard equivalent of the "Load remote content" -button). +Every action now carries a default binding, and every one appears in a menu. +**Help > Keyboard shortcuts** lists the current bindings, generated from the +actions themselves, so it shows your overrides rather than these defaults. An unknown action name in `[keys]` produces a warning at startup rather than binding silently, so a typo is visible. +### Upgrading from 0.1.0 + +0.1.0 used single letters (`j`, `k`, `a`, `d`, `N`, `F`, `h`, `u`, `G`, `/`). +Those still work if you keep them in `[keys]`, and single letters remain safe +to bind: Qt suppresses a plain-letter shortcut while the query bar has focus, +so typing a query is unaffected. + +Three of the old defaults never actually fired. Typing a capital sends +`Shift`+the key, but `N`, `F` and `G` were stored as the unshifted key, a +combination no keystroke produces, so `toggle_unread`, `flag` and `sync` were +dead. A bare capital in `[keys]` is now read as `Shift`+that letter, which is +what you press, so those bindings work whether you keep the old names or move +to the new defaults. Note this makes `y` and `Y` two different keys. + Tag actions apply to **every selected thread**, not only the focused one. ## Security posture of the message view 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 8847b9b..99e26e0 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 @@ -36,13 +36,13 @@ taking that too literally. |---|------|---------|------|--------| | 1 | Splitter/column widths do not survive restart | persistence | S | open | | 2 | No way to see full message details (From/To/Cc/Subject) | information | M | open | -| 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | open | +| 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | **done** | | 4 | Message-pane font size does not survive restart | persistence | S | open | | 5 | Thread list is cramped, poor readability | presentation | S | open | | 6 | Opened message stays unread | behavior | S | open | | 7 | HTML view should be default for HTML messages | behavior | XS | **verify first, may already be done** | -| 8 | No buttons or menu entries for archive, undo, etc | discoverability | M | open | -| 9 | No in-app view of configured shortcuts | discoverability | S | open | +| 8 | No buttons or menu entries for archive, undo, etc | discoverability | M | **done** | +| 9 | No in-app view of configured shortcuts | discoverability | S | **done** | | 10 | Reaching an account's inbox takes two steps | workflow | S | open | | 11 | Icon, `.desktop` file, SlackBuild | packaging | M | open | @@ -167,6 +167,35 @@ smuggle in a "Are you sure?" for Delete. **Verification:** the existing keymap test must still pass unchanged, proving user bindings survive the conversion. That is the load-bearing check here. +### Outcome (done) + +Built as described: menu bar, toolbar, and a generated shortcut reference. +Four things the plan did not anticipate, all verified by probe rather than +assumed: + +- **The event filter was removable, but not for the stated reason.** The plan + worried that `QAction` shortcuts might lose to `QAbstractItemView`'s + type-to-search. They do not: shortcut dispatch runs before the focused + widget sees the key. The filter is gone, and the thread view no longer + needs its own. +- **Qt already solves the query-bar case.** A plain-letter shortcut is + suppressed while an editable widget has focus, so the `hasFocus()` guard + was unnecessary. Removing it also fixed `Ctrl+Q`, which the old filter + swallowed while typing a query. +- **Three default bindings had never worked.** `N`, `F` and `G` stored the + unshifted key, which no keystroke emits, so `toggle_unread`, `flag` and + `sync` were dead in 0.1.0. Fixed in `KeyMap::normalizeSequence()` and + committed separately from the menu work. +- **The drift test did become unnecessary**, as the plan hoped. + `registeredActionNames()` is now derived from the `QAction`s, and + `defaultBindings()` is the single source for the defaults. The two tests + that pinned the hand-maintained lists together were replaced by ones that + check a configured binding actually reaches its action. + +Defaults moved to modifier shortcuts, since a single letter cannot be a menu +accelerator without claiming that letter window-wide. Existing `[keys]` +entries are unaffected. + ## 4. Message-pane font size does not survive restart **Observed:** described as "very annoying", more so than item 1. diff --git a/src/keymap.cpp b/src/keymap.cpp index e53c2bf..42ccd40 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -106,17 +106,33 @@ void KeyMap::loadDefaults() QKeySequence KeyMap::sequenceFor(const QString &action) const { - // Several sequences can point at one action (a default the user did not - // remove, plus their own addition). QHash iteration order is unspecified, - // so pick deterministically rather than taking whichever comes first. + // Several sequences can reach one action: the built-in default, which + // loadOverrides() does not remove, plus whatever the user added. Their + // binding is the one to show and to put on the QAction, or configuring + // "Ctrl+Alt+A = archive" would leave the menu still advertising Ctrl+E. + // + // QHash iteration order is unspecified, so ties are broken on the text + // rather than left to chance. + const QKeySequence builtIn = defaultSequenceFor(action); QKeySequence best; + bool bestIsBuiltIn = false; + for (auto it = m_bindings.cbegin(); it != m_bindings.cend(); ++it) { if (it.value() != action) continue; - const QString candidate = it.key().toString(); - if (best.isEmpty() || candidate.size() < best.toString().size() - || (candidate.size() == best.toString().size() - && candidate < best.toString())) { + + const bool isBuiltIn = !builtIn.isEmpty() && it.key() == builtIn; + if (best.isEmpty()) { + best = it.key(); + bestIsBuiltIn = isBuiltIn; + continue; + } + // A user binding always beats the default. + if (bestIsBuiltIn && !isBuiltIn) { + best = it.key(); + bestIsBuiltIn = false; + } else if (bestIsBuiltIn == isBuiltIn + && it.key().toString() < best.toString()) { best = it.key(); } } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 2a484fb..be80d8a 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -18,13 +18,14 @@ #include "mainwindow.h" +#include #include -#include #include #include -#include #include #include +#include +#include #include #include #include @@ -32,6 +33,7 @@ #include #include #include +#include #include #include "mailsync.h" @@ -41,26 +43,13 @@ #include "threadlistmodel.h" #include "version.h" -QStringList MainWindow::registeredActionNames() +QStringList MainWindow::registeredActionNames() const { - // Keep in sync with registerActions(). Held against KeyMap::knownActions() - // by a test rather than by hope. - return { - QStringLiteral("next_thread"), - QStringLiteral("prev_thread"), - QStringLiteral("open_thread"), - QStringLiteral("archive"), - QStringLiteral("delete"), - QStringLiteral("spam"), - QStringLiteral("toggle_unread"), - QStringLiteral("flag"), - QStringLiteral("focus_query"), - QStringLiteral("toggle_html"), - QStringLiteral("load_remote"), - QStringLiteral("undo"), - QStringLiteral("sync"), - QStringLiteral("quit"), - }; + // Derived from the actions themselves, so it cannot drift from what + // registerActions() really installed. + QStringList names = m_actions.keys(); + names.sort(); + return names; } QString MainWindow::cidPrefixForIndex(int index) @@ -86,19 +75,16 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) buildUi(); registerActions(); + buildMenus(); wireWorker(); showWarnings(); - installEventFilter(this); - - // The thread view needs its own filter, not just the window's. A filter on - // the window only sees key presses the focused child did not consume, and - // QAbstractItemView consumes plain letters for its type-to-search feature: - // with the list focused, 'h' jumped to the next thread whose subject began - // with "h" instead of toggling HTML, and every other single-letter binding - // (j, k, a, d, N, F, u, G) was swallowed the same way. Filtering the view - // itself puts the keymap ahead of that search. - m_threadView->installEventFilter(this); + // No event filter: QAction shortcuts are dispatched before the focused + // widget sees the key, so they beat QAbstractItemView's type-to-search + // without one. Qt also suppresses a plain-letter shortcut while an + // editable widget has focus, so typing in the query bar stays typing; + // modifier shortcuts such as Ctrl+Q still work there, which the old + // filter blocked. if (!m_config.savedQueries().isEmpty()) { m_queryEdit->setText(m_config.savedQueries().first().query); @@ -206,40 +192,77 @@ void MainWindow::buildUi() setWindowTitle(QStringLiteral("qtmaildir %1").arg(QTMAILDIR_VERSION)); } +QAction *MainWindow::addAction(const QString &name, const QString &text, + const QString &description, + const std::function &handler) +{ + auto *action = new QAction(text, this); + action->setObjectName(name); + action->setStatusTip(description); + m_actionDescriptions.insert(name, description); + + // The binding comes from KeyMap, so a [keys] override reaches the menus + // and the shortcut reference as well as the keyboard. + const QKeySequence sequence = m_keyMap.sequenceFor(name); + if (!sequence.isEmpty()) + action->setShortcut(sequence); + + // Shortcuts must work while focus is in the thread list or the message + // view, not only on the window itself. + action->setShortcutContext(Qt::WindowShortcut); + + connect(action, &QAction::triggered, this, handler); + + // Added to the window so the shortcut is live even before the action is + // put in a menu; the ones that never reach a menu depend on this. + QMainWindow::addAction(action); + m_actions.insert(name, action); + return action; +} + void MainWindow::registerActions() { - m_actions[QStringLiteral("focus_query")] = [this]() { + addAction(QStringLiteral("focus_query"), tr("&Find"), + tr("Focus and select the query bar"), [this]() { m_queryEdit->setFocus(); m_queryEdit->selectAll(); - }; - m_actions[QStringLiteral("next_thread")] = [this]() { + }); + addAction(QStringLiteral("next_thread"), tr("&Next thread"), + tr("Select the next thread"), [this]() { const QModelIndex current = m_threadView->currentIndex(); const int row = current.isValid() ? current.row() + 1 : 0; if (row < m_model->rowCount()) m_threadView->selectRow(row); - }; - m_actions[QStringLiteral("prev_thread")] = [this]() { + }); + addAction(QStringLiteral("prev_thread"), tr("&Previous thread"), + tr("Select the previous thread"), [this]() { const QModelIndex current = m_threadView->currentIndex(); if (current.isValid() && current.row() > 0) m_threadView->selectRow(current.row() - 1); - }; - m_actions[QStringLiteral("open_thread")] = [this]() { + }); + addAction(QStringLiteral("open_thread"), tr("&Open thread"), + tr("Focus the thread list"), [this]() { m_threadView->setFocus(); - }; - m_actions[QStringLiteral("archive")] = [this]() { + }); + addAction(QStringLiteral("archive"), tr("&Archive"), + tr("Remove inbox from every selected thread"), [this]() { tagSelected({}, { QStringLiteral("inbox") }, tr("Archive")); - }; - m_actions[QStringLiteral("delete")] = [this]() { + }); + addAction(QStringLiteral("delete"), tr("&Delete"), + tr("Add the deleted tag"), [this]() { tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete")); - }; - m_actions[QStringLiteral("spam")] = [this]() { + }); + addAction(QStringLiteral("spam"), tr("Mark &spam"), + tr("Add spam and remove inbox"), [this]() { tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, tr("Mark spam")); - }; - m_actions[QStringLiteral("flag")] = [this]() { + }); + addAction(QStringLiteral("flag"), tr("&Flag"), + tr("Add the flagged tag"), [this]() { tagSelected({ QStringLiteral("flagged") }, {}, tr("Flag")); - }; - m_actions[QStringLiteral("toggle_unread")] = [this]() { + }); + addAction(QStringLiteral("toggle_unread"), tr("Toggle &unread"), + tr("Toggle the unread tag"), [this]() { // The direction comes from the current row, but the change applies to // the whole selection, so a mixed selection lands in one consistent // state rather than each row flipping its own way. @@ -251,28 +274,123 @@ void MainWindow::registerActions() tagSelected({}, { QStringLiteral("unread") }, tr("Mark read")); else tagSelected({ QStringLiteral("unread") }, {}, tr("Mark unread")); - }; - m_actions[QStringLiteral("toggle_html")] = [this]() { + }); + addAction(QStringLiteral("toggle_html"), tr("Toggle &HTML"), + tr("Switch the thread between HTML and plain text"), [this]() { m_messageView->toggleHtml(); - }; - m_actions[QStringLiteral("load_remote")] = [this]() { + }); + addAction(QStringLiteral("load_remote"), tr("Load &remote content"), + tr("Load remote images for the current thread"), [this]() { m_messageView->loadRemoteContent(); - }; - m_actions[QStringLiteral("undo")] = [this]() { + }); + addAction(QStringLiteral("undo"), tr("&Undo"), + tr("Undo the last tag change"), [this]() { if (m_undoStack.canUndo()) m_undoStack.undo(); else m_statusLabel->setText(tr("Nothing to undo")); - }; - m_actions[QStringLiteral("sync")] = [this]() { + }); + addAction(QStringLiteral("sync"), tr("&Sync"), + tr("Run the configured sync command"), [this]() { if (m_sync->isAvailable()) m_sync->start(); - }; - m_actions[QStringLiteral("quit")] = [this]() { close(); }; + }); + addAction(QStringLiteral("quit"), tr("&Quit"), + tr("Quit qtmaildir"), [this]() { close(); }); - // The two lists are maintained by hand and a test pins them together; this - // catches the same drift in a debug run. - Q_ASSERT(m_actions.size() == registeredActionNames().size()); + // A binding the user wrote for an action that does not exist would be + // silently dead. KeyMap warns about unknown names, but only a check here + // catches the reverse: a known action nothing implements. + Q_ASSERT(m_actions.size() == KeyMap::knownActions().size()); +} + +void MainWindow::buildMenus() +{ + auto *fileMenu = menuBar()->addMenu(tr("&File")); + fileMenu->addAction(m_actions.value(QStringLiteral("sync"))); + fileMenu->addSeparator(); + fileMenu->addAction(m_actions.value(QStringLiteral("quit"))); + + auto *editMenu = menuBar()->addMenu(tr("&Edit")); + editMenu->addAction(m_actions.value(QStringLiteral("undo"))); + editMenu->addSeparator(); + editMenu->addAction(m_actions.value(QStringLiteral("focus_query"))); + + auto *messageMenu = menuBar()->addMenu(tr("&Message")); + messageMenu->addAction(m_actions.value(QStringLiteral("archive"))); + messageMenu->addAction(m_actions.value(QStringLiteral("delete"))); + messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); + messageMenu->addSeparator(); + messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); + messageMenu->addAction(m_actions.value(QStringLiteral("flag"))); + + auto *viewMenu = menuBar()->addMenu(tr("&View")); + viewMenu->addAction(m_actions.value(QStringLiteral("prev_thread"))); + viewMenu->addAction(m_actions.value(QStringLiteral("next_thread"))); + viewMenu->addSeparator(); + viewMenu->addAction(m_actions.value(QStringLiteral("toggle_html"))); + viewMenu->addAction(m_actions.value(QStringLiteral("load_remote"))); + + auto *helpMenu = menuBar()->addMenu(tr("&Help")); + auto *shortcuts = helpMenu->addAction(tr("&Keyboard shortcuts")); + connect(shortcuts, &QAction::triggered, + this, &MainWindow::showShortcutReference); + auto *about = helpMenu->addAction(tr("&About")); + connect(about, &QAction::triggered, this, &MainWindow::showAbout); + + // The frequent subset only. A toolbar holding every action is as + // unreadable as no toolbar. + auto *toolBar = addToolBar(tr("Main")); + toolBar->setObjectName(QStringLiteral("main_toolbar")); + toolBar->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); + toolBar->addAction(m_actions.value(QStringLiteral("sync"))); + toolBar->addSeparator(); + toolBar->addAction(m_actions.value(QStringLiteral("archive"))); + toolBar->addAction(m_actions.value(QStringLiteral("delete"))); + toolBar->addSeparator(); + toolBar->addAction(m_actions.value(QStringLiteral("undo"))); +} + +void MainWindow::showShortcutReference() +{ + // Generated from the actions, so it cannot disagree with what the keys + // really do. A hand-written list would drift the first time a binding + // changed. + QStringList rows; + for (const QString &name : registeredActionNames()) { + const QAction *action = m_actions.value(name); + if (!action) + continue; + const QString sequence = action->shortcut().toString(QKeySequence::NativeText); + rows.append(QStringLiteral("%1%2" + "%3") + .arg(sequence.isEmpty() ? tr("(unbound)") : sequence.toHtmlEscaped(), + m_actionDescriptions.value(name).toHtmlEscaped(), + name.toHtmlEscaped())); + } + + QMessageBox box(this); + box.setWindowTitle(tr("Keyboard shortcuts")); + box.setTextFormat(Qt::RichText); + box.setText(tr("

Keyboard shortcuts

" + "" + "" + "%1
KeyDoesAction name
" + "

Rebind any of these in the [keys] section of " + "qtmaildir.conf, using the action name.

") + .arg(rows.join(QString()))); + box.exec(); +} + +void MainWindow::showAbout() +{ + QMessageBox::about( + this, tr("About qtmaildir"), + tr("

qtmaildir %1

" + "

A Qt6 mail client for notmuch-indexed Maildirs.

" + "

Reads and organizes local mail. Fetching and sending are " + "handled by external scripts.

") + .arg(QStringLiteral(QTMAILDIR_VERSION))); } void MainWindow::wireWorker() @@ -511,23 +629,3 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds, Q_ARG(QString, description)); } -bool MainWindow::eventFilter(QObject *watched, QEvent *event) -{ - if (event->type() != QEvent::KeyPress) - return QMainWindow::eventFilter(watched, event); - - // The query bar must receive ordinary typing, so single-key bindings are - // suppressed while it has focus. - if (m_queryEdit->hasFocus()) - return QMainWindow::eventFilter(watched, event); - - auto *keyEvent = static_cast(event); - const QKeySequence sequence(keyEvent->keyCombination()); - - const QString action = m_keyMap.actionFor(sequence); - if (action.isEmpty() || !m_actions.contains(action)) - return QMainWindow::eventFilter(watched, event); - - m_actions.value(action)(); - return true; -} diff --git a/src/mainwindow.h b/src/mainwindow.h index 30679bb..30a128a 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -30,6 +30,7 @@ #include "keymap.h" #include "types.h" +class QAction; class QLineEdit; class QTableView; class QLabel; @@ -49,10 +50,10 @@ public: explicit MainWindow(const Config &config, QWidget *parent = nullptr); ~MainWindow() override; - /// Every action name registerActions() installs. Exposed so a test can hold - /// it against KeyMap::knownActions(): the two lists are maintained by hand, - /// and a drift either way silently breaks a user's key binding. - static QStringList registeredActionNames(); + /// Every action name registerActions() installs. Derived from the actions + /// themselves rather than hand-maintained, so it cannot drift from what is + /// really registered. + QStringList registeredActionNames() const; /// The cid: namespace prefix for the nth message of a thread. /// @@ -61,9 +62,6 @@ public: /// cid: references from resolving to another's. static QString cidPrefixForIndex(int index); -protected: - bool eventFilter(QObject *watched, QEvent *event) override; - private slots: void runCurrentQuery(); void onThreadsReady(const QVector &threads, quint64 generation); @@ -76,8 +74,17 @@ private slots: private: void buildUi(); void registerActions(); + void buildMenus(); void wireWorker(); void showWarnings(); + void showShortcutReference(); + void showAbout(); + + /// Creates a QAction, binds it to the sequence KeyMap holds for `name`, + /// and registers it. `name` is the action name used in [keys]. + QAction *addAction(const QString &name, const QString &text, + const QString &description, + const std::function &handler); void tagSelected(const QStringList &add, const QStringList &remove, const QString &description); @@ -112,7 +119,15 @@ private: QLabel *m_statusLabel = nullptr; QPlainTextEdit *m_syncLog = nullptr; - QHash> m_actions; + /// Action name (as used in [keys]) to the QAction implementing it. Owned + /// by the window through the QObject parent, not by this hash. + QHash m_actions; + + /// One-line description per action, for the shortcut reference. Kept + /// beside the actions so the dialog is generated, never hand-written in + /// parallel with them. + QHash m_actionDescriptions; + quint64 m_generation = 0; QString m_lastQuery; QString m_currentThreadId; diff --git a/tests/test_keymap.cpp b/tests/test_keymap.cpp index e63c151..c81eeb0 100644 --- a/tests/test_keymap.cpp +++ b/tests/test_keymap.cpp @@ -33,6 +33,7 @@ private slots: void invalidSequenceIsReported(); void collidingOverridesAreReported(); void bareCapitalMatchesShiftedPress(); + void userBindingWinsOverDefaultInMenus(); void defaultsDoNotCollide(); void everyDefaultIsAKnownAction(); }; @@ -100,6 +101,38 @@ void TestKeyMap::bareCapitalMatchesShiftedPress() QVERIFY(caseMap.warnings().isEmpty()); } +void TestKeyMap::userBindingWinsOverDefaultInMenus() +{ + // loadOverrides() adds a binding without removing the default, so two + // sequences reach 'archive'. sequenceFor() is what the menus and the + // shortcut reference display: it must show the user's, not the built-in + // one they were trying to replace. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("Ctrl+Alt+A"), QStringLiteral("archive")); + s.endGroup(); + } + + KeyMap map; + map.loadDefaults(); + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + QCOMPARE(map.sequenceFor(QStringLiteral("archive")), + QKeySequence(QStringLiteral("Ctrl+Alt+A"))); + + // The default still fires; it is only no longer the advertised one. + QCOMPARE(map.actionFor(KeyMap::defaultSequenceFor(QStringLiteral("archive"))), + QStringLiteral("archive")); + + // An action the user left alone still shows its default. + QCOMPARE(map.sequenceFor(QStringLiteral("delete")), + KeyMap::defaultSequenceFor(QStringLiteral("delete"))); +} + void TestKeyMap::defaultsDoNotCollide() { // Two defaults on one sequence means one of them is unreachable, and the diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 57eb763..6bfa925 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -18,18 +18,27 @@ #include +#include +#include +#include +#include + +#include "config.h" #include "keymap.h" #include "mainwindow.h" -/// MainWindow is mostly wiring and needs a live QApplication plus a real -/// database, so it is verified manually in Task 13. Two things do not need -/// either, and both are the kind of drift a comment alone does not prevent. +/// MainWindow is mostly wiring, and the parts that need a real database are +/// still verified manually. What is checked here is the action registry: the +/// bindings a user configures reach the QActions the menus and the keyboard +/// both read from, and no action is left unreachable. class TestMainWindow : public QObject { Q_OBJECT private slots: void everyKnownActionIsRegistered(); void everyRegisteredActionIsKnown(); + void everyActionHasAShortcut(); + void configuredBindingReachesTheAction(); void cidPrefixesAreBangFree(); void cidPrefixesAreDistinctPerMessage(); }; @@ -39,8 +48,14 @@ void TestMainWindow::everyKnownActionIsRegistered() // KeyMap::knownActions() is what loadOverrides() validates config bindings // against. An action listed there but never registered means a user can // bind a key in qtmaildir.conf, get no warning, and have it do nothing. + // + // registeredActionNames() is now derived from the QActions themselves, so + // this compares against what the window really installed. + const Config config; + MainWindow window(config); + const QStringList known = KeyMap::knownActions(); - const QStringList registered = MainWindow::registeredActionNames(); + const QStringList registered = window.registeredActionNames(); for (const QString &action : known) { QVERIFY2(registered.contains(action), @@ -53,8 +68,11 @@ void TestMainWindow::everyRegisteredActionIsKnown() { // The reverse drift: an action MainWindow implements but KeyMap rejects. // The user would get "unknown action" for a binding that is really there. + const Config config; + MainWindow window(config); + const QStringList known = KeyMap::knownActions(); - const QStringList registered = MainWindow::registeredActionNames(); + const QStringList registered = window.registeredActionNames(); for (const QString &action : registered) { QVERIFY2(known.contains(action), @@ -63,6 +81,57 @@ void TestMainWindow::everyRegisteredActionIsKnown() } } +void TestMainWindow::everyActionHasAShortcut() +{ + // An action with no binding is unreachable from the keyboard. Every one + // of them carries a default, so an empty shortcut means the default table + // and the action list have drifted apart. + const Config config; + MainWindow window(config); + + for (const QString &name : window.registeredActionNames()) { + const QAction *action = window.findChild(name); + QVERIFY2(action, qPrintable(QStringLiteral("no QAction named '%1'").arg(name))); + QVERIFY2(!action->shortcut().isEmpty(), + qPrintable(QStringLiteral("action '%1' has no shortcut").arg(name))); + } +} + +void TestMainWindow::configuredBindingReachesTheAction() +{ + // The whole point of [keys]: a user's override must end up on the QAction, + // which is what both the keyboard and the menus read. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("Ctrl+Alt+A"), QStringLiteral("archive")); + s.endGroup(); + } + + // MainWindow reads its keymap from Config::defaultPath(), so point that + // at the temporary file for this test. + const QString previous = qEnvironmentVariable("XDG_CONFIG_HOME"); + QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); + QVERIFY(QFile::copy(path, dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")))); + qputenv("XDG_CONFIG_HOME", dir.path().toUtf8()); + + { + const Config config; + MainWindow window(config); + const QAction *archive = + window.findChild(QStringLiteral("archive")); + QVERIFY(archive); + QCOMPARE(archive->shortcut(), QKeySequence(QStringLiteral("Ctrl+Alt+A"))); + } + + if (previous.isEmpty()) + qunsetenv("XDG_CONFIG_HOME"); + else + qputenv("XDG_CONFIG_HOME", previous.toUtf8()); +} + void TestMainWindow::cidPrefixesAreBangFree() { // MainWindow is the only producer of cidPrefix in the application. The @@ -89,5 +158,17 @@ void TestMainWindow::cidPrefixesAreDistinctPerMessage() } } -QTEST_MAIN(TestMainWindow) +// Constructing a MainWindow needs a QApplication and a platform plugin. The +// test has no display under ctest, so it runs offscreen unless the caller +// asked for something else. +int main(int argc, char *argv[]) +{ + qputenv("QT_QPA_PLATFORM", qgetenv("QT_QPA_PLATFORM").isEmpty() + ? QByteArray("offscreen") + : qgetenv("QT_QPA_PLATFORM")); + QApplication app(argc, argv); + TestMainWindow test; + return QTest::qExec(&test, argc, argv); +} + #include "test_mainwindow.moc" -- cgit v1.2.3 From 95d0d19a4c1323a4839c1544f3bc1c0323f85dd9 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 14:29:06 +0200 Subject: fix: keep the shortcut reference inside the screen Fourteen actions in one table made a dialog taller than the display, which pushed its own title bar off the top. The rows are split into two columns of seven, with the closing note spanning both. QMessageBox is replaced by a plain QDialog. The message box wraps its text at a narrow fixed width, which broke every description into a column of single words and was most of the height: 719x1084 before, 1426x366 after. --- src/mainwindow.cpp | 57 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index be80d8a..eaf98b8 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -20,6 +20,8 @@ #include #include +#include +#include #include #include #include @@ -362,24 +364,55 @@ void MainWindow::showShortcutReference() if (!action) continue; const QString sequence = action->shortcut().toString(QKeySequence::NativeText); - rows.append(QStringLiteral("%1%2" + rows.append(QStringLiteral("%1  " + "%2  " "%3") .arg(sequence.isEmpty() ? tr("(unbound)") : sequence.toHtmlEscaped(), m_actionDescriptions.value(name).toHtmlEscaped(), name.toHtmlEscaped())); } - QMessageBox box(this); - box.setWindowTitle(tr("Keyboard shortcuts")); - box.setTextFormat(Qt::RichText); - box.setText(tr("

Keyboard shortcuts

" - "" - "" - "%1
KeyDoesAction name
" - "

Rebind any of these in the [keys] section of " - "qtmaildir.conf, using the action name.

") - .arg(rows.join(QString()))); - box.exec(); + // Two columns rather than one. Fourteen actions in a single table made a + // dialog taller than the screen, which cut off its own title bar. + const int half = (rows.size() + 1) / 2; + const QString header = + tr("KeyDoes" + "Action name"); + const QString left = header + rows.mid(0, half).join(QString()); + const QString right = header + rows.mid(half).join(QString()); + + // A QDialog rather than QMessageBox: the message box wraps its text at a + // narrow default width, which turned every description into a column of + // single words and made the dialog taller than the screen. + QDialog dialog(this); + dialog.setWindowTitle(tr("Keyboard shortcuts")); + + auto *label = new QLabel(&dialog); + label->setTextFormat(Qt::RichText); + label->setText(tr("" + "" + "" + "" + "
%1
%2
") + .arg(left, right)); + + auto *note = new QLabel( + tr("Rebind any of these in the [keys] section of " + "qtmaildir.conf, using the action name."), + &dialog); + note->setTextFormat(Qt::RichText); + note->setWordWrap(true); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok, &dialog); + connect(buttons, &QDialogButtonBox::accepted, &dialog, &QDialog::accept); + + auto *layout = new QVBoxLayout(&dialog); + layout->addWidget(label); + layout->addWidget(note); + layout->addStretch(); + layout->addWidget(buttons); + + dialog.exec(); } void MainWindow::showAbout() -- cgit v1.2.3 From 1371a3901857a8d2ac4fcf235fb29e7caaf63cae Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 14:57:11 +0200 Subject: feat: show that a tag action landed Selecting a thread and hitting Delete changed nothing on screen, so there was no way to tell the action had stuck. The tag was always applied: applyTagChange() emitted dataChanged across the row, and the Tags column did update. But Subject was set to stretch while Tags came after it, so Subject took all free width and pushed Tags out of view. The feedback lived in the one column that could not be seen. Columns are now Tags, Date, From, Subject, with Subject stretching last so nothing can be pushed off the right edge. A thread tagged deleted or spam fills its whole row, muted red or orange with white struck-through text, through the background, foreground and font roles, so no cue depends on one column remaining visible. Strike-through accompanies the fill on purpose: it survives a theme that overrides backgrounds and reads without colour. Bold for unread still composes with it. Archive adds no tag, so an archived row is left unstyled for now. --- CHANGELOG.md | 7 ++ .../plans/2026-08-03-post-0.1.0-usability.md | 38 ++++++++ src/mainwindow.cpp | 8 ++ src/threadlistmodel.cpp | 46 +++++++++- src/threadlistmodel.h | 15 ++- src/types.h | 7 ++ tests/test_threadlistmodel.cpp | 102 +++++++++++++++++++++ 7 files changed, 218 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/CHANGELOG.md b/CHANGELOG.md index ce2b744..e0ff08b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,13 @@ point at which they are stable. ### Fixed +- Acting on a thread now visibly changes its row. A thread tagged `deleted` + or `spam` is filled dark red or orange, in white struck-through text, across + every column. The tag change was already applied, but `Tags` sat after the + stretching `Subject` column and was pushed off-screen, so Delete looked like + it had done nothing. +- Thread list columns reordered to Tags, Date, From, Subject. Subject stretches + and is now last, so no column can be pushed out of view. - Three default bindings never fired. Typing a capital sends `Shift`+the key, but `N`, `F` and `G` were stored as the unshifted key, which no keystroke produces, leaving `toggle_unread`, `flag` and `sync` dead. A bare capital in 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 99e26e0..1e08bb1 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 @@ -45,6 +45,7 @@ taking that too literally. | 9 | No in-app view of configured shortcuts | discoverability | S | **done** | | 10 | Reaching an account's inbox takes two steps | workflow | S | open | | 11 | Icon, `.desktop` file, SlackBuild | packaging | M | open | +| 13 | No visual feedback that an action stuck | feedback | S | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -360,6 +361,43 @@ Packaging, independent of everything above, and can proceed in parallel. --- +## 13. No visual feedback that an action stuck + +**Observed:** selecting a thread and hitting Delete changed nothing on screen. +No way to tell whether the thread was really going to be deleted on the next +sync, which is bad UX for every tag action, not only delete. + +**Cause:** not a missing update. `ThreadListModel::applyTagChange()` already +added the tag and emitted `dataChanged` across the whole row, so the Tags +column did change. But `SubjectColumn` was set to `QHeaderView::Stretch` while +`TagsColumn` came after it, so Subject absorbed all free width and pushed Tags +out of view. The feedback existed in the one column that could not be seen. + +**Approach:** two changes, since the cause was two things. + +- Column order is now Tags, Date, From, Subject. Subject stretches and is + last, so nothing sits to its right to be pushed out. The other three size + to their contents. +- A thread tagged `deleted` or `spam` styles its entire row: muted dark red + (`#8b2c2c`) or orange (`#a85c18`) fill, white text, struck through. Applied + through `Qt::BackgroundRole`, `Qt::ForegroundRole` and `Qt::FontRole` for + every column, so no cue depends on a single column staying visible. + +Strike-through rides along with the fill deliberately: it survives a theme +that overrides background colours, a colourblind reader, and a screenshot. +Bold for unread still composes with it. + +**Decisions:** no status-bar or toast changes, the existing `tagSelected()` +message stays as it is. Archive removes `inbox` and adds nothing, so an +archived thread gets no row styling; whether it should disappear from an inbox +query is deliberately left open rather than guessed at. + +**Verification:** four model tests covering the colours, the strike-through, +that styling spans every column, and that undo restores a plain row. Rendered +and inspected: normal, unread, deleted, spam, and deleted-plus-unread rows. + +--- + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index eaf98b8..915c1ae 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -169,8 +169,16 @@ void MainWindow::buildUi() m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection); m_threadView->verticalHeader()->hide(); m_threadView->horizontalHeader()->setStretchLastSection(false); + // Subject is the last column and takes the leftover width; the three + // fixed-width ones size to their contents. Nothing sits to the right of + // the stretching column, so no column can be pushed out of view. m_threadView->horizontalHeader()->setSectionResizeMode( ThreadListModel::SubjectColumn, QHeaderView::Stretch); + for (int column : { ThreadListModel::TagsColumn, ThreadListModel::DateColumn, + ThreadListModel::AuthorsColumn }) { + m_threadView->horizontalHeader()->setSectionResizeMode( + column, QHeaderView::ResizeToContents); + } connect(m_threadView->selectionModel(), &QItemSelectionModel::currentRowChanged, diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index c129be1..a083145 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -18,8 +18,24 @@ #include "threadlistmodel.h" +#include #include +QColor ThreadListModel::deletedColour() +{ + // Desaturated crimson: legible under white text on a dark theme, and calm + // enough that deleting fifty threads does not repaint the list as a + // warning banner. + return QColor(0x8b, 0x2c, 0x2c); +} + +QColor ThreadListModel::spamColour() +{ + // Distinct hue rather than a lighter red, so spam and deleted are told + // apart by colour and not by shade. + return QColor(0xa8, 0x5c, 0x18); +} + ThreadListModel::ThreadListModel(QObject *parent) : QAbstractTableModel(parent) { @@ -67,10 +83,34 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const } } - if (role == Qt::FontRole && thread.isUnread()) { + // A thread tagged deleted or spam is on its way out, and the user needs to + // see that the moment they act. Every one of these roles applies to the + // whole row: a cue on a single column disappears as soon as that column + // scrolls out of view, which is exactly how the tag change used to go + // unnoticed. + if (thread.isDoomed()) { + if (role == Qt::BackgroundRole) + return QBrush(thread.isDeleted() ? deletedColour() : spamColour()); + if (role == Qt::ForegroundRole) + return QBrush(QColor(Qt::white)); + } + + if (role == Qt::FontRole) { QFont font; - font.setBold(true); - return font; + bool styled = false; + if (thread.isUnread()) { + font.setBold(true); + styled = true; + } + // Struck through as well as filled, so the state survives a + // screenshot, a colourblind reader, and a theme that overrides the + // background. + if (thread.isDoomed()) { + font.setStrikeOut(true); + styled = true; + } + if (styled) + return font; } return {}; diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index eb1a7ff..01fd241 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include "types.h" @@ -29,11 +30,14 @@ class ThreadListModel : public QAbstractTableModel { Q_OBJECT public: + /// Subject stretches to fill the view, so it must come last: anything + /// after it is pushed out of sight. Tags leads, being the column that + /// changes when the user acts on a thread. enum Column { - DateColumn = 0, + TagsColumn = 0, + DateColumn, AuthorsColumn, SubjectColumn, - TagsColumn, ColumnCount, }; @@ -44,6 +48,13 @@ public: ThreadIdRole = Qt::UserRole + 1, }; + /// Row fill for a thread tagged `deleted`, and for one tagged `spam`. + /// Muted rather than saturated: a bulk delete paints every selected row, + /// and a wall of pure red is harder to read than the list it replaces. + /// Exposed so a test names the same colour the model uses. + static QColor deletedColour(); + static QColor spamColour(); + explicit ThreadListModel(QObject *parent = nullptr); int rowCount(const QModelIndex &parent = {}) const override; diff --git a/src/types.h b/src/types.h index 2de6129..e25c3a9 100644 --- a/src/types.h +++ b/src/types.h @@ -35,6 +35,13 @@ struct ThreadSummary bool isUnread() const { return tags.contains(QStringLiteral("unread")); } bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); } + bool isDeleted() const { return tags.contains(QStringLiteral("deleted")); } + bool isSpam() const { return tags.contains(QStringLiteral("spam")); } + + /// True while the thread is tagged for removal. notmuch deletes nothing + /// itself: the tag marks the thread for whatever the user's sync script + /// does next, so the row has to show it is on its way out. + bool isDoomed() const { return isDeleted() || isSpam(); } }; struct MessageRef diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 24ba9e2..98c477c 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -33,6 +33,11 @@ private slots: void reportsSubjectAndAuthors(); void subjectShowsMessageCountOnlyForRealThreads(); void unreadThreadsRenderBold(); + void tagsAreTheFirstColumnAndSubjectTheLast(); + void deletedThreadsAreRedAndStruckThrough(); + void spamThreadsAreOrangeAndStruckThrough(); + void doomedStylingCoversEveryColumn(); + void ordinaryThreadsCarryNoRowColour(); void threadIdIsReachableFromAnIndex(); void invalidIndexesReturnNothing(); void threadAtOutOfRangeIsSafe(); @@ -153,6 +158,103 @@ void TestThreadListModel::unreadThreadsRenderBold() QVERIFY(unreadFont.value().bold()); } +void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast() +{ + // Subject stretches to fill the view, so whatever sits after it is pushed + // off-screen. Tags used to be there, which is why acting on a thread + // looked like it did nothing: the only column that changed was invisible. + QCOMPARE(ThreadListModel::TagsColumn, 0); + QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1); + + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); + QCOMPARE(model.headerData(ThreadListModel::TagsColumn, Qt::Horizontal, + Qt::DisplayRole).toString(), + QStringLiteral("Tags")); + QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal, + Qt::DisplayRole).toString(), + QStringLiteral("Subject")); +} + +void TestThreadListModel::deletedThreadsAreRedAndStruckThrough() +{ + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + const QVariant background = model.data(subject, Qt::BackgroundRole); + QVERIFY(background.isValid()); + QCOMPARE(background.value().color(), ThreadListModel::deletedColour()); + + // White text on the fill, and struck through so the state reads even in a + // screenshot with the colours stripped. + QCOMPARE(model.data(subject, Qt::ForegroundRole).value().color(), + QColor(Qt::white)); + QVERIFY(model.data(subject, Qt::FontRole).value().strikeOut()); +} + +void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() +{ + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("junk")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {}); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, Qt::BackgroundRole).value().color(), + ThreadListModel::spamColour()); + QVERIFY(model.data(subject, Qt::FontRole).value().strikeOut()); + + // Spam and deleted must be distinguishable, not two shades of one colour. + QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour()); +} + +void TestThreadListModel::doomedStylingCoversEveryColumn() +{ + // A cue on one column would vanish the moment that column scrolled out of + // view, which is the bug this whole change exists to fix. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + const QModelIndex index = model.index(0, column); + QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), + qPrintable(QStringLiteral("column %1 has no background").arg(column))); + QVERIFY2(model.data(index, Qt::FontRole).value().strikeOut(), + qPrintable(QStringLiteral("column %1 is not struck through").arg(column))); + } +} + +void TestThreadListModel::ordinaryThreadsCarryNoRowColour() +{ + // Undo has to restore the plain look, not merely drop the tag. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("normal")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); + QVERIFY(!model.data(subject, Qt::ForegroundRole).isValid()); + const QVariant font = model.data(subject, Qt::FontRole); + QVERIFY(!font.isValid() || !font.value().strikeOut()); +} + void TestThreadListModel::threadIdIsReachableFromAnIndex() { // The view hands MainWindow a QModelIndex; the worker needs a thread id. -- cgit v1.2.3 From df9b3a47f125e65c5610c62a14894c8c7514f954 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 14:58:45 +0200 Subject: fix: let the thread list columns be resized again Tags, Date and From were set to ResizeToContents while fixing the pushed-off-screen Tags column. That mode computes the width itself and discards a drag, so the columns stopped being resizable. Verified: with ResizeToContents a request for 250px yields 150, with Stretch 478, with Interactive 250. The reorder alone already fixed the original bug, since Subject stretches and is last, so nothing can be pushed past it. Locking the other three was unnecessary and also blocked the saved-column-widths item, which needs widths a user can actually set. They are Interactive again, with starting widths a drag overrides. --- src/mainwindow.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 915c1ae..bebe379 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -169,16 +169,21 @@ void MainWindow::buildUi() m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection); m_threadView->verticalHeader()->hide(); m_threadView->horizontalHeader()->setStretchLastSection(false); - // Subject is the last column and takes the leftover width; the three - // fixed-width ones size to their contents. Nothing sits to the right of - // the stretching column, so no column can be pushed out of view. + // Subject is last and takes the leftover width, so no column can be + // pushed off the right edge. The other three stay Interactive: both + // ResizeToContents and Stretch ignore a drag, and the user resizes these. m_threadView->horizontalHeader()->setSectionResizeMode( ThreadListModel::SubjectColumn, QHeaderView::Stretch); for (int column : { ThreadListModel::TagsColumn, ThreadListModel::DateColumn, ThreadListModel::AuthorsColumn }) { m_threadView->horizontalHeader()->setSectionResizeMode( - column, QHeaderView::ResizeToContents); + column, QHeaderView::Interactive); } + // Starting widths only; a drag overrides them, and they are what the + // saved-widths item will persist. + m_threadView->setColumnWidth(ThreadListModel::TagsColumn, 160); + m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130); + m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180); connect(m_threadView->selectionModel(), &QItemSelectionModel::currentRowChanged, -- cgit v1.2.3 From ab16342f6181f69e5cb1ef528f1dd0eb433bb74b Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 15:04:28 +0200 Subject: feat: make every thread list column resizable Subject was Stretch, which computes its own width and discards a drag, so it alone could not be resized. Every column is Interactive now. Nothing absorbs spare width as a consequence, so the view scrolls horizontally instead of squeezing columns when their total exceeds the viewport. Per-pixel, so scrolling does not jump a column at a time. --- src/mainwindow.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index bebe379..ba690b4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -169,21 +169,24 @@ void MainWindow::buildUi() m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection); m_threadView->verticalHeader()->hide(); m_threadView->horizontalHeader()->setStretchLastSection(false); - // Subject is last and takes the leftover width, so no column can be - // pushed off the right edge. The other three stay Interactive: both - // ResizeToContents and Stretch ignore a drag, and the user resizes these. - m_threadView->horizontalHeader()->setSectionResizeMode( - ThreadListModel::SubjectColumn, QHeaderView::Stretch); - for (int column : { ThreadListModel::TagsColumn, ThreadListModel::DateColumn, - ThreadListModel::AuthorsColumn }) { + // Every column Interactive, Subject included: Stretch and ResizeToContents + // both compute a width and discard the user's drag. Nothing absorbs spare + // width as a result, so the columns end where they end. + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { m_threadView->horizontalHeader()->setSectionResizeMode( column, QHeaderView::Interactive); } + // Widening a column past the viewport scrolls rather than squeezing the + // others. Per-pixel so the scroll does not jump a whole column at a time. + m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded); + m_threadView->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel); + // Starting widths only; a drag overrides them, and they are what the // saved-widths item will persist. m_threadView->setColumnWidth(ThreadListModel::TagsColumn, 160); m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130); m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180); + m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520); connect(m_threadView->selectionModel(), &QItemSelectionModel::currentRowChanged, -- cgit v1.2.3 From cd0be6dc5e4741c2ad180d6bf6a275f000ab943a Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 15:41:16 +0200 Subject: feat: render tags as chips instead of a text column Spelled out per row, tags ran to 500 pixels of largely repeated text and took most of the thread list's width. The column is gone; tags render as coloured chips split by what they actually mean. An account tag says which mailbox a thread arrived in, and draws as a chip in front of the subject. A functional tag says what state a thread is in, and those fill one row under the message pane. One row keeps the message area from shifting between threads with different tag counts, so whatever does not fit collapses into a +N chip that names the rest in its tooltip. TagColors resolves a colour by exact tag first, then by top-level prefix, so a single "shopping" entry covers shopping/amazon and shopping/nike while shopping/amazon can still override its own. That matters at 96 tags. Built-in defaults cover the usual state tags, and anything left unconfigured falls back to a hash of the name, stable so a chip does not change colour as the list scrolls. --- src/messageview.cpp | 18 +++ src/messageview.h | 9 ++ src/tagchip.cpp | 126 +++++++++++++++++++++ src/tagchip.h | 62 ++++++++++ src/tagcolors.cpp | 171 ++++++++++++++++++++++++++++ src/tagcolors.h | 92 +++++++++++++++ src/tagstrip.cpp | 136 ++++++++++++++++++++++ src/tagstrip.h | 63 +++++++++++ src/threadlistmodel.cpp | 23 +++- src/threadlistmodel.h | 27 ++++- tests/CMakeLists.txt | 1 + tests/test_tagcolors.cpp | 251 +++++++++++++++++++++++++++++++++++++++++ tests/test_threadlistmodel.cpp | 88 +++++++++++++-- 13 files changed, 1052 insertions(+), 15 deletions(-) create mode 100644 src/tagchip.cpp create mode 100644 src/tagchip.h create mode 100644 src/tagcolors.cpp create mode 100644 src/tagcolors.h create mode 100644 src/tagstrip.cpp create mode 100644 src/tagstrip.h create mode 100644 tests/test_tagcolors.cpp (limited to 'src') diff --git a/src/messageview.cpp b/src/messageview.cpp index 3bb08a0..aebb81b 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -34,6 +34,7 @@ #include "cidschemehandler.h" #include "htmlbuilder.h" #include "requestinterceptor.h" +#include "tagstrip.h" #include "threadcidmap.h" namespace { @@ -118,17 +119,33 @@ MessageView::MessageView(QWidget *parent) m_attachmentBar = new QWidget(this); new QHBoxLayout(m_attachmentBar); + // Tags live under the message rather than in the thread list, where + // spelling them out cost most of the list's width. + m_tagStrip = new TagStrip(this); + m_tagStrip->hide(); + auto *layout = new QVBoxLayout(this); layout->addWidget(m_headerLabel); layout->addLayout(blockedRow); layout->addWidget(m_view, 1); layout->addWidget(m_attachmentBar); + layout->addWidget(m_tagStrip); clear(); } MessageView::~MessageView() = default; +void MessageView::setTagColors(const TagColors *colours) +{ + m_tagStrip->setTagColors(colours); +} + +void MessageView::setTags(const QStringList &tags) +{ + m_tagStrip->setTags(tags); +} + /// The single place that loads a document into the view. /// /// RequestInterceptor trusts exactly one qtmaildir: URL and denies every other @@ -145,6 +162,7 @@ void MessageView::setDocument(const QString &html) void MessageView::clear() { m_items.clear(); + m_tagStrip->setTags({}); // No thread is displayed, so nothing may be served or allowed. Without // this, the previous thread's parts would stay reachable. diff --git a/src/messageview.h b/src/messageview.h index f3bd96f..9570db5 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -30,6 +30,8 @@ class QPushButton; class QWebEngineView; class QWebEngineProfile; class CidSchemeHandler; +class TagColors; +class TagStrip; class RequestInterceptor; /// The message pane: thread header, body, attachment bar. @@ -57,6 +59,12 @@ public: void showError(const QString &text, const QString &filePath); void clear(); + /// Supplies the tag strip's colours. Not owned; must outlive the view. + void setTagColors(const TagColors *colours); + + /// Tags of the thread on display, shown as chips along the bottom. + void setTags(const QStringList &tags); + public slots: void toggleHtml(); void loadRemoteContent(); @@ -81,4 +89,5 @@ private: QLabel *m_blockedLabel = nullptr; QPushButton *m_loadRemoteButton = nullptr; QWidget *m_attachmentBar = nullptr; + TagStrip *m_tagStrip = nullptr; }; diff --git a/src/tagchip.cpp b/src/tagchip.cpp new file mode 100644 index 0000000..2e21419 --- /dev/null +++ b/src/tagchip.cpp @@ -0,0 +1,126 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "tagchip.h" + +#include +#include +#include + +#include "tagcolors.h" +#include "threadlistmodel.h" + +namespace TagChip { + +QSize sizeFor(const QFontMetrics &metrics, const QString &text) +{ + return QSize(metrics.horizontalAdvance(text) + kPaddingX * 2, + metrics.height() + kPaddingY * 2); +} + +void paint(QPainter *painter, const QRect &rect, const QString &text, + const QColor &background) +{ + painter->save(); + painter->setRenderHint(QPainter::Antialiasing, true); + painter->setPen(Qt::NoPen); + painter->setBrush(background); + painter->drawRoundedRect(rect, kRadius, kRadius); + + painter->setPen(TagColors::textColourOn(background)); + painter->drawText(rect, Qt::AlignCenter, text); + painter->restore(); +} + +} // namespace TagChip + +void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, + const QModelIndex &index) const +{ + const QString account = + index.data(ThreadListModel::AccountLabelRole).toString(); + if (account.isEmpty()) { + QStyledItemDelegate::paint(painter, option, index); + return; + } + + // Draw the row's own background and selection first, then the chip and the + // subject on top, so a selected or struck-through row still looks right. + QStyleOptionViewItem chrome = option; + initStyleOption(&chrome, index); + chrome.text.clear(); + const QWidget *widget = option.widget; + QStyle *style = widget ? widget->style() : QApplication::style(); + style->drawControl(QStyle::CE_ItemViewItem, &chrome, painter, widget); + + const QFontMetrics metrics(option.font); + const QSize chipSize = TagChip::sizeFor(metrics, account); + const QRect chipRect(option.rect.left() + TagChip::kSpacing, + option.rect.top() + + (option.rect.height() - chipSize.height()) / 2, + chipSize.width(), chipSize.height()); + + const QColor colour = + index.data(ThreadListModel::AccountColourRole).value(); + TagChip::paint(painter, chipRect, account, + colour.isValid() ? colour : QColor(0x55, 0x55, 0x5f)); + + // The subject follows the chip, elided so a long one cannot overflow. + QRect textRect = option.rect; + textRect.setLeft(chipRect.right() + TagChip::kSpacing * 2); + if (textRect.width() <= 0) + return; + + painter->save(); + // The model supplies the row's colours; honouring them keeps a deleted + // thread white-on-red here as everywhere else. + const QVariant foreground = index.data(Qt::ForegroundRole); + if (foreground.isValid()) + painter->setPen(foreground.value().color()); + else if (option.state & QStyle::State_Selected) + painter->setPen(option.palette.highlightedText().color()); + else + painter->setPen(option.palette.text().color()); + + // The model's font carries bold for unread and strike-out for deleted. + // initStyleOption() already resolved it into chrome.font; using it rather + // than option.font is what keeps those cues on a delegate-drawn subject. + const QVariant fontData = index.data(Qt::FontRole); + const QFont rowFont = fontData.isValid() ? fontData.value() + : chrome.font; + painter->setFont(rowFont); + const QFontMetrics rowMetrics(rowFont); + painter->drawText(textRect, Qt::AlignVCenter | Qt::AlignLeft, + rowMetrics.elidedText(index.data(Qt::DisplayRole).toString(), + Qt::ElideRight, textRect.width())); + painter->restore(); +} + +QSize SubjectDelegate::sizeHint(const QStyleOptionViewItem &option, + const QModelIndex &index) const +{ + QSize size = QStyledItemDelegate::sizeHint(option, index); + const QString account = + index.data(ThreadListModel::AccountLabelRole).toString(); + if (!account.isEmpty()) { + const QFontMetrics metrics(option.font); + size.setWidth(size.width() + TagChip::sizeFor(metrics, account).width() + + TagChip::kSpacing * 3); + } + return size; +} diff --git a/src/tagchip.h b/src/tagchip.h new file mode 100644 index 0000000..9bd4e78 --- /dev/null +++ b/src/tagchip.h @@ -0,0 +1,62 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include +#include +#include +#include + +class QPainter; +class QFontMetrics; + +/// Draws one rounded, filled tag chip. Shared so the account chip in the +/// thread list and the strip under the message pane cannot drift apart. +namespace TagChip { + +/// Padding inside a chip and the gap between two of them. +constexpr int kPaddingX = 6; +constexpr int kPaddingY = 1; +constexpr int kSpacing = 4; +constexpr int kRadius = 3; + +QSize sizeFor(const QFontMetrics &metrics, const QString &text); + +/// Paints the chip into `rect`, using `text` and `background`. The text colour +/// is derived from the fill so it stays legible. +void paint(QPainter *painter, const QRect &rect, const QString &text, + const QColor &background); + +} // namespace TagChip + +/// Item delegate for the subject column: draws the account chip in front of +/// the subject text, so which mailbox a thread came from reads at a glance +/// without a tags column spelling it out. +class SubjectDelegate : public QStyledItemDelegate +{ + Q_OBJECT +public: + using QStyledItemDelegate::QStyledItemDelegate; + + void paint(QPainter *painter, const QStyleOptionViewItem &option, + const QModelIndex &index) const override; + QSize sizeHint(const QStyleOptionViewItem &option, + const QModelIndex &index) const override; +}; diff --git a/src/tagcolors.cpp b/src/tagcolors.cpp new file mode 100644 index 0000000..88ca2ab --- /dev/null +++ b/src/tagcolors.cpp @@ -0,0 +1,171 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "tagcolors.h" + +#include +#include + +namespace { + +/// Colours for the tags every notmuch setup has. Chosen to stay legible on a +/// dark theme, which is where the message pane already sits. +QHash builtInColours() +{ + return { + { QStringLiteral("flagged"), QColor(0xd4, 0x9c, 0x1a) }, + { QStringLiteral("unread"), QColor(0x2f, 0x6f, 0xa8) }, + { QStringLiteral("deleted"), QColor(0x8b, 0x2c, 0x2c) }, + { QStringLiteral("spam"), QColor(0xa8, 0x5c, 0x18) }, + { QStringLiteral("attachment"), QColor(0x5a, 0x5a, 0x64) }, + { QStringLiteral("replied"), QColor(0x3d, 0x7a, 0x4a) }, + { QStringLiteral("passed"), QColor(0x3d, 0x7a, 0x62) }, + { QStringLiteral("draft"), QColor(0x77, 0x66, 0x33) }, + { QStringLiteral("encrypted"), QColor(0x6a, 0x4a, 0x8a) }, + { QStringLiteral("signed"), QColor(0x53, 0x4a, 0x8a) }, + { QStringLiteral("inbox"), QColor(0x44, 0x4a, 0x52) }, + { QStringLiteral("mailing-list"), QColor(0x36, 0x6a, 0x6a) }, + }; +} + +} // namespace + +bool TagColors::isAccountTag(const QString &tag) +{ + // The prefix alone, with nothing after it, names no account. + return tag.startsWith(accountTagPrefix()) + && tag.size() > accountTagPrefix().size(); +} + +QString TagColors::accountKeyForTag(const QString &tag) +{ + if (!isAccountTag(tag)) + return {}; + return tag.mid(accountTagPrefix().size()); +} + +QString TagColors::tagForAccountKey(const QString &key) +{ + return accountTagPrefix() + key; +} + +QColor TagColors::textColourOn(const QColor &background) +{ + // Perceived luminance: the eye weights green far above blue, so a plain + // average would call a saturated blue "light" and print black on it. + const double luminance = (0.299 * background.red() + + 0.587 * background.green() + + 0.114 * background.blue()) / 255.0; + return luminance > 0.55 ? QColor(Qt::black) : QColor(Qt::white); +} + +QString TagColors::topLevelPrefix(const QString &tag) +{ + const int slash = tag.indexOf(QLatin1Char('/')); + return slash < 0 ? tag : tag.left(slash); +} + +void TagColors::load(QSettings &settings) +{ + settings.beginGroup(QStringLiteral("tagcolors")); + // allKeys(), not childKeys(): QSettings treats '/' in a key as a group + // separator, so a hierarchical tag like shopping/amazon becomes a nested + // key that childKeys() does not return. allKeys() reports both, and the + // nested one comes back in the "shopping/amazon" form the tag already has. + // (In the INI file itself it is written as shopping\amazon.) + const QStringList keys = settings.allKeys(); + for (const QString &key : keys) { + const QString value = settings.value(key).toString(); + const QColor colour(value); + if (!colour.isValid()) { + m_warnings.append( + QStringLiteral("Unparseable colour '%1' for tag '%2' in " + "[tagcolors]").arg(value, key)); + continue; + } + m_colours.insert(key, colour); + } + settings.endGroup(); +} + +void TagColors::setAccountColour(const QString &accountKey, const QColor &colour) +{ + if (accountKey.isEmpty() || !colour.isValid()) + return; + m_accountColours.insert(accountKey, colour); +} + +void TagColors::setAccountLabel(const QString &accountKey, const QString &label) +{ + if (accountKey.isEmpty() || label.isEmpty()) + return; + m_accountLabels.insert(accountKey, label); +} + +QString TagColors::labelForAccountTag(const QString &tag) const +{ + const QString key = accountKeyForTag(tag); + if (key.isEmpty()) + return {}; + return m_accountLabels.value(key, key); +} + +bool TagColors::hasColour(const QString &tag) const +{ + if (isAccountTag(tag)) + return m_accountColours.contains(accountKeyForTag(tag)); + + const QHash builtIn = builtInColours(); + return m_colours.contains(tag) || builtIn.contains(tag) + || m_colours.contains(topLevelPrefix(tag)) + || builtIn.contains(topLevelPrefix(tag)); +} + +QColor TagColors::colourFor(const QString &tag) const +{ + // An account's colour lives in its own stanza, not in [tagcolors]. + if (isAccountTag(tag)) { + const QColor colour = m_accountColours.value(accountKeyForTag(tag)); + if (colour.isValid()) + return colour; + } + + const QHash builtIn = builtInColours(); + + // Most specific first: an exact entry must beat the prefix it falls under, + // or a single child tag could never be singled out. + if (m_colours.contains(tag)) + return m_colours.value(tag); + if (builtIn.contains(tag)) + return builtIn.value(tag); + + const QString prefix = topLevelPrefix(tag); + if (m_colours.contains(prefix)) + return m_colours.value(prefix); + if (builtIn.contains(prefix)) + return builtIn.value(prefix); + + // Nothing configured: derive a colour from the name so the chip is still + // readable and distinguishable. Hashing keeps it stable across calls, and + // the fixed saturation and lightness keep it in the same family as the + // built-ins rather than producing neon. + const QByteArray digest = + QCryptographicHash::hash(tag.toUtf8(), QCryptographicHash::Md5); + const int hue = static_cast(digest.at(0)) * 360 / 256; + return QColor::fromHsl(hue, 90, 80); +} diff --git a/src/tagcolors.h b/src/tagcolors.h new file mode 100644 index 0000000..f9e1a95 --- /dev/null +++ b/src/tagcolors.h @@ -0,0 +1,92 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include +#include +#include + +class QSettings; + +/// Colours for tag chips. +/// +/// Tags fall into two taxonomies. A functional tag says what state a thread is +/// in (flagged, replied, shopping/amazon) and is coloured from built-in +/// defaults or the [tagcolors] config group. An account tag says which mailbox +/// it arrived in, is named account- after the [account.] stanza, and +/// takes its colour from that stanza instead. +/// +/// Lookup is by exact tag first, then by top-level prefix, so one entry can +/// colour a whole hierarchy: "shopping" covers shopping/amazon and +/// shopping/nike, while "shopping/amazon" still overrides its own. +class TagColors +{ +public: + /// The prefix marking a tag as naming an account rather than a state. + static QString accountTagPrefix() { return QStringLiteral("account-"); } + + static bool isAccountTag(const QString &tag); + + /// The [account.] suffix behind an account tag, empty if not one. + static QString accountKeyForTag(const QString &tag); + + /// The tag notmuch carries for an account key. The mapping is derived, + /// never configured, so the two cannot drift. + static QString tagForAccountKey(const QString &key); + + /// Black or white, whichever stays legible on the given fill. + static QColor textColourOn(const QColor &background); + + /// Reads the [tagcolors] group. An unparseable colour is collected into + /// warnings() and the previous value kept, so one typo cannot leave a tag + /// unstyled. + void load(QSettings &settings); + + /// Registers an account's colour, taken from its own stanza. + void setAccountColour(const QString &accountKey, const QColor &colour); + + /// Registers the text shown on an account's chip. Empty is ignored: a + /// blank label would render an unreadable chip. The notmuch tag itself is + /// never renamed, only what the chip displays. + void setAccountLabel(const QString &accountKey, const QString &label); + + /// Chip text for an account tag, falling back to the account key. Empty + /// when the tag does not name an account. + QString labelForAccountTag(const QString &tag) const; + + /// True when this tag resolves to a colour that was chosen for it, as + /// opposed to the fallback every unknown tag receives. + bool hasColour(const QString &tag) const; + + /// Always valid: an unconfigured tag falls back to a colour derived from + /// its name, stable across calls so a chip never changes as you scroll. + QColor colourFor(const QString &tag) const; + + QStringList warnings() const { return m_warnings; } + +private: + /// The part before the first '/', which is the whole tag when it has none. + static QString topLevelPrefix(const QString &tag); + + QHash m_colours; ///< Exact tags and prefixes. + QHash m_accountColours; ///< Keyed by account key. + QHash m_accountLabels; ///< Keyed by account key. + QStringList m_warnings; +}; diff --git a/src/tagstrip.cpp b/src/tagstrip.cpp new file mode 100644 index 0000000..bad116a --- /dev/null +++ b/src/tagstrip.cpp @@ -0,0 +1,136 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "tagstrip.h" + +#include +#include + +#include "tagchip.h" +#include "tagcolors.h" + +namespace { + +/// Text of the chip standing in for tags that did not fit. +QString overflowText(int count) +{ + return QStringLiteral("+%1").arg(count); +} + +} // namespace + +TagStrip::TagStrip(QWidget *parent) + : QWidget(parent) +{ + setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); +} + +void TagStrip::setTagColors(const TagColors *colours) +{ + m_tagColors = colours; + update(); +} + +void TagStrip::setTags(const QStringList &tags) +{ + m_tags.clear(); + for (const QString &tag : tags) { + // The account tag is shown as a chip in the thread list instead: it + // says which mailbox the thread came from, not what state it is in. + if (!TagColors::isAccountTag(tag)) + m_tags.append(tag); + } + m_tags.sort(); + + relayout(); + setVisible(!m_tags.isEmpty()); + update(); +} + +void TagStrip::relayout() +{ + m_visible.clear(); + m_hidden.clear(); + if (m_tags.isEmpty()) + return; + + const QFontMetrics metrics(font()); + // Reserve room for the overflow chip up front. Sizing it for the worst + // case avoids the loop having to back out a chip it already placed. + const int overflowWidth = + TagChip::sizeFor(metrics, overflowText(m_tags.size())).width() + + TagChip::kSpacing; + + int used = 0; + for (int i = 0; i < m_tags.size(); ++i) { + const int chipWidth = + TagChip::sizeFor(metrics, m_tags.at(i)).width() + TagChip::kSpacing; + const bool isLast = (i == m_tags.size() - 1); + // Every chip but the last must also leave room for the overflow chip, + // since anything after it will be hidden. + const int needed = used + chipWidth + (isLast ? 0 : overflowWidth); + if (needed > width() && !m_visible.isEmpty()) { + m_hidden = m_tags.mid(i); + break; + } + m_visible.append(m_tags.at(i)); + used += chipWidth; + } + + setToolTip(m_hidden.isEmpty() ? QString() + : m_hidden.join(QStringLiteral(", "))); +} + +QSize TagStrip::sizeHint() const +{ + const QFontMetrics metrics(font()); + return QSize(0, metrics.height() + TagChip::kPaddingY * 2 + + TagChip::kSpacing * 2); +} + +void TagStrip::resizeEvent(QResizeEvent *event) +{ + QWidget::resizeEvent(event); + relayout(); +} + +void TagStrip::paintEvent(QPaintEvent *) +{ + if (m_visible.isEmpty()) + return; + + QPainter painter(this); + const QFontMetrics metrics(font()); + const int top = (height() - (metrics.height() + TagChip::kPaddingY * 2)) / 2; + + int x = 0; + for (const QString &tag : m_visible) { + const QSize size = TagChip::sizeFor(metrics, tag); + const QColor colour = m_tagColors ? m_tagColors->colourFor(tag) + : TagColors().colourFor(tag); + TagChip::paint(&painter, QRect(QPoint(x, top), size), tag, colour); + x += size.width() + TagChip::kSpacing; + } + + if (!m_hidden.isEmpty()) { + const QString text = overflowText(m_hidden.size()); + const QSize size = TagChip::sizeFor(metrics, text); + TagChip::paint(&painter, QRect(QPoint(x, top), size), text, + QColor(0x44, 0x44, 0x4c)); + } +} diff --git a/src/tagstrip.h b/src/tagstrip.h new file mode 100644 index 0000000..4102bed --- /dev/null +++ b/src/tagstrip.h @@ -0,0 +1,63 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include + +class TagColors; + +/// One row of tag chips under the message pane. +/// +/// A single row by design: the message area must not shift as you move between +/// threads with different numbers of tags. Whatever does not fit collapses +/// into a trailing "+N" chip whose tooltip names the hidden tags. +class TagStrip : public QWidget +{ + Q_OBJECT +public: + explicit TagStrip(QWidget *parent = nullptr); + + /// Not owned; must outlive the strip. + void setTagColors(const TagColors *colours); + + /// Account tags are filtered out: they belong to the thread list chip, + /// being a different taxonomy from the functional tags shown here. + void setTags(const QStringList &tags); + + QSize sizeHint() const override; + + /// The tags actually drawn, in order. Exposed for testing the overflow + /// split without rendering. + QStringList visibleTags() const { return m_visible; } + QStringList hiddenTags() const { return m_hidden; } + +protected: + void paintEvent(QPaintEvent *event) override; + void resizeEvent(QResizeEvent *event) override; + +private: + /// Recomputes the visible/hidden split for the current width. + void relayout(); + + QStringList m_tags; ///< Functional tags only, account ones removed. + QStringList m_visible; + QStringList m_hidden; + const TagColors *m_tagColors = nullptr; +}; diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index a083145..2f2882e 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -65,6 +65,26 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const if (role == ThreadIdRole) return thread.threadId; + if (role == TagsRole) + return thread.tags; + + if (role == AccountLabelRole || role == AccountColourRole) { + // At most one account tag per thread in practice, but a thread whose + // messages landed in two mailboxes carries both; the first is shown. + for (const QString &tag : thread.tags) { + if (!TagColors::isAccountTag(tag)) + continue; + if (role == AccountLabelRole) { + // The configured label when there is one, otherwise the key. + return m_tagColors ? m_tagColors->labelForAccountTag(tag) + : TagColors::accountKeyForTag(tag); + } + return m_tagColors ? m_tagColors->colourFor(tag) + : TagColors().colourFor(tag); + } + return {}; + } + if (role == Qt::DisplayRole) { switch (index.column()) { case DateColumn: @@ -76,8 +96,6 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const ? QStringLiteral("%1 (%2)").arg(thread.subject) .arg(thread.totalCount) : thread.subject; - case TagsColumn: - return thread.tags.join(QLatin1Char(' ')); default: return {}; } @@ -126,7 +144,6 @@ QVariant ThreadListModel::headerData(int section, Qt::Orientation orientation, case DateColumn: return QStringLiteral("Date"); case AuthorsColumn: return QStringLiteral("From"); case SubjectColumn: return QStringLiteral("Subject"); - case TagsColumn: return QStringLiteral("Tags"); default: return {}; } } diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 01fd241..7ed8fef 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -22,6 +22,7 @@ #include #include +#include "tagcolors.h" #include "types.h" /// Table model over query results, filled in batches so a large query paints @@ -30,12 +31,12 @@ class ThreadListModel : public QAbstractTableModel { Q_OBJECT public: - /// Subject stretches to fill the view, so it must come last: anything - /// after it is pushed out of sight. Tags leads, being the column that - /// changes when the user acts on a thread. + /// No tags column: spelling out a dozen tags per row cost most of the + /// list's width and was unreadable. Functional tags moved to a chip strip + /// under the message pane, and the account tag renders as a chip in front + /// of the subject. enum Column { - TagsColumn = 0, - DateColumn, + DateColumn = 0, AuthorsColumn, SubjectColumn, ColumnCount, @@ -46,6 +47,17 @@ public: /// worker speaks thread ids, so the mapping belongs on the model /// rather than in every caller. ThreadIdRole = Qt::UserRole + 1, + + /// The account tag on this thread without its "account-" prefix, for + /// the chip drawn in front of the subject. Empty when the thread + /// carries none. + AccountLabelRole, + + /// Fill colour for that chip. + AccountColourRole, + + /// Every tag on the thread, for the strip under the message pane. + TagsRole, }; /// Row fill for a thread tagged `deleted`, and for one tagged `spam`. @@ -57,6 +69,10 @@ public: explicit ThreadListModel(QObject *parent = nullptr); + /// Supplies the account chip colours. Not owned; must outlive the model. + /// Without one, chips fall back to a colour generated from the tag name. + void setTagColors(const TagColors *colours) { m_tagColors = colours; } + int rowCount(const QModelIndex &parent = {}) const override; int columnCount(const QModelIndex &parent = {}) const override; QVariant data(const QModelIndex &index, int role) const override; @@ -76,4 +92,5 @@ public: private: QVector m_threads; + const TagColors *m_tagColors = nullptr; }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1833f29..e761cb6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -13,6 +13,7 @@ target_compile_definitions(test_mimeparser PRIVATE add_qtmaildir_test(interceptor) add_qtmaildir_test(htmlbuilder) add_qtmaildir_test(notmuchworker) +add_qtmaildir_test(tagcolors) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) add_qtmaildir_test(threadcidmap) diff --git a/tests/test_tagcolors.cpp b/tests/test_tagcolors.cpp new file mode 100644 index 0000000..53c0210 --- /dev/null +++ b/tests/test_tagcolors.cpp @@ -0,0 +1,251 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include +#include +#include + +#include "tagcolors.h" + +class TestTagColors : public QObject +{ + Q_OBJECT +private slots: + void builtInDefaultsExist(); + void prefixColoursWholeHierarchy(); + void exactTagBeatsItsPrefix(); + void configOverridesABuiltIn(); + void unknownTagStillGetsAColour(); + void accountTagsAreRecognised(); + void accountColourComesFromTheAccount(); + void accountLabelDefaultsToTheKey(); + void accountLabelCanBeOverridden(); + void malformedColourIsReported(); + void textContrastsWithItsBackground(); +}; + +void TestTagColors::builtInDefaultsExist() +{ + // The common state tags must be styled out of the box: a user who never + // writes a [tagcolors] section still needs flagged to stand out. + TagColors colours; + const QStringList expected = { QStringLiteral("flagged"), + QStringLiteral("unread"), + QStringLiteral("deleted"), + QStringLiteral("spam"), + QStringLiteral("attachment"), + QStringLiteral("replied") }; + for (const QString &tag : expected) { + QVERIFY2(colours.hasColour(tag), + qPrintable(QStringLiteral("no built-in colour for '%1'").arg(tag))); + } +} + +void TestTagColors::prefixColoursWholeHierarchy() +{ + // 96 tags, many of them shopping/foo and mailing-list/bar. Colouring by + // top-level prefix is what keeps the config from listing every one. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("shopping"), QStringLiteral("#3366cc")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("shopping/amazon")), + QColor(QStringLiteral("#3366cc"))); + QCOMPARE(colours.colourFor(QStringLiteral("shopping/nike")), + QColor(QStringLiteral("#3366cc"))); + // The bare prefix itself is a tag too. + QCOMPARE(colours.colourFor(QStringLiteral("shopping")), + QColor(QStringLiteral("#3366cc"))); + // A different hierarchy is unaffected. + QVERIFY(colours.colourFor(QStringLiteral("mailing-list/SBo")) + != QColor(QStringLiteral("#3366cc"))); +} + +void TestTagColors::exactTagBeatsItsPrefix() +{ + // Specific beats general, or you could never single out one child tag. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("shopping"), QStringLiteral("#3366cc")); + s.setValue(QStringLiteral("shopping/amazon"), QStringLiteral("#ff9900")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("shopping/amazon")), + QColor(QStringLiteral("#ff9900"))); + QCOMPARE(colours.colourFor(QStringLiteral("shopping/nike")), + QColor(QStringLiteral("#3366cc"))); + + // Regression: QSettings treats '/' as a group separator, so a + // hierarchical tag is a nested key that childKeys() never returns. Reading + // the group with childKeys() silently dropped every tag with a '/' in it, + // which is most of this user's, and they all fell through to their prefix. + QVERIFY(colours.hasColour(QStringLiteral("shopping/amazon"))); +} + +void TestTagColors::configOverridesABuiltIn() +{ + TagColors colours; + const QColor original = colours.colourFor(QStringLiteral("flagged")); + + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("flagged"), QStringLiteral("#00ff00")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("flagged")), + QColor(QStringLiteral("#00ff00"))); + QVERIFY(colours.colourFor(QStringLiteral("flagged")) != original); +} + +void TestTagColors::unknownTagStillGetsAColour() +{ + // A chip with no colour would render as an invisible blank, so every tag + // resolves to something even when nothing is configured for it. + TagColors colours; + const QColor colour = colours.colourFor(QStringLiteral("no-such-tag-anywhere")); + QVERIFY(colour.isValid()); + + // Stable across calls: a tag must not change colour as you scroll. + QCOMPARE(colours.colourFor(QStringLiteral("no-such-tag-anywhere")), colour); +} + +void TestTagColors::accountTagsAreRecognised() +{ + // Account tags are a different taxonomy from functional tags: which + // mailbox a thread came from, not what state it is in. They are shown + // separately, so they have to be identifiable. + QVERIFY(TagColors::isAccountTag(QStringLiteral("account-gmail-danixland"))); + QVERIFY(!TagColors::isAccountTag(QStringLiteral("flagged"))); + QVERIFY(!TagColors::isAccountTag(QStringLiteral("shopping/amazon"))); + + // The INI key for [account.gmail-danixland] is what follows "account-". + QCOMPARE(TagColors::accountKeyForTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("gmail-danixland")); + QVERIFY(TagColors::accountKeyForTag(QStringLiteral("flagged")).isEmpty()); + + // Round trip, since the mapping is derived rather than configured. + QCOMPARE(TagColors::tagForAccountKey(QStringLiteral("gmail-danixland")), + QStringLiteral("account-gmail-danixland")); +} + +void TestTagColors::accountColourComesFromTheAccount() +{ + // Per the account stanza, not [tagcolors]: the colour belongs to the + // account, and the tag name is derived from its key. + TagColors colours; + colours.setAccountColour(QStringLiteral("gmail-danixland"), + QColor(QStringLiteral("#cc0000"))); + + QCOMPARE(colours.colourFor(QStringLiteral("account-gmail-danixland")), + QColor(QStringLiteral("#cc0000"))); +} + +void TestTagColors::accountLabelDefaultsToTheKey() +{ + // Without a configured label the chip shows the account key, which is what + // it did before labels existed. + TagColors colours; + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("gmail-danixland")); + + // Not an account tag: nothing to label. + QVERIFY(colours.labelForAccountTag(QStringLiteral("flagged")).isEmpty()); +} + +void TestTagColors::accountLabelCanBeOverridden() +{ + // "account-privateemail-danilo.macri" is 33 characters of chip for what is + // really one bit of information, so the label is configurable. + TagColors colours; + colours.setAccountLabel(QStringLiteral("gmail-danixland"), + QStringLiteral("GM-danixland")); + colours.setAccountLabel(QStringLiteral("privateemail-danix"), + QStringLiteral("PE-danix")); + + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("GM-danixland")); + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-privateemail-danix")), + QStringLiteral("PE-danix")); + + // An account left unlabelled still falls back to its key. + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-work")), + QStringLiteral("work")); + + // An empty label is not an override: it would render a blank chip. + colours.setAccountLabel(QStringLiteral("gmail-danixland"), QString()); + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("GM-danixland")); +} + +void TestTagColors::malformedColourIsReported() +{ + // A typo must be visible rather than silently ignored, matching how the + // rest of the config reports its problems. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("flagged"), QStringLiteral("not-a-colour")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.warnings().size(), 1); + QVERIFY(colours.warnings().first().contains(QStringLiteral("flagged"))); + // The built-in survives, so one bad line does not leave the tag unstyled. + QVERIFY(colours.colourFor(QStringLiteral("flagged")).isValid()); +} + +void TestTagColors::textContrastsWithItsBackground() +{ + // A chip is coloured text on a coloured fill, so the pair has to stay + // legible whatever colour the user picks. + QCOMPARE(TagColors::textColourOn(QColor(Qt::black)), QColor(Qt::white)); + QCOMPARE(TagColors::textColourOn(QColor(Qt::white)), QColor(Qt::black)); + QCOMPARE(TagColors::textColourOn(QColor(QStringLiteral("#8b2c2c"))), + QColor(Qt::white)); + QCOMPARE(TagColors::textColourOn(QColor(QStringLiteral("#ffee88"))), + QColor(Qt::black)); +} + +QTEST_MAIN(TestTagColors) +#include "test_tagcolors.moc" diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 98c477c..e8a5fa8 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -34,6 +34,9 @@ private slots: void subjectShowsMessageCountOnlyForRealThreads(); void unreadThreadsRenderBold(); void tagsAreTheFirstColumnAndSubjectTheLast(); + void accountTagBecomesAChipLabel(); + void unreadStylingSurvivesAnAccountChip(); + void accountChipUsesTheConfiguredColour(); void deletedThreadsAreRedAndStruckThrough(); void spamThreadsAreOrangeAndStruckThrough(); void doomedStylingCoversEveryColumn(); @@ -118,9 +121,11 @@ void TestThreadListModel::reportsSubjectAndAuthors() const QModelIndex date = model.index(0, ThreadListModel::DateColumn); QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty()); - const QModelIndex tags = model.index(0, ThreadListModel::TagsColumn); - QCOMPARE(model.data(tags, Qt::DisplayRole).toString(), - QStringLiteral("inbox unread")); + // Tags are no longer a column; they reach the strip under the message + // pane through a role instead. + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(), + QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") })); } void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() @@ -163,17 +168,86 @@ void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast() // Subject stretches to fill the view, so whatever sits after it is pushed // off-screen. Tags used to be there, which is why acting on a thread // looked like it did nothing: the only column that changed was invisible. - QCOMPARE(ThreadListModel::TagsColumn, 0); QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1); ThreadListModel model; model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); - QCOMPARE(model.headerData(ThreadListModel::TagsColumn, Qt::Horizontal, - Qt::DisplayRole).toString(), - QStringLiteral("Tags")); QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal, Qt::DisplayRole).toString(), QStringLiteral("Subject")); + + // No tags column at all: spelling out a dozen tags per row consumed most + // of the list's width and was unreadable. + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole) + .toString() != QStringLiteral("Tags")); + } +} + +void TestThreadListModel::accountTagBecomesAChipLabel() +{ + // The account tag is a different taxonomy from a functional one: which + // mailbox the thread arrived in. It renders as a chip in front of the + // subject, so the model exposes its label and colour separately. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(), + QStringLiteral("gmail-danixland")); + QVERIFY(model.data(subject, ThreadListModel::AccountColourRole) + .value().isValid()); + + // A thread with no account tag gets no chip rather than an empty one. + ThreadListModel plain; + ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi")); + untagged.tags = QStringList{ QStringLiteral("inbox") }; + plain.appendBatch({ untagged }); + QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn), + ThreadListModel::AccountLabelRole).toString().isEmpty()); +} + +void TestThreadListModel::unreadStylingSurvivesAnAccountChip() +{ + // The subject cell is drawn by a delegate when the thread has an account + // chip. The delegate paints the text itself, so it has to keep honouring + // the model's font: otherwise an unread thread stops rendering bold for + // exactly those threads that carry an account tag, which is all of them. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread"), + QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole) + .toString().isEmpty()); + + const QVariant font = model.data(subject, Qt::FontRole); + QVERIFY2(font.isValid(), "unread thread with an account tag has no font"); + QVERIFY2(font.value().bold(), "unread thread is not bold"); +} + +void TestThreadListModel::accountChipUsesTheConfiguredColour() +{ + // The colour comes from the account's own stanza, so a configured one must + // reach the chip rather than the generated fallback. + TagColors colours; + colours.setAccountColour(QStringLiteral("gmail-danixland"), + QColor(QStringLiteral("#cc0000"))); + + ThreadListModel model; + model.setTagColors(&colours); + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), + ThreadListModel::AccountColourRole).value(), + QColor(QStringLiteral("#cc0000"))); } void TestThreadListModel::deletedThreadsAreRedAndStruckThrough() -- cgit v1.2.3 From db7bbb81f02b52c2a1b299fe140abffab7e74642 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 15:41:33 +0200 Subject: feat: let an account set its chip colour and label Two optional keys in an [account.] stanza. color fills the chip, label sets its text. Both belong to the account rather than to [tagcolors] because an account tag is a different taxonomy: which mailbox a thread arrived in, not what state it is in. label is display only. "account-privateemail-danilo.macri" is a lot of row for one bit of information, but the notmuch tag is never renamed, so existing queries and external tagging are unaffected. Unset falls back to the account key, and an empty label is ignored rather than rendering a blank chip. --- src/config.cpp | 17 +++++++++++++++++ src/config.h | 10 ++++++++++ src/mainwindow.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- src/mainwindow.h | 2 ++ 4 files changed, 77 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/config.cpp b/src/config.cpp index de6a783..f21bba9 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -89,6 +89,23 @@ void Config::load(const QString &path) account.address = settings.value(QStringLiteral("address")).toString(); account.maildir = settings.value(QStringLiteral("maildir")).toString(); account.drafts = settings.value(QStringLiteral("drafts")).toString(); + + // Both optional, and both describe this account's chip in the thread + // list. An account tag is a different taxonomy from a functional one, + // saying which mailbox a thread arrived in rather than what state it + // is in, so these live here rather than in [tagcolors]. + account.label = settings.value(QStringLiteral("label")).toString(); + + const QString colour = settings.value(QStringLiteral("color")).toString(); + if (!colour.isEmpty()) { + account.color = QColor(colour); + if (!account.color.isValid()) { + addProblem( + QStringLiteral("Account '%1' has an unparseable color '%2'; " + "using a generated one.") + .arg(account.key, colour)); + } + } settings.endGroup(); if (!account.isValid()) { diff --git a/src/config.h b/src/config.h index 270b780..943cbd8 100644 --- a/src/config.h +++ b/src/config.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include #include @@ -33,6 +34,15 @@ struct Account QString maildir; ///< Relative to notmuch's database.path. QString drafts; ///< Unused in v1; send is v2. + /// Chip colour in the thread list. Invalid when unset, in which case one + /// is generated from the account tag's name. + QColor color; + + /// Text shown on the chip. Empty falls back to the key, which can be long: + /// "privateemail-danilo.macri" is a lot of row for one bit of information. + /// This renames nothing in notmuch, only what the chip displays. + QString label; + bool isValid() const { return !key.isEmpty() && !maildir.isEmpty(); } /// Restricts a notmuch query to this account's subtree. diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index ba690b4..48b475c 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -42,6 +42,7 @@ #include "messageview.h" #include "mimeparser.h" #include "notmuchworker.h" +#include "tagchip.h" #include "threadlistmodel.h" #include "version.h" @@ -73,6 +74,14 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) { QSettings settings(Config::defaultPath(), QSettings::IniFormat); m_keyMap.loadOverrides(settings); + m_tagColors.load(settings); + } + + // An account's chip colour comes from its own stanza, since an account tag + // is a different taxonomy from a functional one. + for (const Account &account : m_config.accounts()) { + m_tagColors.setAccountColour(account.key, account.color); + m_tagColors.setAccountLabel(account.key, account.label); } buildUi(); @@ -163,6 +172,7 @@ void MainWindow::buildUi() // Thread list and message pane. m_model = new ThreadListModel(this); + m_model->setTagColors(&m_tagColors); m_threadView = new QTableView(central); m_threadView->setModel(m_model); m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows); @@ -176,6 +186,10 @@ void MainWindow::buildUi() m_threadView->horizontalHeader()->setSectionResizeMode( column, QHeaderView::Interactive); } + + // The subject cell carries the account chip in front of its text. + m_threadView->setItemDelegateForColumn(ThreadListModel::SubjectColumn, + new SubjectDelegate(this)); // Widening a column past the viewport scrolls rather than squeezing the // others. Per-pixel so the scroll does not jump a whole column at a time. m_threadView->setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded); @@ -183,7 +197,6 @@ void MainWindow::buildUi() // Starting widths only; a drag overrides them, and they are what the // saved-widths item will persist. - m_threadView->setColumnWidth(ThreadListModel::TagsColumn, 160); m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130); m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180); m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520); @@ -193,6 +206,7 @@ void MainWindow::buildUi() this, &MainWindow::onThreadSelected); m_messageView = new MessageView(central); + m_messageView->setTagColors(&m_tagColors); connect(m_messageView, &MessageView::statusMessage, this, [this](const QString &text) { m_statusLabel->setText(text); }); @@ -356,6 +370,28 @@ void MainWindow::buildMenus() auto *about = helpMenu->addAction(tr("&About")); connect(about, &QAction::triggered, this, &MainWindow::showAbout); + // Standard names from the icon theme, so the buttons match the rest of the + // desktop rather than shipping bespoke art. A theme that lacks one leaves + // that action with text alone, which still works. + const QHash themeIcons = { + { QStringLiteral("sync"), QStringLiteral("mail-receive") }, + { QStringLiteral("archive"), QStringLiteral("mail-mark-read") }, + { QStringLiteral("delete"), QStringLiteral("edit-delete") }, + { QStringLiteral("undo"), QStringLiteral("edit-undo") }, + { QStringLiteral("spam"), QStringLiteral("mail-mark-junk") }, + { QStringLiteral("flag"), QStringLiteral("mail-mark-important") }, + { QStringLiteral("quit"), QStringLiteral("application-exit") }, + { QStringLiteral("focus_query"), QStringLiteral("edit-find") }, + }; + for (auto it = themeIcons.cbegin(); it != themeIcons.cend(); ++it) { + QAction *action = m_actions.value(it.key()); + if (!action) + continue; + const QIcon icon = QIcon::fromTheme(it.value()); + if (!icon.isNull()) + action->setIcon(icon); + } + // The frequent subset only. A toolbar holding every action is as // unreadable as no toolbar. auto *toolBar = addToolBar(tr("Main")); @@ -543,7 +579,9 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, if (!current.isValid()) return; - m_currentThreadId = m_model->threadAt(current.row()).threadId; + const ThreadSummary thread = m_model->threadAt(current.row()); + m_currentThreadId = thread.threadId; + m_messageView->setTags(thread.tags); QMetaObject::invokeMethod(m_worker, "loadThread", Qt::QueuedConnection, Q_ARG(QString, m_currentThreadId), Q_ARG(QString, m_lastQuery), @@ -665,6 +703,14 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds, for (const QString &threadId : threadIds) m_model->applyTagChange(threadId, add, remove); + // The strip shows the open thread's tags, so it has to follow a change to + // that thread rather than waiting for the next selection. + if (threadIds.contains(m_currentThreadId)) { + const QModelIndex current = m_threadView->currentIndex(); + if (current.isValid()) + m_messageView->setTags(m_model->threadAt(current.row()).tags); + } + m_pendingThreadIds = threadIds; m_pendingChange = TagChange{ {}, add, remove, description }; diff --git a/src/mainwindow.h b/src/mainwindow.h index 30a128a..4445894 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -28,6 +28,7 @@ #include "config.h" #include "keymap.h" +#include "tagcolors.h" #include "types.h" class QAction; @@ -103,6 +104,7 @@ private: Config m_config; KeyMap m_keyMap; + TagColors m_tagColors; QThread m_workerThread; NotmuchWorker *m_worker = nullptr; -- cgit v1.2.3 From f62ced3c2c85675e746bff7ef8aca5c75c9737e0 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 15:41:56 +0200 Subject: feat: use the application icon The icon was committed in a previous session and referenced nowhere: no qrc, no .desktop entry, no setWindowIcon. It is wired up now, as a window icon, a desktop entry, and install rules placing both into hicolor and share/applications. resources.qrc belongs to the executable rather than to qtmaildir_lib. A qrc compiled into a static library registers itself from a global initialiser, and the linker discards that object because nothing references it: the build succeeded, qInitResources_resources() was present in the .a, and QFile::exists(":/icons/qtmaildir.svg") still returned false at runtime. Verified loading at 16, 32 and 64 pixels after the move. Toolbar and menu actions take icons from the system theme by their standard names, so they match the rest of the desktop rather than shipping bespoke art. A theme lacking one leaves that action as text, which still works. --- CHANGELOG.md | 26 ++++++++++++-- CMakeLists.txt | 1 + README.md | 35 +++++++++++++++++++ assets/qtmaildir.desktop | 13 +++++++ .../plans/2026-08-03-post-0.1.0-usability.md | 40 +++++++++++++++++++++- src/CMakeLists.txt | 16 ++++++++- src/main.cpp | 8 +++++ src/resources.qrc | 6 ++++ 8 files changed, 141 insertions(+), 4 deletions(-) create mode 100644 assets/qtmaildir.desktop create mode 100644 src/resources.qrc (limited to 'src') diff --git a/CHANGELOG.md b/CHANGELOG.md index e0ff08b..c78cb94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,22 @@ point at which they are stable. ### Added +- Tags render as coloured chips instead of text in a column. The account tag + sits in front of the subject in the thread list, and the functional tags fill + a single row under the message pane, with anything that does not fit + collapsing into a `+N` chip whose tooltip names the rest. +- `[tagcolors]` config group. Colours resolve by exact tag first, then by + top-level prefix, so one `shopping` entry covers `shopping/amazon` and + `shopping/nike` while `shopping/amazon` can still override its own. Built-in + defaults cover the usual state tags; anything unconfigured gets a stable + colour derived from its name. +- `color` and `label` keys in an account stanza, setting the account chip's + fill and its text. `label` shortens a long key for display only and renames + nothing in notmuch; unset falls back to the key. +- The application icon is now used: window icon, a `.desktop` entry, and + install rules placing both into `hicolor` and `share/applications`. +- Toolbar and menu actions carry icons from the system theme, falling back to + text where a theme lacks one. - Menu bar covering every action: File, Edit, Message, View and Help. - Toolbar with the frequent subset, Sync, Archive, Delete and Undo. - **Help > Keyboard shortcuts**, listing the current bindings. Generated from @@ -29,8 +45,14 @@ point at which they are stable. every column. The tag change was already applied, but `Tags` sat after the stretching `Subject` column and was pushed off-screen, so Delete looked like it had done nothing. -- Thread list columns reordered to Tags, Date, From, Subject. Subject stretches - and is now last, so no column can be pushed out of view. +- Thread list columns are Date, From and Subject, all resizable. The tags + column is gone: spelling out a dozen tags per row consumed most of the list's + width. Widening past the viewport scrolls horizontally rather than squeezing + the other columns. +- Hierarchical tags in `[tagcolors]` were silently ignored. QSettings treats + `/` in a key as a group separator, so `shopping/amazon` becomes a nested key + that `childKeys()` never returns, and every tag containing a `/` fell through + to its prefix. - Three default bindings never fired. Typing a capital sends `Shift`+the key, but `N`, `F` and `G` were stored as the unshifted key, which no keystroke produces, leaving `toggle_unread`, `flag` and `sync` dead. A bare capital in diff --git a/CMakeLists.txt b/CMakeLists.txt index ed9db9c..7b73a7f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,6 +4,7 @@ project(qtmaildir VERSION 0.1.0 LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_AUTOMOC ON) +set(CMAKE_AUTORCC ON) find_package(Qt6 6.5 REQUIRED COMPONENTS Widgets WebEngineWidgets Test) diff --git a/README.md b/README.md index f7e2444..37ea3c9 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,8 @@ name = Your Name address = you@example.org maildir = work-mail ; relative to notmuch's database.path drafts = Drafts ; recorded for v2; unused today +label = W ; optional chip text; defaults to the key +color = #2f6fa8 ; optional chip colour; generated when unset [account.personal] name = Your Name @@ -98,6 +100,13 @@ address = you@example.net maildir = personal drafts = Drafts +[tagcolors] +; Optional. Colours resolve by exact tag first, then by top-level prefix, so +; one entry covers a whole hierarchy. +shopping = #3366cc ; also colours shopping/amazon, shopping/nike, ... +shopping/amazon = #ff9900 ; ... unless the exact tag overrides it +work = #cc4444 + [queries] Inbox = tag:inbox Unread = tag:unread @@ -114,6 +123,32 @@ Saved-query buttons appear in alphabetical order rather than file order: QSettings returns keys sorted, and preserving file order would mean hand-rolling an INI parser. +## Tags + +Tags render as coloured chips, and fall into two kinds. + +**Account tags** (`account-`, matching an `[account.]` stanza) say +which mailbox a thread arrived in. They appear as a chip in front of the +subject in the thread list, coloured by that account's `color` key and labelled +by its `label` key. `label` changes the chip text only; the notmuch tag is +never renamed, so queries and external tagging are unaffected. + +**Functional tags** say what state a thread is in. They fill one row under the +message pane, sorted, with whatever does not fit collapsing into a `+N` chip +whose tooltip lists the rest. Colours come from `[tagcolors]`, falling back to +built-in defaults for the usual state tags (`flagged`, `unread`, `deleted`, +`spam`, `attachment`, `replied`, and others), and finally to a colour derived +from the tag name so no chip is ever unstyled. + +Lookup is exact tag first, then top-level prefix. One `shopping` entry +therefore covers `shopping/amazon` and `shopping/nike`, while a +`shopping/amazon` entry still overrides its own. + +Note that a `/` in an INI key is a group separator to QSettings, so +`shopping/amazon = #ff9900` is stored as a nested key and written to the file +as `shopping\amazon`. It is read back correctly; the escaping is QSettings' +own. + ## Keybindings Defaults, all rebindable through `[keys]`: diff --git a/assets/qtmaildir.desktop b/assets/qtmaildir.desktop new file mode 100644 index 0000000..98e435b --- /dev/null +++ b/assets/qtmaildir.desktop @@ -0,0 +1,13 @@ +[Desktop Entry] +Type=Application +Version=1.0 +Name=qtmaildir +GenericName=Mail Reader +Comment=Read and organize a local notmuch-indexed Maildir +Exec=qtmaildir +Icon=qtmaildir +Terminal=false +Categories=Network;Email;Qt; +Keywords=mail;email;notmuch;maildir; +StartupNotify=true +StartupWMClass=qtmaildir 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 1e08bb1..915c8cb 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 @@ -44,8 +44,9 @@ taking that too literally. | 8 | No buttons or menu entries for archive, undo, etc | discoverability | M | **done** | | 9 | No in-app view of configured shortcuts | discoverability | S | **done** | | 10 | Reaching an account's inbox takes two steps | workflow | S | open | -| 11 | Icon, `.desktop` file, SlackBuild | packaging | M | open | +| 11 | Icon, `.desktop` file, SlackBuild | packaging | M | **partly done**: icon and `.desktop` landed, SlackBuild open | | 13 | No visual feedback that an action stuck | feedback | S | **done** | +| 14 | Tag column unreadable, tags need another home | presentation | M | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -398,6 +399,43 @@ and inspected: normal, unread, deleted, spam, and deleted-plus-unread rows. --- +## 14. Tag column unreadable + +**Observed:** with tags spelled out per row the column ran to 500 pixels of +mostly repeated text ("account-privateemail-danilo.macri attachment flagged +inbox passed replied"), dominated by the account prefix, and consumed most of +the list's width. + +**Cause:** presentation, not data. 96 tags in this database, many hierarchical +(`shopping/amazon`, `mailing-list/SBo`), rendered as a joined string. + +**Approach:** the column is gone. Tags now render as coloured chips in two +places, split by taxonomy: + +- The **account tag** says which mailbox a thread came from. It draws as a chip + in front of the subject, coloured and labelled from its own `[account.]` + stanza via new `color` and `label` keys. `label` is display-only; the notmuch + tag is never renamed. +- **Functional tags** say what state a thread is in. They fill a single row + under the message pane, with overflow collapsing into a `+N` chip whose + tooltip lists the hidden ones. A single row keeps the message area from + shifting between threads with different tag counts. + +Colours resolve exact tag first, then top-level prefix, so one `shopping` entry +covers the hierarchy without listing all 96. Unconfigured tags fall back to a +hash of the name, stable so a chip never changes colour as the list scrolls. + +**Defect found while building:** QSettings treats `/` in a key as a group +separator, so `shopping/amazon` becomes a nested key that `childKeys()` never +returns. Reading `[tagcolors]` with `childKeys()` silently dropped every +hierarchical tag, and each fell through to its prefix colour. Fixed by reading +`allKeys()`, with a regression test. The same gotcha is already documented in +`CLAUDE.md` for `[account.work]` section names. + +**Deferred:** clicking a chip to search that tag. Display only for now. + +--- + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7e4cea8..26cb37c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -6,6 +6,9 @@ add_library(qtmaildir_lib STATIC htmlbuilder.cpp cidschemehandler.cpp notmuchworker.cpp + tagchip.cpp + tagcolors.cpp + tagstrip.cpp threadlistmodel.cpp mailsync.cpp threadcidmap.cpp @@ -20,7 +23,18 @@ target_include_directories(qtmaildir_lib target_link_libraries(qtmaildir_lib PUBLIC Qt6::Widgets Qt6::WebEngineWidgets PkgConfig::GMIME ${NOTMUCH_LIBRARY}) -add_executable(qtmaildir main.cpp) +# resources.qrc belongs to the executable, not to the static library. A qrc +# compiled into a .a registers itself from a global initialiser, and the linker +# drops that object because nothing references it, so the resource silently +# fails to exist at runtime. +add_executable(qtmaildir main.cpp resources.qrc) target_link_libraries(qtmaildir PRIVATE qtmaildir_lib) install(TARGETS qtmaildir RUNTIME DESTINATION bin) + +# The icon goes into the hicolor theme under its scalable directory, which is +# where a desktop environment looks for the Icon= name in the .desktop entry. +install(FILES ${CMAKE_SOURCE_DIR}/assets/icons/qtmaildir.svg + DESTINATION share/icons/hicolor/scalable/apps) +install(FILES ${CMAKE_SOURCE_DIR}/assets/qtmaildir.desktop + DESTINATION share/applications) diff --git a/src/main.cpp b/src/main.cpp index 4d3ed0a..231594f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -17,6 +17,7 @@ */ #include +#include #include #include @@ -76,6 +77,13 @@ int main(int argc, char *argv[]) app.setOrganizationName(QStringLiteral("qtmaildir")); app.setApplicationVersion(QStringLiteral(QTMAILDIR_VERSION)); + // Compiled in rather than read from disk, so the icon is there whether or + // not the app was installed. setDesktopFileName() is what lets a Wayland + // compositor match the window to its .desktop entry, which is where the + // taskbar icon really comes from there. + app.setWindowIcon(QIcon(QStringLiteral(":/icons/qtmaildir.svg"))); + app.setDesktopFileName(QStringLiteral("qtmaildir")); + // Fail loudly on an ABI mismatch rather than crashing later. if (LIBNOTMUCH_MAJOR_VERSION < 5) { QMessageBox::critical(nullptr, QObject::tr("qtmaildir"), diff --git a/src/resources.qrc b/src/resources.qrc new file mode 100644 index 0000000..7bdb592 --- /dev/null +++ b/src/resources.qrc @@ -0,0 +1,6 @@ + + + + ../assets/icons/qtmaildir.svg + + -- cgit v1.2.3