diff options
| -rw-r--r-- | src/threadlistmodel.cpp | 64 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 11 | ||||
| -rw-r--r-- | src/threadlistview.cpp | 109 | ||||
| -rw-r--r-- | src/threadlistview.h | 10 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 123 |
5 files changed, 311 insertions, 6 deletions
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 043e982..f9efee7 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -75,6 +75,46 @@ QString ThreadListModel::flagGlyph() return glyph; } +QColor ThreadListModel::replyBackground() +{ + // Mixed from the palette rather than fixed, for the same reason as + // readColour: a tint that reads as "grouped" on a light theme is either + // invisible or muddy on a dark one. + // + // Toward Text rather than toward a hue, so it darkens on a light theme and + // lightens on a dark one without picking a colour that means something + // else. 0.07 is deliberately near the threshold of noticing: it is a + // grouping cue sitting beside the deleted and spam fills, which carry + // actual meaning and must stay the loudest thing in the list. + const QPalette palette = QGuiApplication::palette(); + const QColor base = palette.color(QPalette::Base); + const QColor text = palette.color(QPalette::Text); + + constexpr qreal kWeight = 0.07; + const qreal inverse = 1.0 - kWeight; + return QColor::fromRgbF( + text.redF() * kWeight + base.redF() * inverse, + text.greenF() * kWeight + base.greenF() * inverse, + text.blueF() * kWeight + base.blueF() * inverse); +} + +QColor ThreadListModel::threadLineColour() +{ + // Stronger than the tint, weaker than the text: the line is structure, so + // it has to be followable down a long expansion without competing with the + // senders beside it. + const QPalette palette = QGuiApplication::palette(); + const QColor base = palette.color(QPalette::Base); + const QColor text = palette.color(QPalette::Text); + + constexpr qreal kWeight = 0.35; + const qreal inverse = 1.0 - kWeight; + return QColor::fromRgbF( + text.redF() * kWeight + base.redF() * inverse, + text.greenF() * kWeight + base.greenF() * inverse, + text.blueF() * kWeight + base.blueF() * inverse); +} + QColor ThreadListModel::readColour() { // Derived from the palette, never hardcoded: a fixed grey that reads as @@ -248,9 +288,29 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const default: return {}; } + case Qt::BackgroundRole: + // Tinted, so an expanded thread reads as one block rather than as + // more table rows. Applied per cell here; ThreadListView fills the + // same colour across the strip's band so the row does not end up + // half tinted. + return replyBackground(); + case Qt::FontRole: { + // A size down from the thread rows, so a thread reads as the + // heading and its replies as the contents. Never bold: an unread + // reply is still subordinate to the thread it belongs to, and the + // thread row above already carries the unread cue for the whole + // conversation. + QFont font = QGuiApplication::font(); + if (font.pointSize() > 0) + font.setPointSize(qMax(6, font.pointSize() - 1)); + else if (font.pixelSize() > 0) + font.setPixelSize(qMax(8, font.pixelSize() - 2)); + return font; + } case Qt::ForegroundRole: - // Same rule as a thread row: read recedes, unread stays at the - // palette's own colour. + // Dimmed whether read or not, for the same reason as the font: a + // reply is subordinate content. An unread one is left undimmed so + // it can still be found. return node.isUnread() ? QVariant() : QVariant(readColour()); default: return {}; diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 700b3a6..c56e80a 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -120,6 +120,17 @@ public: static QColor deletedColour(); static QColor spamColour(); + /// Background for a reply row, so an expanded thread reads as one block + /// rather than as more table rows. + /// + /// Derived from the palette and deliberately subtle: it marks a grouping, + /// and a tint strong enough to notice on its own would compete with the + /// deleted and spam row colours, which carry real meaning. + static QColor replyBackground(); + + /// The line drawn down the left of an expanded thread's replies. + static QColor threadLineColour(); + /// The dimmed text colour a READ thread carries. /// /// Unread rows are left at the palette's own colour and read ones recede, diff --git a/src/threadlistview.cpp b/src/threadlistview.cpp index ebca4cc..1ffec04 100644 --- a/src/threadlistview.cpp +++ b/src/threadlistview.cpp @@ -21,10 +21,44 @@ #include "tagchip.h" #include "threadlistmodel.h" +#include <QMouseEvent> #include <QPaintEvent> #include <QPainter> #include <QScrollBar> +void ThreadListView::mousePressEvent(QMouseEvent *event) +{ + const QModelIndex index = indexAt(event->pos()); + + // Only a thread row, only the subject column, only the strip the delegate + // reserved for the glyph. Anything wider would swallow clicks meant to + // select the row, which is what the rest of the subject cell is for. + if (event->button() == Qt::LeftButton && index.isValid() + && !index.parent().isValid() + && index.column() == ThreadListModel::SubjectColumn + && index.data(ThreadListModel::HasRepliesRole).toBool()) { + + const QRect rect = visualRect(index); + if (event->pos().x() >= rect.left() + && event->pos().x() < rect.left() + SubjectDelegate::kExpanderWidth) { + // Column 0, not the clicked index. Expansion state belongs to the + // ROW, and QTreeView keys it on the first column: asking + // isExpanded() about the subject-column index always answers false, + // so every click expanded again instead of toggling. + const QModelIndex row = index.siblingAtColumn(0); + setExpanded(row, !isExpanded(row)); + + // Swallowed, so the click that opened a thread does not also load + // it into the message pane: expanding is a request to see the + // thread's shape, not to read it. + event->accept(); + return; + } + } + + QTreeView::mousePressEvent(event); +} + void ThreadListView::paintEvent(QPaintEvent *event) { QTreeView::paintEvent(event); @@ -55,16 +89,74 @@ void ThreadListView::paintEvent(QPaintEvent *event) // the same one. int visualRow = 0; + // The spine's extent, collected across the reply rows and drawn once after + // the loop. Per-row segments leave a gap at every row boundary and read as + // a column of dashes rather than as one line. + int spineX = -1; + int spineTop = std::numeric_limits<int>::max(); + int spineBottom = std::numeric_limits<int>::min(); + for (; walk.isValid(); walk = indexBelow(walk), ++visualRow) { const QRect rowRect = visualRect(walk); if (rowRect.top() > viewport()->height()) break; - // No strip under a message row. The strip carries the THREAD's tags, so - // one under each reply would stripe the list and repeat identical tags - // down the whole expansion. - if (walk.parent().isValid()) + // A message row: no tag strip, but it does get the band filled to its + // own tint and a thread line down its left. + // + // The band has to be filled here for the same reason a thread row's is. + // The cells paint the tint per cell, so nothing covers the width to the + // right of the last column or the lower band the strip normally + // occupies, and an untouched reply row comes out tinted across its text + // and bare underneath it. + if (walk.parent().isValid()) { + // Only the band BELOW the text, never the whole row. paintEvent + // runs after the cells, so filling the row's full height paints + // over the sender and subject the delegate just drew: measured at + // zero surviving text pixels, a block of blank tinted rows. + const int bandTop = rowRect.top() + SubjectDelegate::kRowPadding + + rowMetrics.height(); + const QRect band(columnViewportPosition(ThreadListModel::DateColumn), + bandTop, + viewport()->width() + - columnViewportPosition( + ThreadListModel::DateColumn), + rowRect.bottom() - bandTop + 1); + + if (selectionModel() + && selectionModel()->isSelected( + walk.siblingAtColumn(ThreadListModel::SubjectColumn))) { + painter.fillRect(band, palette().brush(QPalette::Highlight)); + } else { + painter.fillRect(band, ThreadListModel::replyBackground()); + } + + // The spine is NOT drawn here. Drawing it per row leaves a gap + // wherever consecutive rows do not abut exactly, which is every row + // boundary once the rows carry padding: the result reads as a + // column of dashes rather than as one line. It is drawn as a single + // continuous run after this loop, from the collected extents below. + const int subjectLeft = + columnViewportPosition(ThreadListModel::SubjectColumn); + const int lineX = subjectLeft + SubjectDelegate::kExpanderWidth / 2; + + if (spineX < 0) + spineX = lineX; + spineTop = qMin(spineTop, rowRect.top()); + spineBottom = qMax(spineBottom, rowRect.bottom() + 1); + + // A stub out to the row, so each reply is visibly attached to the + // spine rather than merely beside it. Drawn in the LOWER band, not + // at the row's midpoint: the midpoint crosses the sender text, and + // this paints after the cells. + const int stubY = bandTop + (rowRect.bottom() - bandTop) / 2; + painter.setPen(QPen(ThreadListModel::threadLineColour(), 2)); + painter.drawLine(lineX, stubY, + subjectLeft + SubjectDelegate::kReplyIndent + - TagChip::kSpacing * 2, + stubY); continue; + } const QModelIndex index = walk.siblingAtColumn( ThreadListModel::SubjectColumn); @@ -156,4 +248,13 @@ void ThreadListView::paintEvent(QPaintEvent *event) x += size.width() + TagChip::kSpacing; } } + + // One continuous spine over every visible reply row, drawn last so no + // cell fill can break it. Segments drawn per row left a dash at every row + // boundary, which read as a dotted decoration rather than as the structure + // holding the block together. + if (spineX >= 0 && spineBottom > spineTop) { + painter.setPen(QPen(ThreadListModel::threadLineColour(), 2)); + painter.drawLine(spineX, spineTop, spineX, spineBottom); + } } diff --git a/src/threadlistview.h b/src/threadlistview.h index 9e0da28..0ebe3ea 100644 --- a/src/threadlistview.h +++ b/src/threadlistview.h @@ -57,4 +57,14 @@ public: protected: void paintEvent(QPaintEvent *event) override; + /// Toggles a thread when its expander glyph is clicked. + /// + /// The view owns this because the glyph is drawn by SubjectDelegate and a + /// delegate gets no click of its own without an editor. Being VISIBLE and + /// being CLICKABLE are separate properties: setRootIsDecorated(false), + /// needed to stop the style drawing its own indicator underneath ours, also + /// removed the style's hit area, so the expander painted correctly and did + /// nothing at all. + void mousePressEvent(QMouseEvent *event) override; + }; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 474d856..cee32c6 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -99,6 +99,8 @@ private slots: void childRowsAreIndentedUnderTheirThread(); void aThreadWithRepliesDrawsAVisibleExpander(); void noTagStripIsPaintedUnderAMessageRow(); + void replyRowsKeepTheirTextUnderTheThreadLine(); + void clickingTheExpanderTogglesTheThread(); void markAllReadIsDisabledUntilTheQueryFinishes(); void markAllReadActsOnEveryRowAndUndoesInOneStep(); void markAllReadDoesNothingWhenNothingIsUnread(); @@ -798,6 +800,127 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander() QCOMPARE(control, 0); } +void TestMainWindow::clickingTheExpanderTogglesTheThread() +{ + // The glyph being VISIBLE and the glyph being CLICKABLE are separate + // properties, and the pixel test for the first passes happily against a + // triangle nothing can hit. Turning off rootIsDecorated to stop the style + // drawing its own dot under ours also removed the style's hit area, so the + // expander rendered perfectly and did nothing. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread( + QStringLiteral("t1"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); + t.totalCount = 3; + model->appendBatch({ t }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + QApplication::processEvents(); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + const QModelIndex subject = + model->index(0, ThreadListModel::SubjectColumn, QModelIndex()); + const QRect rect = view->visualRect(subject); + + // Guards: the row is drawn, it claims to have replies, and it starts + // collapsed. Without the last one a toggle test can pass by doing nothing. + QVERIFY2(rect.height() > 0, "the thread row is not on screen"); + QVERIFY(model->data(subject, ThreadListModel::HasRepliesRole).toBool()); + QVERIFY(!view->isExpanded(root)); + + // Aimed at the glyph itself: the delegate reserves kExpanderWidth at the + // left of the subject cell and centres the triangle in it. + const QPoint hit(rect.left() + SubjectDelegate::kExpanderWidth / 2, + rect.top() + SubjectDelegate::kRowPadding + + QFontMetrics(view->font()).height() / 2); + + QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit); + QApplication::processEvents(); + QVERIFY2(view->isExpanded(root), + "clicking the expander did not open the thread"); + + QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit); + QApplication::processEvents(); + QVERIFY2(!view->isExpanded(root), + "clicking the expander again did not close the thread"); +} + +void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine() +{ + // paintEvent runs AFTER the cells, so anything it fills across a reply row + // covers the text the delegate just drew. The tint and the thread line are + // both painted there, which makes this the obvious way to ship a block of + // blank rows. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread( + QStringLiteral("t1"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); + t.totalCount = 2; + model->appendBatch({ t }); + + MessageNode first; + first.messageId = QStringLiteral("m0@example.org"); + first.threadId = QStringLiteral("t1"); + first.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.from = QStringLiteral("A Replier <replier@example.org>"); + reply.subject = QStringLiteral("Re: a subject"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { first, reply }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + view->expand(root); + QApplication::processEvents(); + + const QModelIndex child = + model->index(0, ThreadListModel::AuthorsColumn, root); + const QRect rect = view->visualRect(child); + QVERIFY2(rect.height() > 0, "the reply row is not on screen"); + + QImage shot(view->viewport()->size(), QImage::Format_ARGB32); + shot.fill(Qt::transparent); + view->viewport()->render(&shot); + + // Count pixels in the sender cell that differ from the row's own tint. + // Text is the only thing that can produce them. + const QRgb tint = ThreadListModel::replyBackground().rgb() | 0xff000000; + int textPixels = 0; + for (int y = rect.top(); y < qMin(rect.bottom(), shot.height()); ++y) { + for (int x = rect.left(); x < qMin(rect.right(), shot.width()); ++x) { + if ((shot.pixel(x, y) | 0xff000000) != tint) + ++textPixels; + } + } + + QVERIFY2(textPixels > 20, + qPrintable(QStringLiteral("only %1 non-background pixels in the " + "reply's sender cell: the row was " + "painted over after its text was drawn") + .arg(textPixels))); +} + void TestMainWindow::noTagStripIsPaintedUnderAMessageRow() { // The strip is a row-wide band of the THREAD's tags. Painted under every |
