diff options
| -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" |