[network/ruqola] src/widgets: Improve vertical spacing in normal message layout
Laurent Montel <[email protected]> Wed, 5 Aug 2026 11:11:56 +0000 (UTC)
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit 0dac0d4284c49d3102be576523830253f07a2ce9 by Laurent Montel, on behalf of Till Adam.
Committed on 05/08/2026 at 10:14.
Pushed by tilladam into branch 'master'.
Improve vertical spacing in normal message layout
Balance the author-to-text and inter-message gaps, and reserve a fixed author line so large inline emoji cannot overlap sender names.
M +0 -10 src/widgets/delegateutils/messagedelegateutils.cpp
M +0 -6 src/widgets/delegateutils/messagedelegateutils.h
M +24 -9 src/widgets/room/autotests/messagelistdelegatetest.cpp
M +16 -36 src/widgets/room/delegate/messagelistlayout/messagelistnormallayout.cpp
https://invent.kde.org/network/ruqola/-/commit/0dac0d4284c49d3102be576523830253f07a2ce9
diff --git a/src/widgets/delegateutils/messagedelegateutils.cpp b/src/widgets/delegateutils/messagedelegateutils.cpp
index 2bceded1b3..e851f35127 100644
--- a/src/widgets/delegateutils/messagedelegateutils.cpp
+++ b/src/widgets/delegateutils/messagedelegateutils.cpp
@@ -159,16 +159,6 @@ qreal MessageDelegateUtils::basicMargin()
return 8;
}
-int MessageDelegateUtils::senderBlockSpacing()
-{
- return 10;
-}
-
-int MessageDelegateUtils::groupedMessageSpacing()
-{
- return 1;
-}
-
QSize MessageDelegateUtils::timeStampSize(const QString &timeStampText, const QStyleOptionViewItem &option)
{
// This gives incorrect results (too small bounding rect), no idea why!
diff --git a/src/widgets/delegateutils/messagedelegateutils.h b/src/widgets/delegateutils/messagedelegateutils.h
index b92cb702e8..3c6deb0a41 100644
--- a/src/widgets/delegateutils/messagedelegateutils.h
+++ b/src/widgets/delegateutils/messagedelegateutils.h
@@ -52,12 +52,6 @@ void setClipboardSelection(TextSelection *selection);
[[nodiscard]] QSizeF dprAwareSize(const QPixmap &pixmap);
[[nodiscard]] qreal basicMargin();
-// Vertical spacing scale for the message list. Empty space is placed above each message block:
-// senderBlockSpacing() when the author changes (so a new speaker reads as a new block) and the
-// smaller groupedMessageSpacing() for a consecutive message from the same author. Both are
-// deliberately larger than the name-to-text gap, so proximity groups each author with their text.
-[[nodiscard]] int senderBlockSpacing();
-[[nodiscard]] int groupedMessageSpacing();
[[nodiscard]] QSize timeStampSize(const QString &timeStampText, const QStyleOptionViewItem &option);
[[nodiscard]] QSize textSizeHint(QTextDocument *doc, qreal *pBaseLine);
[[nodiscard]] bool showIgnoreMessages(const QModelIndex &index);
diff --git a/src/widgets/room/autotests/messagelistdelegatetest.cpp b/src/widgets/room/autotests/messagelistdelegatetest.cpp
index cbf13e7cdb..5c797cb441 100644
--- a/src/widgets/room/autotests/messagelistdelegatetest.cpp
+++ b/src/widgets/room/autotests/messagelistdelegatetest.cpp
@@ -11,6 +11,7 @@
#include "rocketchataccount.h"
#include "room/delegate/messagelistdelegate.h"
#include "ruqola.h"
+#include "ruqolaglobalconfig.h"
#include "testdata.h"
#include <QStandardItemModel>
@@ -34,6 +35,7 @@ void MessageListDelegateTest::layoutChecks_data()
{
QTest::addColumn<Message>("message");
QTest::addColumn<bool>("withDateHeader");
+ QTest::addColumn<bool>("normalLayout");
Message message;
message.setMessageId("someNonEmptyId"_ba);
@@ -42,8 +44,12 @@ void MessageListDelegateTest::layoutChecks_data()
message.setTimeStamp(QDateTime(QDate(2020, 2, 1), QTime(4, 7, 15)).toMSecsSinceEpoch());
message.setMessageType(Message::NormalText);
- QTest::newRow("text_no_date") << message << false;
- QTest::newRow("text_with_date") << message << true;
+ QTest::newRow("text_no_date") << message << false << false;
+ QTest::newRow("text_with_date") << message << true << false;
+
+ Message messageWithLargeEmoji = message;
+ messageWithLargeEmoji.setText(uR"(<span style="font: x-large NotoColorEmoji">💰</span> Text)"_s);
+ QTest::newRow("large_emoji_no_date") << messageWithLargeEmoji << false << true;
message.setMessageType(Message::NormalText);
MessageAttachment msgAttach = testAttachment();
@@ -56,17 +62,17 @@ void MessageListDelegateTest::layoutChecks_data()
message.setAttachments(attachments);
- QTest::newRow("attachment_no_text_no_date") << message << false;
- QTest::newRow("attachment_no_text_with_date") << message << true;
+ QTest::newRow("attachment_no_text_no_date") << message << false << false;
+ QTest::newRow("attachment_no_text_with_date") << message << true << false;
message.setText(u"The <b>text</b>"_s);
- QTest::newRow("attachment_with_text_no_date") << message << false;
- QTest::newRow("attachment_with_text_with_date") << message << true;
+ QTest::newRow("attachment_with_text_no_date") << message << false << false;
+ QTest::newRow("attachment_with_text_with_date") << message << true << false;
message.setEditedByUsername(message.username());
- QTest::newRow("edited_with_attachment_with_text_with_date") << message << true;
+ QTest::newRow("edited_with_attachment_with_text_with_date") << message << true << false;
// TODO tests with reactions
}
@@ -75,14 +81,20 @@ void MessageListDelegateTest::layoutChecks()
{
QFETCH(Message, message);
QFETCH(bool, withDateHeader);
+ QFETCH(bool, normalLayout);
// GIVEN a delegate and an index pointing to a message
+ const auto previousMessageStyle = RuqolaGlobalConfig::self()->messageStyle();
+ if (normalLayout) {
+ RuqolaGlobalConfig::self()->setMessageStyle(RuqolaGlobalConfig::EnumMessageStyle::Normal);
+ }
MessageListDelegate delegate(Ruqola::self()->rocketChatAccount(), nullptr);
delegate.setRocketChatAccount(Ruqola::self()->rocketChatAccount());
+ RuqolaGlobalConfig::self()->setMessageStyle(previousMessageStyle);
QStyleOptionViewItem option;
QWidget fakeWidget;
option.widget = &fakeWidget;
- option.rect = QRect(100, 100, 500, 500);
+ option.rect = QRect(normalLayout ? 0 : 100, 100, 500, 500);
QStandardItemModel model;
auto item = new QStandardItem;
@@ -110,8 +122,11 @@ void MessageListDelegateTest::layoutChecks()
// THEN
QCOMPARE(layout.senderText, u"dfaure"_s);
- QCOMPARE(layout.timeStampText, u"04:07"_s);
+ QCOMPARE(layout.timeStampText, normalLayout ? u"· 04:07"_s : u"04:07"_s);
QVERIFY(option.rect.contains(layout.usableRect));
+ if (normalLayout) {
+ QCOMPARE(qRound(layout.senderRect.top()), layout.usableRect.top());
+ }
// Text
if (message.text().isEmpty()) {
diff --git a/src/widgets/room/delegate/messagelistlayout/messagelistnormallayout.cpp b/src/widgets/room/delegate/messagelistlayout/messagelistnormallayout.cpp
index 34c620d492..d733a7cf02 100644
--- a/src/widgets/room/delegate/messagelistlayout/messagelistnormallayout.cpp
+++ b/src/widgets/room/delegate/messagelistlayout/messagelistnormallayout.cpp
@@ -40,6 +40,7 @@ MessageListLayoutBase::Layout MessageListNormalLayout::doLayout(const QStyleOpti
const QFontMetricsF senderFontMetrics(layout.senderFont);
const qreal senderAscent = layout.sameSenderAsPreviousMessage ? 0 : senderFontMetrics.ascent();
+ const int senderLineHeight = layout.sameSenderAsPreviousMessage ? 0 : qCeil(senderFontMetrics.height());
const QSizeF senderTextSize = senderFontMetrics.size(Qt::TextSingleLine, layout.senderText);
if (mRocketChatAccount && mRocketChatAccount->displayAvatars()) {
@@ -49,12 +50,6 @@ MessageListLayoutBase::Layout MessageListNormalLayout::doLayout(const QStyleOpti
QRect usableRect = option.rect;
const bool displayLastSeenMessage = index.data(MessagesModel::DisplayLastSeenMessage).toBool();
const bool dateDiffersFromPrevious = index.data(MessagesModel::DateDiffersFromPrevious).toBool();
- // Empty space above the message block that sets the vertical rhythm: a full gap when the
- // author changes (so a new speaker reads as a new block) and a small one for a grouped
- // consecutive message from the same author. Deliberately larger than the name-to-text gap
- // (textVMargin) further down, so proximity groups each author with their own text.
- const int blockTopSpacing = layout.sameSenderAsPreviousMessage ? MessageDelegateUtils::groupedMessageSpacing() : MessageDelegateUtils::senderBlockSpacing();
-
// A date header and a standalone unread-messages line each occupy a band at the top of
// the row. Reserve it here; the author line is shifted down by the same amount below.
int topBandHeight = 0;
@@ -66,10 +61,10 @@ MessageListLayoutBase::Layout MessageListNormalLayout::doLayout(const QStyleOpti
// top of the next message.
layout.displayLastSeenMessageY = usableRect.top() + topBandHeight / 2;
}
- usableRect.setTop(usableRect.top() + topBandHeight + blockTopSpacing);
+ usableRect.setTop(usableRect.top() + topBandHeight);
layout.usableRect = usableRect; // Just for the top, for now. The left will move later on.
- usableRect.setTop(usableRect.top() + senderAscent); // FIXME position.
+ usableRect.setTop(usableRect.top() + senderLineHeight);
const qreal margin = MessageDelegateUtils::basicMargin();
const int avatarWidth = MessageDelegateUtils::dprAwareSize(layout.avatarPixmap).width();
const int senderX = option.rect.x() + avatarWidth + 2 * margin;
@@ -151,39 +146,25 @@ MessageListLayoutBase::Layout MessageListNormalLayout::doLayout(const QStyleOpti
#endif
const int widthAfterMessage = hoverActionsWidth + margin / 2 + (timeStampUsesRightEdge ? timeSize.width() + margin : 0);
const int maxWidth = qMax(30, option.rect.width() - textLeft - widthAfterMessage);
- layout.baseLine = 0;
- const QSize textSize = mDelegate->helperText()->sizeHint(index, maxWidth, option, &layout.baseLine);
+ qreal textBaseLine = 0;
+ const QSize textSize = mDelegate->helperText()->sizeHint(index, maxWidth, option, &textBaseLine);
int attachmentsY;
- const int textVMargin = 3; // adjust this for "compactness"
+ // This margin is included both above and below the message text. Together with the row's
+ // two-pixel bottom breather, it keeps the sender-to-text and inter-message gaps in balance.
+ const int textVMargin = 5;
if (textSize.isValid()) {
- layout.textRect = QRect(textLeft,
- usableRect.top() + textVMargin + (layout.sameSenderAsPreviousMessage ? 0 : layout.senderRect.height()),
- maxWidth,
- textSize.height() + textVMargin);
+ layout.textRect = QRect(textLeft, usableRect.top() + textVMargin, maxWidth, textSize.height() + textVMargin);
attachmentsY = layout.textRect.y() + layout.textRect.height();
- layout.baseLine += option.rect.top(); // make it absolute
} else {
attachmentsY = usableRect.top() + textVMargin;
- layout.baseLine = attachmentsY + option.fontMetrics.ascent();
}
layout.usableRect.setLeft(textLeft);
- // Align top of sender rect so it matches the baseline of the richtext
- layout.senderRect =
- QRectF(senderX, layout.baseLine - senderAscent, senderTextSize.width(), (layout.sameSenderAsPreviousMessage ? 0 : senderTextSize.height()));
- // usableRect already pushed the message text down by the top band (a date header drawn by
- // drawDate, or a standalone unread-messages line) plus the block-top spacing. Shift the whole
- // author line — the name baseline, its rect, and therefore the avatar — down by the same
- // amount, so the name stays exactly one line above its own text regardless of that offset.
- // Only the text branch needs this: it derived baseLine from option.rect.top() (unshifted).
- // The empty-text branch (attachment/blocks/urls only) already derived baseLine from
- // usableRect.top(), which includes both offsets, so shifting again would double-count and
- // drop the author line below its own attachment.
- const int authorLineShift = topBandHeight + blockTopSpacing;
- if (textSize.isValid() && authorLineShift > 0) {
- layout.baseLine += authorLineShift;
- layout.senderRect.moveTop(layout.senderRect.top() + authorLineShift);
- }
+ // Keep the author in its own fixed-height line. Deriving this baseline from the first
+ // message line would let tall inline content (such as an emoji) push the author downward
+ // into the message text.
+ layout.baseLine = layout.usableRect.top() + senderAscent;
+ layout.senderRect = QRectF(senderX, layout.usableRect.top(), senderTextSize.width(), senderLineHeight);
// Align top of avatar with top of sender rect
const double senderRectY{layout.senderRect.y()};
layout.avatarPos = QPointF(option.rect.x() + margin, senderRectY);
@@ -351,9 +332,8 @@ QSize MessageListNormalLayout::sizeHint(const QStyleOptionViewItem &option, cons
// Note: option.rect in this method is huge (as big as the viewport)
const MessageListLayoutBase::Layout layout = doLayout(option, index);
- // Most inter-message separation now lives in the block-top spacing (added in doLayout to the
- // top of the next block), so only a small breather is needed under each row to keep the two
- // gaps from stacking into an oversized space between messages.
+ // textVMargin already provides most of the inter-message separation, so only a small
+ // breather is needed under each row.
int additionalHeight = 2;
// A little bit of margin below the very last item, it just looks better
if (index.row() == index.model()->rowCount() - 1) {