diff options
| -rw-r--r-- | src/querycompleter.cpp | 33 | ||||
| -rw-r--r-- | tests/test_querycompleter.cpp | 95 |
2 files changed, 121 insertions, 7 deletions
diff --git a/src/querycompleter.cpp b/src/querycompleter.cpp index 7e9dfd9..9848faf 100644 --- a/src/querycompleter.cpp +++ b/src/querycompleter.cpp @@ -88,10 +88,10 @@ int tokenEnd(const QString &text, int cursor) /// Draws the description greyed and right-aligned beside the value. /// -/// The two are drawn into disjoint halves of the row rather than simply -/// painted on top of each other: a long value ("application/vnd.oasis..." -/// exceeds the popup width on its own) would otherwise run underneath the -/// description and render both unreadable. +/// The two are drawn into disjoint spans of the row rather than simply painted +/// on top of each other: a long value ("application/vnd.oasis..." exceeds the +/// popup width on its own) would otherwise run underneath the description and +/// render both unreadable. The split is not even, see paint(). class CompletionDelegate : public QStyledItemDelegate { public: @@ -111,11 +111,30 @@ public: const int gap = 12; const int rightMargin = 6; - // The description never takes more than its share, so a long value - // keeps room to be legible and a long description gets elided too. + // Give the description what the value does not need, up to a ceiling. + // + // An even split reads as fair but spends width on the wrong column: + // the values here are short (the longest prefix is "attachment:") while + // the descriptions are ordinary prose, so at a 400px popup two thirds + // of them elided while the value half sat mostly empty. Measuring the + // value and lending the description the remainder clears every built-in + // description at ~500px instead of ~700px. + // + // The ceiling still matters: a long value ("application/vnd.oasis...") + // must keep enough room to stay legible rather than be squeezed to an + // ellipsis by a description that happens to be wordy. const int available = option.rect.width() - gap - rightMargin; + const QString value = index.data(Qt::DisplayRole).toString(); + const int valueWidth = metrics.horizontalAdvance(value); + const int ceiling = (available * 65) / 100; + + // A value too long to fit would otherwise claim the whole row and + // leave no description at all. Cap what it can reserve, so an + // "application/vnd.oasis..." elides itself rather than silencing the + // column that explains what it is. + const int reservedForValue = qMin(valueWidth, available - ceiling); int descriptionWidth = qMin(metrics.horizontalAdvance(description), - available / 2); + qMin(available - reservedForValue, ceiling)); descriptionWidth = qMax(descriptionWidth, 0); // Let the base class draw the selection background and the value, but diff --git a/tests/test_querycompleter.cpp b/tests/test_querycompleter.cpp index 09eedc3..6cf9651 100644 --- a/tests/test_querycompleter.cpp +++ b/tests/test_querycompleter.cpp @@ -20,7 +20,9 @@ #include <QTemporaryDir> #include <QLineEdit> +#include <QImage> #include <QListView> +#include <QPixmap> #include "config.h" #include "querycompleter.h" @@ -81,6 +83,7 @@ private slots: void returnRunsTheQueryOnceCompletionIsDone(); void returnRunsTheQueryAfterAMouseAccept(); void returnRunsTheQueryWhenThePopupMatchesNothing(); + void theDescriptionSurvivesAModestPopupWidth(); }; // Copied from tests/test_config.cpp rather than shared, so the two test files @@ -781,5 +784,97 @@ void TestQueryCompleter::returnRunsTheQueryWhenThePopupMatchesNothing() QVERIFY(ran); } +void TestQueryCompleter::theDescriptionSurvivesAModestPopupWidth() +{ + // The delegate lends the description whatever the value does not need, up + // to 65% of the row. Under the previous even split the longest built-in + // description needed a ~650px popup to survive; it now needs ~500px, which + // is the difference between the column working at an ordinary window size + // and being decorative. + // + // 550px is chosen to sit inside that band: the current rule paints the text + // in full there, an even split cannot. A width outside the band would pass + // against both rules and prove nothing. + // + // The delegate is private to the .cpp, so this renders the real popup and + // reads the pixels back rather than reaching for the class: whether the text + // is legible on screen is a painting question, not an arithmetic one. + Config config; + QLineEdit edit; + edit.resize(550, edit.sizeHint().height()); + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + + QTest::keyClicks(&edit, QStringLiteral("path")); + QListView *popup = findPopup(); + QVERIFY(popup && popup->isVisible()); + QVERIFY(QTest::qWaitForWindowExposed(popup)); + + // "path:" carries the longest built-in description, so it is the row that + // fails first if the column is starved. + const QModelIndex row = popup->model()->index(0, 0); + QVERIFY(row.isValid()); + QCOMPARE(row.data(Qt::DisplayRole).toString(), QStringLiteral("path:")); + + const QRect rect = popup->visualRect(row); + QVERIFY(rect.isValid()); + QPixmap shot = popup->viewport()->grab(rect); + QVERIFY(!shot.isNull()); + const QImage image = shot.toImage(); + + // Measure how much of the row carries ink, then compare that against the + // width the description needs when it is NOT elided. + // + // "Something was drawn" is too weak a check: the previous even-split rule + // also drew the description, just cut down to an ellipsis, so a blank-or-not + // test passes against the very code this replaces. What distinguishes the + // two is whether the full text fits, which is a width comparison. + const QRgb background = image.pixel(image.width() - 2, image.height() / 2); + int rightmostInk = -1; + int leftmostInkAfterValue = image.width(); + for (int x = 0; x < image.width(); ++x) { + for (int y = 0; y < image.height(); ++y) { + if (image.pixel(x, y) != background) { + rightmostInk = qMax(rightmostInk, x); + break; + } + } + } + QVERIFY2(rightmostInk >= 0, "the row rendered entirely blank"); + + // The description is right-aligned, so the ink running to the right edge is + // the description itself. Walk left from there over the contiguous run to + // find how wide it was actually painted. + int x = rightmostInk; + int gapRun = 0; + while (x > 0 && gapRun < 8) { + bool column = false; + for (int y = 0; y < image.height(); ++y) { + if (image.pixel(x, y) != background) { + column = true; + break; + } + } + gapRun = column ? 0 : gapRun + 1; + if (column) + leftmostInkAfterValue = x; + --x; + } + + const QFontMetrics metrics(popup->font()); + const QString description = popup->model()->index(0, 1).data().toString(); + QCOMPARE(description, QStringLiteral("directory below the Maildir root")); + const int painted = rightmostInk - leftmostInkAfterValue; + const int needed = metrics.horizontalAdvance(description); + + // Allow a little slack for antialiasing at the glyph edges. + QVERIFY2(painted >= needed - 4, + qPrintable(QStringLiteral("description elided: painted %1px of the " + "%2px it needs") + .arg(painted).arg(needed))); +} + QTEST_MAIN(TestQueryCompleter) #include "test_querycompleter.moc" |