From a4c445d1a5e5b690cfec3bf71a5fa412f8267ce7 Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 8 Nov 2022 19:40:56 +0000 Subject: [PATCH] Add a section label at the top which shows the date label of the next section **Updated** Add a section label at the top which shows the date label of the next section up. This means that the user will always be able to see the date of all messages on screen. ![image](/uploads/ecbcdc0740877ea0d72e735176353036/image.png) From the feedback given I've added a background at the top. I also added an underline to the heading which applies both at the top and in the listView since they use the same component. I added it originally for the top because I felt it looked a bit weird having messages appear from behind a heading background the same colour as the listView background. Note: I know the gaps between messages are not right. I had to set the spacing in the listView to 0 to prevent itemAt returning null. I plan to add it back in as part of the delegate code before it would be merge. Fixes BUG:454880 --- .../Component/Timeline/SectionDelegate.qml | 46 +++++++-- src/qml/Component/Timeline/StateDelegate.qml | 95 ++++++++++--------- .../Component/Timeline/TimelineContainer.qml | 15 +-- src/qml/Page/RoomPage.qml | 44 ++++++++- 4 files changed, 137 insertions(+), 63 deletions(-) diff --git a/src/qml/Component/Timeline/SectionDelegate.qml b/src/qml/Component/Timeline/SectionDelegate.qml index e94fba922..a8ecc047e 100644 --- a/src/qml/Component/Timeline/SectionDelegate.qml +++ b/src/qml/Component/Timeline/SectionDelegate.qml @@ -3,15 +3,45 @@ // SPDX-License-Identifier: GPL-3.0-only import QtQuick 2.15 +import QtQuick.Controls 2.15 as QQC2 +import QtQuick.Layouts 1.15 import org.kde.kirigami 2.15 as Kirigami -Kirigami.Heading { - level: 4 - text: model.showSection ? section : "" - color: Kirigami.Theme.disabledTextColor - horizontalAlignment: Text.AlignHCenter - verticalAlignment: Text.AlignVCenter - topPadding: Kirigami.Units.largeSpacing * 2 - bottomPadding: Kirigami.Units.smallSpacing +import org.kde.neochat 1.0 + +QQC2.ItemDelegate { + id: sectionDelegate + + property alias labelText: sectionLabel.text + property var maxWidth: Number.POSITIVE_INFINITY + + topPadding: Kirigami.Units.largeSpacing + bottomPadding: 0 // Note not 0 by default + + contentItem: ColumnLayout { + spacing: Kirigami.Units.smallSpacing + Layout.fillWidth: true + + Kirigami.Heading { + id: sectionLabel + level: 4 + color: Kirigami.Theme.disabledTextColor + horizontalAlignment: Text.AlignHCenter + verticalAlignment: Text.AlignVCenter + Layout.fillWidth: true + Layout.maximumWidth: maxWidth + } + Kirigami.Separator { + Layout.minimumHeight: 2 + Layout.fillWidth: true + Layout.maximumWidth: maxWidth + } + } + + background: Rectangle { + color: Config.blur ? "transparent" : Kirigami.Theme.backgroundColor + Kirigami.Theme.inherit: false + Kirigami.Theme.colorSet: Kirigami.Theme.Window + } } diff --git a/src/qml/Component/Timeline/StateDelegate.qml b/src/qml/Component/Timeline/StateDelegate.qml index 2e9a035b4..3309b96d9 100644 --- a/src/qml/Component/Timeline/StateDelegate.qml +++ b/src/qml/Component/Timeline/StateDelegate.qml @@ -11,13 +11,14 @@ import org.kde.neochat 1.0 Control { id: stateDelegate + + readonly property bool sectionVisible: model.showSection + // extraWidth defines how the delegate can grow after the listView gets very wide readonly property int extraWidth: messageListView.width >= Kirigami.Units.gridUnit * 46 ? Math.min((messageListView.width - Kirigami.Units.gridUnit * 46), Kirigami.Units.gridUnit * 20) : 0 readonly property int delegateMaxWidth: Config.compactLayout ? messageListView.width: Math.min(messageListView.width, Kirigami.Units.gridUnit * 40 + extraWidth) width: delegateMaxWidth -// anchors.leftMargin: Kirigami.Units.largeSpacing -// anchors.rightMargin: Kirigami.Units.largeSpacing state: Config.compactLayout ? "alignLeft" : "alignCenter" // Align left when in compact mode and center when using bubbles @@ -46,56 +47,60 @@ Control { } ] - height: sectionDelegate.height + rowLayout.height - SectionDelegate { - id: sectionDelegate + height: columnLayout.implicitHeight + columnLayout.anchors.topMargin + + ColumnLayout { + id: columnLayout + spacing: sectionVisible ? Kirigami.Units.largeSpacing : 0 anchors.top: parent.top + anchors.topMargin: sectionVisible ? 0 : Kirigami.Units.largeSpacing anchors.left: parent.left anchors.right: parent.right - visible: model.showSection - height: visible ? implicitHeight : 0 - } - RowLayout { - id: rowLayout - height: label.contentHeight - anchors.bottom: parent.bottom - anchors.left: parent.left - anchors.right: parent.right - anchors.leftMargin: Kirigami.Units.gridUnit * 1.5 + Kirigami.Units.smallSpacing + (Config.compactLayout ? Kirigami.Units.largeSpacing * 1.25 : 0) - anchors.rightMargin: Kirigami.Units.largeSpacing - - Kirigami.Avatar { - id: icon - Layout.preferredWidth: Kirigami.Units.iconSizes.small - Layout.preferredHeight: Kirigami.Units.iconSizes.small - Layout.alignment: Qt.AlignTop - - name: model.displayNameForInitials - source: author.avatarMediaId ? ("image://mxc/" + author.avatarMediaId) : "" - color: author.color - - Component { - id: userDetailDialog - - UserDetailDialog {} - } - - MouseArea { - anchors.fill: parent - onClicked: userDetailDialog.createObject(ApplicationWindow.overlay, {room: currentRoom, user: author.object, displayName: author.displayName, avatarMediaId: author.avatarMediaId, avatarUrl: author.avatarUrl}).open() - } + SectionDelegate { + id: sectionDelegate + Layout.fillWidth: true + visible: sectionVisible + labelText: sectionVisible ? section : "" } - Label { - id: label - Layout.alignment: Qt.AlignVCenter + RowLayout { + id: rowLayout + implicitHeight: label.contentHeight Layout.fillWidth: true - Layout.preferredHeight: icon.height - wrapMode: Text.WordWrap - textFormat: Text.RichText - text: `${model.authorDisplayName} ${aggregateDisplay}` - onLinkActivated: userDetailDialog.createObject(ApplicationWindow.overlay, {room: currentRoom, user: author.object, displayName: author.displayName, avatarMediaId: author.avatarMediaId, avatarUrl: author.avatarUrl}).open() + Layout.leftMargin: Kirigami.Units.gridUnit * 1.5 + Kirigami.Units.smallSpacing * 1.5 + (Config.compactLayout ? Kirigami.Units.largeSpacing * 1.25 : 0) + Layout.rightMargin: Kirigami.Units.largeSpacing + + Kirigami.Avatar { + id: icon + Layout.preferredWidth: Kirigami.Units.iconSizes.small + Layout.preferredHeight: Kirigami.Units.iconSizes.small + + name: author.displayName + source: author.avatarMediaId ? ("image://mxc/" + author.avatarMediaId) : "" + color: author.color + + Component { + id: userDetailDialog + + UserDetailDialog {} + } + + MouseArea { + anchors.fill: parent + onClicked: userDetailDialog.createObject(ApplicationWindow.overlay, {room: currentRoom, user: author.object, displayName: author.displayName, avatarMediaId: author.avatarMediaId, avatarUrl: author.avatarUrl}).open() + } + } + + Label { + id: label + Layout.alignment: Qt.AlignVCenter + Layout.fillWidth: true + wrapMode: Text.WordWrap + textFormat: Text.RichText + text: `${currentRoom.htmlSafeMemberName(author.id)} ${aggregateDisplay}` + onLinkActivated: userDetailDialog.createObject(ApplicationWindow.overlay, {room: currentRoom, user: author.object, displayName: author.displayName, avatarMediaId: author.avatarMediaId, avatarUrl: author.avatarUrl}).open() + } } } } diff --git a/src/qml/Component/Timeline/TimelineContainer.qml b/src/qml/Component/Timeline/TimelineContainer.qml index 6a59c8772..d1813e54e 100644 --- a/src/qml/Component/Timeline/TimelineContainer.qml +++ b/src/qml/Component/Timeline/TimelineContainer.qml @@ -14,6 +14,8 @@ QQC2.ItemDelegate { default property alias innerObject : column.children // readonly property bool failed: marks == EventStatus.SendingFailed + readonly property bool sectionVisible: model.showSection + property bool isEmote: false property bool cardBackground: true property bool isHighlighted: model.isHighlighted || isTemporaryHighlighted @@ -55,7 +57,7 @@ QQC2.ItemDelegate { leftInset: Kirigami.Units.smallSpacing rightInset: Kirigami.Units.smallSpacing width: delegateMaxWidth - height: sectionDelegate.height + Math.max(model.showAuthor ? avatar.height : 0, bubble.implicitHeight) + loader.height + (showAuthor ? Kirigami.Units.largeSpacing : (Config.compactLayout ? 1 : Kirigami.Units.smallSpacing)) + height: sectionDelegate.height + Math.max(model.showAuthor ? avatar.height : 0, bubble.implicitHeight) + loader.height + loader.anchors.topMargin + avatar.anchors.topMargin background: Rectangle { visible: timelineContainer.hovered color: Kirigami.ColorUtils.tintWithAlpha(Kirigami.Theme.backgroundColor, Kirigami.Theme.highlightColor, 0.15) @@ -110,11 +112,11 @@ QQC2.ItemDelegate { SectionDelegate { id: sectionDelegate - width: parent.width - anchors.left: avatar.left - anchors.leftMargin: Kirigami.Units.smallSpacing - visible: model.showSection + anchors.left: timelineContainer.left + anchors.right: timelineContainer.right + visible: sectionVisible height: visible ? implicitHeight : 0 + labelText: model.showSection ? section : "" } Kirigami.Avatar { @@ -304,10 +306,9 @@ QQC2.ItemDelegate { left: bubble.left right: parent.right top: bubble.bottom - topMargin: active && Config.compactLayout ? 0 : Kirigami.Units.smallSpacing + topMargin: active ? Kirigami.Units.smallSpacing : 0 } height: active ? item.implicitHeight : 0 - //Layout.bottomMargin: readMarker ? Kirigami.Units.smallSpacing : 0 active: eventType !== MessageEventModel.State && eventType !== MessageEventModel.Notice && reaction != undefined && reaction.length > 0 visible: active sourceComponent: ReactionDelegate { } diff --git a/src/qml/Page/RoomPage.qml b/src/qml/Page/RoomPage.qml index c1cf68487..edaa83f13 100644 --- a/src/qml/Page/RoomPage.qml +++ b/src/qml/Page/RoomPage.qml @@ -187,8 +187,12 @@ Kirigami.ScrollablePage { readonly property int largestVisibleIndex: count > 0 ? indexAt(contentX + (width / 2), contentY + height - 1) : -1 readonly property bool isLoaded: page.width * page.height > 10 + // Spacing needs to be zero or the top sectionLabel overlay will be disrupted. + // This is because itemAt returns null in the spaces. + // All spacing should be handled by the delegates themselves spacing: 0 - + // Ensures that the top item is not covered by sectionBanner if the page is scrolled all the way up + // topMargin: sectionBanner.height verticalLayoutDirection: ListView.BottomToTop highlightMoveDuration: 500 @@ -226,6 +230,40 @@ Kirigami.ScrollablePage { hasScrolledUpBefore = true; } + // Not rendered because the sections are part of the TimelineContainer.qml, this is only so that items have the section property available for use by sectionBanner. + // This is due to the fact that the ListView verticalLayout is BottomToTop. + // This also flips the sections which would appear at the bottom but for a timeline they still need to be at the top (bottom from the qml perspective). + // There is currently no option to put section headings at the bottom in qml. + section.property: "section" + + readonly property var sectionBannerItem: contentHeight >= height ? itemAtIndex(sectionBannerIndex()) : undefined + + function sectionBannerIndex() { + let center = messageListView.x + messageListView.width / 2; + let yStart = messageListView.y + messageListView.contentY; + let index = -1 + let i = 0 + while (index === -1 && i < 100) { + index = messageListView.indexAt(center, yStart + i); + i++; + } + return index + } + + footer: SectionDelegate { + id: sectionBanner + + anchors.left: parent.left + anchors.leftMargin: messageListView.sectionBannerItem ? messageListView.sectionBannerItem.x : 0 + anchors.right: parent.right + + maxWidth: messageListView.sectionBannerItem ? messageListView.sectionBannerItem.width - Kirigami.Units.largeSpacing * 2 : 0 + z: 3 + visible: messageListView.sectionBannerItem && messageListView.sectionBannerItem.ListView.section != "" + labelText: messageListView.sectionBannerItem ? messageListView.sectionBannerItem.ListView.section : "" + } + footerPositioning: ListView.OverlayHeader + QQC2.Popup { anchors.centerIn: parent @@ -589,7 +627,7 @@ Kirigami.ScrollablePage { let center = messageListView.x + messageListView.width / 2; let index = -1 let i = 0 - while(index === -1 && i < 100) { + while (index === -1 && i < 100) { index = messageListView.indexAt(center, messageListView.y + messageListView.contentY + i); i++; } @@ -600,7 +638,7 @@ Kirigami.ScrollablePage { let center = messageListView.x + messageListView.width / 2; let index = -1 let i = 0 - while(index === -1 && i < 100) { + while (index === -1 && i < 100) { index = messageListView.indexAt(center, messageListView.y + messageListView.contentY + messageListView.height - i); i++ }