diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-13 19:29:18 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-13 19:29:18 +0200 |
| commit | 14842cd136e45c551dde4f276af1176bfdf22023 (patch) | |
| tree | 77c7d5a14884a4a9f59585930c136cfae54dedf7 | |
| parent | 0c5eea8f0d0ccc5b8eb6220814c9e212d6c1ccc2 (diff) | |
| download | qtmaildir-14842cd136e45c551dde4f276af1176bfdf22023.tar.gz qtmaildir-14842cd136e45c551dde4f276af1176bfdf22023.zip | |
fix(queries): put the Save query button beside the query bar
The spec asked for "a Save query button beside the search bar" and what shipped
was a menu entry and Ctrl+S. The user went looking for the button where the
design said it would be and did not find it.
Saving is a thing you decide on while looking at the results, so it belongs
where the results came from rather than behind a menu or a remembered chord.
The button takes the action through setDefaultAction rather than a second
connect, so it inherits the text, icon, tooltip and enabled state and cannot end
up offering to save an empty query while the menu entry correctly refuses. The
mutation that replaces it with a plain clicked() connection fails the test.
Also records item 82: a saved query cannot be edited, unpinned or deleted from
the UI. Item 23 specified saving and nothing else, and that is exactly what was
built, so the only way to unpin a query is a text editor or retyping it in full
under the same name. An action that creates something the UI cannot then change
or remove is incomplete, and this was found within minutes of the first hand
test. It is filed as a defect rather than an enhancement, and the spec now says
so where a reader would otherwise take the design for complete.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -rw-r--r-- | CHANGELOG.md | 9 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 44 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-13-saved-queries-design.md | 11 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 17 | ||||
| -rw-r--r-- | src/mainwindow.h | 3 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 34 |
6 files changed, 114 insertions, 4 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d264e2..428e3e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,11 @@ point at which they are stable. ### Added -- A **Save query** action (`Ctrl+S`) keeps the query in the bar as a saved - query, naming it, optionally scoping it to one account, and choosing whether - it appears as a button or in a menu. Saved queries no longer have to be added - by hand-editing the config file (item 23). +- A **Save query** button beside the query bar, also on the Edit menu and bound + to `Ctrl+S`, keeps the query in the bar as a saved query: naming it, + optionally scoping it to one account, and choosing whether it appears as a + button or in a menu. Saved queries no longer have to be added by + hand-editing the config file (item 23). - Saved queries live in `~/.config/qtmaildir/queries.json`, which carries their **order**, a `pinned` flag and an optional account scope. The order in the file is the order the buttons appear in, so rearranging them is a matter of 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 268dbb4..9d19e4d 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 @@ -146,6 +146,7 @@ taking that too literally. | 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13 on `rule-builder`, unreleased. Follows item 76 | | 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand | | 81 | No way to turn a saved query into a tagging rule | workflow | S | open; depends on 23, which builds the dialog, and is excluded from its spec on purpose. Writes to the shared rules file, so it spans this repo and `mailctl` | +| 82 | A saved query cannot be edited, unpinned or deleted from the UI | defect | S | open; found by hand-testing item 23 on 2026-08-13. Saving works, unsaving does not | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -574,6 +575,49 @@ failing silently. **Size: S** on top of 23, and not meaningful before it. +## 82. A saved query cannot be edited, unpinned or deleted from the UI + +**Observed (user, 2026-08-13):** hand-testing item 23. The user saved a query, +then asked how to unpin it, and there is no answer that does not involve either +a text editor or retyping the whole query. + +**Cause:** item 23 specified saving and nothing else, and that is exactly what +shipped. `SaveQueryDialog` opens on the contents of the query BAR, not on a +stored entry, so the only route to changing one field of an existing query is to +reconstruct the whole query, name it identically, and let +`MainWindow::saveCurrentQuery()` replace it by name. There is no delete at any +price: nothing in the UI removes an entry from `queries.json`. + +This is a defect rather than a missing enhancement. An action that creates +something the UI cannot then edit or remove is incomplete, and the user hit it +within minutes of the first hand test. + +**Approach.** A context menu on a saved-query button and on each **More +queries** entry, offering Edit, Unpin (or Pin) and Delete. + +- **Edit** opens `SaveQueryDialog` prefilled from the STORED entry rather than + from the query bar. The dialog already carries every field it needs; what it + lacks is a constructor that takes a `SavedQuery`. +- **Unpin** is a one-field write and does not need the dialog at all. +- **Delete** removes the entry and rewrites the file. + +**Constraints.** + +- `saveCurrentQuery()` already merges an existing entry's `unknown` fields over + the dialog's fresh value, and every one of these paths must do the same or a + field written by a later build is dropped by an edit here. +- Renaming through Edit is a rename, not a second entry: match on the name the + dialog was OPENED with, not the one it returns, or renaming silently creates a + duplicate and leaves the original behind. +- Delete is destructive and the file is user config, so it is one of the few + places in this application that wants a confirmation. The no-confirmation rule + in CLAUDE.md is about tag mutations, which are undoable through the undo + stack; this is not on that stack and cannot be undone. +- A test must exercise every route the way item 75's did not: the dialog's + Cancel goes through `done(int)` and never sends a `QCloseEvent`. + +**Size: S**, and it should land before the saved-query work is called done. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/docs/superpowers/specs/2026-08-13-saved-queries-design.md b/docs/superpowers/specs/2026-08-13-saved-queries-design.md index 6d0019f..ba6b23e 100644 --- a/docs/superpowers/specs/2026-08-13-saved-queries-design.md +++ b/docs/superpowers/specs/2026-08-13-saved-queries-design.md @@ -160,6 +160,17 @@ Sent stays where it is. It is not a saved query, it is built from would mean generating a per-account path query into stored config, which is the duplication the `account`-key decision just rejected. +## Editing and deleting are NOT here, and that is a defect + +This document specifies creating a saved query and says nothing about changing +or removing one. That gap shipped: the first hand test produced "how do I unpin +a query?", and the honest answer was a text editor. Recorded as **item 82**, +sized S, and it should land before this work is called finished. + +Anything built there must merge the stored entry's `unknown` fields the way +`saveCurrentQuery()` does, and must match a rename on the name the dialog was +opened with rather than the one it returns. + ## What is deliberately not here **Item 81, saving a query as a tagging rule.** A saved query is a view and costs diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 3966e9f..9eff3d5 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -45,6 +45,7 @@ #include <QScrollBar> #include <QTimer> #include <QToolBar> +#include <QToolButton> #include <QVBoxLayout> #include "mailsync.h" @@ -348,6 +349,12 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) // also set programmatically, by the saved-query buttons and by // recoverStaleThread(), and the action must track those too. if (QAction *save = m_actions.value(QStringLiteral("save_query"))) { + // setDefaultAction, not a second connect: the button then takes the + // action's text, icon, tooltip and ENABLED state, so it cannot end up + // offering to save an empty query while the menu entry refuses. + m_saveQueryButton->setDefaultAction(save); + m_saveQueryButton->setToolButtonStyle(Qt::ToolButtonIconOnly); + auto updateSaveState = [this, save]() { save->setEnabled(!m_queryEdit->text().trimmed().isEmpty()); }; @@ -566,6 +573,16 @@ void MainWindow::buildUi() queryRow->addWidget(m_accountBox); queryRow->addWidget(m_sortOrder); queryRow->addWidget(m_queryEdit, 1); + + // Beside the field, where a user looks for it. The menu entry and Ctrl+S + // were not enough on their own: saving is a thing you decide on while + // looking at the results, so it needs to be visible at the query bar + // rather than remembered. Created here and given its action in the + // constructor, since registerActions() has not run yet. + m_saveQueryButton = new QToolButton(central); + m_saveQueryButton->setObjectName(QStringLiteral("saveQueryButton")); + queryRow->addWidget(m_saveQueryButton); + layout->addLayout(queryRow); buildSavedQueryRow(central, layout); diff --git a/src/mainwindow.h b/src/mainwindow.h index 8b43fd0..e6ee322 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -49,6 +49,7 @@ class QPlainTextEdit; class QSplitter; class QProgressBar; class QTimer; +class QToolButton; class QVBoxLayout; class ThreadListModel; @@ -625,6 +626,8 @@ private: QUndoStack m_undoStack; QLineEdit *m_queryEdit = nullptr; + /// Save query, beside the field. Driven by the save_query action. + QToolButton *m_saveQueryButton = nullptr; QueryCompleter *m_queryCompleter = nullptr; /// Its own type, not the QTreeView base. The strip painting and the /// expander column are ThreadListView's, and holding the base here only diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 2c70e0f..581daae 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -195,6 +195,7 @@ private slots: void aScopedSavedQuerySelectsItsAccount(); void anUnscopedSavedQueryClearsTheAccount(); void theSaveQueryActionIsDisabledOnAnEmptyQuery(); + void thereIsASaveButtonBesideTheQueryBar(); private: /// Owns the throwaway lock table init() points every test at. A pointer @@ -5540,4 +5541,37 @@ void TestMainWindow::theSaveQueryActionIsDisabledOnAnEmptyQuery() QVERIFY(!save->isEnabled()); } +/// A menu entry and a shortcut are not a button. The spec asks for one beside +/// the query bar, and the user went looking for it there and did not find it. +void TestMainWindow::thereIsASaveButtonBesideTheQueryBar() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, "queries": [] + })")); + + MainWindow window(config); + auto *button = + window.findChild<QAbstractButton *>(QStringLiteral("saveQueryButton")); + QVERIFY2(button, "no Save query button beside the query bar"); + + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + // In the query row itself, not somewhere else in the window that a + // findChild would also reach. + QCOMPARE(button->parentWidget(), queryEdit->parentWidget()); + + // Follows the action, so it cannot offer to save an empty query while the + // menu entry correctly refuses. + queryEdit->clear(); + QVERIFY2(!button->isEnabled(), + "the button must follow the action's enabled state"); + queryEdit->setText(QStringLiteral("tag:inbox")); + QVERIFY(button->isEnabled()); +} + #include "test_mainwindow.moc" |
