diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-09 09:54:00 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-09 09:54:00 +0200 |
| commit | 955c273565521d9559e5760ca252a77c53a7aaa9 (patch) | |
| tree | 08ff85b9b46c5c823851acffa41ff11940ac6630 | |
| parent | 2e66bb8b66df016164d44bf72a2d53b8c4a67dde (diff) | |
| download | qtmaildir-955c273565521d9559e5760ca252a77c53a7aaa9.tar.gz qtmaildir-955c273565521d9559e5760ca252a77c53a7aaa9.zip | |
feat(ui): make the toolbar icon size configurable
Follow-up to item 56. With the toolbar now following the desktop's
button style, an "icon only" desktop makes the icon the whole control,
and this style reports PM_ToolBarIconSize as 16px, which is a small
target for a button with no text beside it.
A [general] toolbar_icon_size key, 16 to 64, defaulting to 24 rather
than to the style's own metric. Setting it to 16 restores the theme's
value.
Clamped and reported, unlike message_zoom, which documents a 0.5 to 3.0
range in the README and enforces none of it. Both ends here break the
UI that would be used to fix them: too small is an invisible icon, too
large is a toolbar taller than the window.
The unenforced message_zoom range is recorded as item 58 rather than
fixed here, since it is a separate defect that predates this change.
Also documents in the README that saved-query button labels are the key
names from the user's own [queries] section, which is why the "Flagged"
button still read that way after the action was renamed: it is a user's
query name, not a string this code owns. The sample config now shows
`Important = tag:flagged` to teach the wording the UI uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | README.md | 11 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 34 | ||||
| -rw-r--r-- | src/config.cpp | 38 | ||||
| -rw-r--r-- | src/config.h | 13 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 8 | ||||
| -rw-r--r-- | tests/test_config.cpp | 83 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 29 |
8 files changed, 219 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 99de25c..444ef93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ point at which they are stable. - **An optional `[sync] log` key**, naming the sync script's log file. It defaults to where `assets/mailsync.sh` writes, and only needs setting if you changed the script's `LOGFILE`. +- **An optional `[general] toolbar_icon_size` key**, 16 to 64 pixels, defaulting + to 24. Most styles report 16, which is a small target now that the toolbar can + be icons only. Out-of-range values are clamped and reported rather than + applied. ### Changed @@ -110,6 +110,11 @@ identity. ; Unread. Falls back to the first saved query if no query by this name ; exists, and warns if you named one explicitly. ; startup_query = Unread +; Optional. Toolbar icon size in pixels, 16 to 64. Defaults to 24. The +; toolbar follows your desktop's toolbar button style, so if that is set to +; "icon only" this is the whole size of the control; 16 matches what most +; styles report, which is small for a button with no text beside it. +; toolbar_icon_size = 24 ; Optional. Open the completion popup as soon as an empty query bar takes ; focus, without pressing the shortcut. Defaults to false. ; completion_on_focus = false @@ -173,7 +178,7 @@ work = #cc4444 [queries] Inbox = tag:inbox Unread = tag:unread -Flagged = tag:flagged +Important = tag:flagged [keys] Ctrl+E = archive @@ -187,6 +192,10 @@ QSettings returns keys sorted, and preserving file order would mean hand-rolling an INI parser. Which query opens at startup is therefore a separate setting, `[general] startup_query`, rather than "the first one". +The button text is the key you write here, so these names are yours to +choose. `Important = tag:flagged` and `Flagged = tag:flagged` run the same +query and differ only in what the button says. + ## The query bar The bar at the top takes a notmuch query and shows the matching threads. 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 52cc023..ba0ade5 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 @@ -115,6 +115,7 @@ taking that too literally. | 55 | In a narrow window the message pane is invisible | presentation | XS | open | | 56 | No action carries an icon, so the toolbar reserves space for nothing | presentation | S | **done** | | 57 | "Flag" would read better as "Important" or "Starred" | presentation | XS | **done** | +| 58 | `message_zoom` documents a 0.5 to 3.0 range and enforces none of it | correctness | XS | open | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -3542,6 +3543,39 @@ guard proving the thread did not already carry it. Mutating the tag to `important` fails it, and also fails two pre-existing held-edit tests, which is independent confirmation that `flagged` is load-bearing across the suite. +## 58. `message_zoom` documents a 0.5 to 3.0 range and enforces none of it + +**Observed:** not by the user. Found on 2026-08-09 while adding +`toolbar_icon_size`, by reading `message_zoom` as the model for a bounded +numeric key and noticing it is not bounded. + +**Cause, verified in code.** `src/config.cpp:79-90` parses the value with +`toDouble()` and assigns it on success. The parse failure is reported, but +nothing checks the range. The README documents "0.5 to 3.0" (`README.md`, the +`[general]` block), and `m_messageZoom` is never clamped anywhere else: +`src/mainwindow.cpp:145` passes it straight to the saved-state default. + +So `message_zoom = 500` is accepted and applied. Unlike the toolbar icon size, +this one is recoverable, since the zoom is adjustable from the UI and the +adjusted value is what gets saved, but the first render is unusable. + +**Approach.** Clamp with a report, exactly as `toolbar_icon_size` does +(`src/config.cpp`, the `kMinToolbarIconSize`/`kMaxToolbarIconSize` block). Both +ends already have documented bounds, so this is applying the README's own +numbers. + +**Constraints.** + +- Report rather than silently clamp. Silence is how this went unnoticed: the + key parses, so nothing ever said the value was not being honoured. +- Do not extend this to `mark_read_delay_ms`, whose zero and negative values are + meaningful and deliberately unclamped (`src/config.cpp:96-102`). + +**Verification.** `test_config` already has the pattern in +`toolbarIconSizeIsClampedAndReported`; mirror it. Note that +`messageZoomDefaultsAndValidates` exists and passes today, so it is asserting +only on the parse and not on the range. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/src/config.cpp b/src/config.cpp index 12c5632..b54d5c6 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -24,6 +24,16 @@ #include <QSettings> #include <QStandardPaths> +namespace { + +/// Bounds for [general] toolbar_icon_size. 16 is the smallest size the icon +/// themes actually ship art for, and is what this desktop's style reports; +/// above 64 the toolbar is taller than the thread rows it sits over. +constexpr int kMinToolbarIconSize = 16; +constexpr int kMaxToolbarIconSize = 64; + +} // namespace + QString Account::scopedQuery(const QString &query) const { const QString prefix = QStringLiteral("path:\"%1/**\"").arg(maildir); @@ -93,6 +103,34 @@ void Config::load(const QString &path) m_completionOnFocus = settings.value(QStringLiteral("completion_on_focus"), false).toBool(); + // Clamped, unlike message_zoom above, which documents a 0.5 to 3.0 range in + // the README and enforces none of it. Both ends here are unrecoverable from + // the UI they break: too small is an invisible icon, too large is a toolbar + // taller than the window, and in either case the control the user would + // reach for to fix it is the one that just broke. + const QVariant iconSize = settings.value(QStringLiteral("toolbar_icon_size")); + if (iconSize.isValid()) { + bool ok = false; + const int value = iconSize.toString().trimmed().toInt(&ok); + if (!ok) { + addProblem(QStringLiteral("Toolbar icon size '%1' is not a number; " + "using %2.") + .arg(iconSize.toString()) + .arg(m_toolbarIconSize)); + } else if (value < kMinToolbarIconSize || value > kMaxToolbarIconSize) { + m_toolbarIconSize = + qBound(kMinToolbarIconSize, value, kMaxToolbarIconSize); + addProblem(QStringLiteral("Toolbar icon size %1 is outside %2 to " + "%3; using %4.") + .arg(value) + .arg(kMinToolbarIconSize) + .arg(kMaxToolbarIconSize) + .arg(m_toolbarIconSize)); + } else { + m_toolbarIconSize = value; + } + } + // Absent is silent, the default being 2000. Present but unparseable warns, // for the same reason message_zoom does: the user asked for something and // is not getting it. diff --git a/src/config.h b/src/config.h index 3174a03..3dd9011 100644 --- a/src/config.h +++ b/src/config.h @@ -90,6 +90,18 @@ public: /// Empty when unset; the caller disables the Sync button in that case. QString syncCommand() const { return m_syncCommand; } + /// Toolbar icon size in pixels, 16 to 64, defaulting to 24. + /// + /// The desktop's own PM_ToolBarIconSize was the obvious default and was + /// rejected empirically: it reports 16 here, which is a small target now + /// that the toolbar follows the platform's "icon only" style and the icon + /// is the whole control. Setting this to 16 restores the theme's value. + /// + /// Clamped rather than trusted: a 4px icon is invisible and a 4000px one + /// makes the toolbar taller than the window, and neither is recoverable + /// from the UI the value just broke. + int toolbarIconSize() const { return m_toolbarIconSize; } + /// The sync script's log file, read to learn the outcome of a sync this /// process did not start (item 54). /// @@ -171,6 +183,7 @@ private: QList<SavedQuery> m_savedQueries; QString m_syncCommand; QString m_syncLog; + int m_toolbarIconSize = 24; QString m_notmuchConfig; qreal m_messageZoom = 1.0; bool m_completionOnFocus = false; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 865c691..a176d71 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -988,6 +988,14 @@ void MainWindow::buildMenus() // which would ignore the setting just as thoroughly in the other direction. toolBar->setToolButtonStyle(static_cast<Qt::ToolButtonStyle>( style()->styleHint(QStyle::SH_ToolButtonStyle, nullptr, toolBar))); + + // Set explicitly rather than left to the style. With the button style above + // resolving to icon-only on this desktop, the icon IS the control, and this + // style's PM_ToolBarIconSize is 16px, which is a small target for it. + // Configurable because the right answer depends on the display, not on + // anything this code can see. + const int iconSize = m_config.toolbarIconSize(); + toolBar->setIconSize(QSize(iconSize, iconSize)); QAction *syncAction = m_actions.value(QStringLiteral("sync")); // Carried over from the QPushButton this replaced: with no command // configured the control is disabled, and the tooltip is the only thing diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 079ba3b..ec630d4 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -31,6 +31,10 @@ private slots: void missingSyncCommandIsEmpty(); void syncLogDefaultsToTheScriptsOwnPath(); void syncLogCanBeOverridden(); + void toolbarIconSizeDefaultsTo24(); + void toolbarIconSizeIsActuallyRead(); + void toolbarIconSizeIsClampedAndReported(); + void toolbarIconSizeRejectsGarbage(); void accountWithoutMaildirIsRejected(); void scopedQueryWrapsCorrectly(); void absentSyncCommandIsNoticeNotProblem(); @@ -170,6 +174,85 @@ void TestConfig::syncLogCanBeOverridden() QCOMPARE(config.syncLog(), QStringLiteral("/var/log/mail/sync.log")); } +void TestConfig::toolbarIconSizeDefaultsTo24() +{ + // The desktop's own metric is the obvious default and was rejected: this + // style reports PM_ToolBarIconSize as 16, which is a small click target for + // a toolbar that now shows icons with no text beside them. 24 is a normal + // toolbar size, and setting the key back to 16 restores the theme's value. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral("[general]\n")); + + Config config; + config.load(path); + + QCOMPARE(config.toolbarIconSize(), 24); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::toolbarIconSizeIsActuallyRead() +{ + // [general] keys are read WITHOUT the general/ prefix, per the note at the + // top of Config::load(). A key that silently matched nothing would leave + // the default in place and look exactly like a working default. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "toolbar_icon_size = 32\n" + )); + + Config config; + config.load(path); + + QCOMPARE(config.toolbarIconSize(), 32); +} + +void TestConfig::toolbarIconSizeIsClampedAndReported() +{ + // Out of range is clamped rather than honoured: a 4px icon is invisible and + // a 4000px one makes the toolbar taller than the window, and neither is + // recoverable from the UI the value just broke. Reported, because silently + // ignoring what the user asked for is how message_zoom's documented 0.5-3.0 + // range came to be unenforced without anyone noticing. + QTemporaryDir dir; + const QString tooBig = writeIni(dir, QStringLiteral( + "[general]\n" + "toolbar_icon_size = 4000\n" + )); + + Config big; + big.load(tooBig); + QCOMPARE(big.toolbarIconSize(), 64); + QVERIFY(!big.warnings().isEmpty() || !big.problems().isEmpty()); + + QTemporaryDir dir2; + const QString tooSmall = writeIni(dir2, QStringLiteral( + "[general]\n" + "toolbar_icon_size = 2\n" + )); + + Config small; + small.load(tooSmall); + QCOMPARE(small.toolbarIconSize(), 16); +} + +void TestConfig::toolbarIconSizeRejectsGarbage() +{ + // Unparseable falls back to the default and says so, matching how + // mark_read_delay_ms treats the same mistake. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "toolbar_icon_size = enormous\n" + )); + + Config config; + config.load(path); + + QCOMPARE(config.toolbarIconSize(), 24); + QVERIFY(!config.problems().isEmpty()); +} + void TestConfig::accountWithoutMaildirIsRejected() { QTemporaryDir dir; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 4a911c9..84a3a7d 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -120,6 +120,7 @@ private slots: void theToolbarDoesNotOverrideTheDesktopButtonStyle(); void theImportantActionIsLabelledImportant(); void theImportantActionStillWritesTheFlaggedTag(); + void theToolbarUsesTheConfiguredIconSize(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -2513,6 +2514,34 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag() "written, which no other tool reading this Maildir knows"); } +void TestMainWindow::theToolbarUsesTheConfiguredIconSize() +{ + // With the toolbar following the desktop's "Icon only" style, the icons are + // the whole control, and this style reports 16px, which is a small target. + // The size is configurable with a 24px default; this proves the config + // value actually reaches the widget rather than sitting in Config unread. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("[general]\ntoolbar_icon_size = 40\n"); + file.close(); + + Config config; + config.load(path); + QCOMPARE(config.toolbarIconSize(), 40); + + MainWindow window(config); + auto *toolBar = window.findChild<QToolBar *>(QStringLiteral("main_toolbar")); + QVERIFY(toolBar); + + // 40 is deliberately not any of this style's own metrics (16 small, 32 + // large), so the assertion cannot pass by the widget happening to agree + // with the theme. + QCOMPARE(toolBar->iconSize(), QSize(40, 40)); +} + // 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. |
