From 1304ecf7c683a7874b8f571433379caf72e0483b Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 8 Aug 2026 11:07:03 +0200 Subject: feat(ui): mark replies with a thread line, a tint and dimmer text Indentation alone still read as a table, which was the user's original complaint about the whole item. Three cues now say the rows belong to the thread above them: a spine down the left of the expanded block with a stub out to each reply, a background tint, and text a size down and undimmed only when unread. Both colours are mixed from the palette rather than fixed, the same rule readColour follows: a tint that reads as grouping on a light theme is invisible or muddy on a dark one. The tint is deliberately near the threshold of noticing, since it sits beside the deleted and spam fills, which carry real meaning and must stay the loudest thing in the list. The spine is accumulated across the visible reply rows and drawn once after the loop. Drawn per row it left a gap at every row boundary and read as a column of dashes rather than as the structure holding the block together. Two bugs fixed here, both mine, both from the previous commit: Clicking the expander did nothing. setRootIsDecorated(false), needed to stop the style painting its own indicator under ours, also removed the style's hit area, so the glyph rendered perfectly and was inert. ThreadListView handles the press itself now, over the strip the delegate reserves, leaving the rest of the subject cell to select the row. Every click then expanded rather than toggling, because isExpanded and setExpanded are keyed on column 0 and were being asked about the subject-column index, which always answers false. Visible, clickable and toggling are three separate properties and a test for one passes against the other two being broken: the pixel test proved the triangle was drawn while it could not be clicked, and the first click test proved it opened while it could never close. The test now clicks twice and asserts open then closed. replyRowsKeepTheirTextUnderTheThreadLine covers the other trap. paintEvent runs AFTER the cells, so the first version of the tint filled the whole reply row and erased the sender and subject the delegate had just drawn: zero surviving text pixels, a block of blank tinted rows. The fill and the stub stay in the band below the text, where the tag strip lives on thread rows. --- tests/test_mainwindow.cpp | 123 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) (limited to 'tests') 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(); + QVERIFY(model); + auto *view = window.findChild(); + 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(); + QVERIFY(model); + auto *view = window.findChild(); + 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 "); + 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 -- cgit v1.2.3