diff options
| -rw-r--r-- | CHANGELOG.md | 7 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 38 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 8 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 46 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 15 | ||||
| -rw-r--r-- | src/types.h | 7 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 102 |
7 files changed, 218 insertions, 5 deletions
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 <QBrush> #include <QFont> +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 <QAbstractTableModel> +#include <QColor> #include <QVector> #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<QFont>().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<QBrush>().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<QBrush>().color(), + QColor(Qt::white)); + QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().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<QBrush>().color(), + ThreadListModel::spamColour()); + QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().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<QFont>().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<QFont>().strikeOut()); +} + void TestThreadListModel::threadIdIsReachableFromAnIndex() { // The view hands MainWindow a QModelIndex; the worker needs a thread id. |