From 1860de12eab1c481072fce6b1d105ebf8cb75202 Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 17 Jun 2025 16:41:38 +0100 Subject: [PATCH] RoomPage cleanup This does some further cleanup of RoomPage, mostly removing all the vestigial bits from when we could have multiple windows. But also stuff is moved to TimelineView where possible. The loading placeholder is removed as TimelineModel already has this built in. TimelineView now gets room from it's model. This is to ensure we're always using the same room as it which may not be true momentarily when RoomManager.currentRoom changes as the model does it's own reset sequence. This will prevent some race conditions in future (and which I already hit creating other MRs) --- src/app/qml/RoomPage.qml | 50 +++++++--------------------- src/timeline/TimelineView.qml | 61 +++++++++++++++++++---------------- 2 files changed, 45 insertions(+), 66 deletions(-) diff --git a/src/app/qml/RoomPage.qml b/src/app/qml/RoomPage.qml index 4ba2303ee..429d56df1 100644 --- a/src/app/qml/RoomPage.qml +++ b/src/app/qml/RoomPage.qml @@ -11,15 +11,14 @@ import org.kde.kirigami as Kirigami import org.kde.kitemmodels import org.kde.neochat -import org.kde.neochat.chatbar Kirigami.Page { id: root - /// Not readonly because of the separate window view. - property NeoChatRoom currentRoom: RoomManager.currentRoom - - required property NeoChatConnection connection + /** + * @brief The NeoChatRoom the delegate is being displayed in. + */ + readonly property NeoChatRoom currentRoom: RoomManager.currentRoom /** * @brief The TimelineModel to use. @@ -59,11 +58,6 @@ Kirigami.Page { */ property MediaMessageFilterModel mediaMessageFilterModel: RoomManager.mediaMessageFilterModel - property bool loading: !root.currentRoom || (root.currentRoom.timelineSize === 0 && !root.currentRoom.allHistoryLoaded) - - /// Disable cancel shortcut. Used by the separate window since it provides its own cancel implementation. - property bool disableCancelShortcut: false - title: root.currentRoom ? root.currentRoom.displayName : "" focus: true padding: 0 @@ -86,9 +80,9 @@ Kirigami.Page { } Connections { - target: root.connection + target: root.currentRoom.connection function onIsOnlineChanged() { - if (!root.connection.isOnline) { + if (!root.currentRoom.connection.isOnline) { banner.text = i18n("NeoChat is offline. Please check your network connection."); banner.visible = true; banner.type = Kirigami.MessageType.Error; @@ -109,10 +103,9 @@ Kirigami.Page { Loader { id: timelineViewLoader anchors.fill: parent - active: root.currentRoom && !root.currentRoom.isInvite && !root.loading && !root.currentRoom.isSpace + active: root.currentRoom && !root.currentRoom.isInvite && !root.currentRoom.isSpace sourceComponent: TimelineView { id: timelineView - room: root.currentRoom messageFilterModel: root.messageFilterModel compactLayout: NeoChatConfig.compactLayout fileDropEnabled: !Controller.isFlatpak @@ -147,14 +140,6 @@ Kirigami.Page { } } - Loader { - active: root.loading && !invitationLoader.active && RoomManager.currentRoom && !spaceLoader.active - anchors.centerIn: parent - sourceComponent: Kirigami.LoadingPlaceholder { - anchors.centerIn: parent - } - } - background: Rectangle { Kirigami.Theme.colorSet: Kirigami.Theme.View Kirigami.Theme.inherit: false @@ -169,7 +154,7 @@ Kirigami.Page { id: chatBar width: parent.width currentRoom: root.currentRoom - connection: root.connection + connection: root.currentRoom.connection } } @@ -186,21 +171,8 @@ Kirigami.Page { } } - Shortcut { - sequence: StandardKey.Cancel - onActivated: { - if (!timelineViewLoader.item.atYEnd || !root.currentRoom.partiallyReadStats.empty()) { - timelineViewLoader.item.goToLastMessage(); - root.currentRoom.markAllMessagesAsRead(); - } else { - applicationWindow().pageStack.get(0).forceActiveFocus(); - } - } - enabled: !root.disableCancelShortcut - } - Connections { - target: root.connection + target: root.currentRoom.connection function onJoinedRoom(room, invited) { if (root.currentRoom.id === invited.id) { RoomManager.resolveResource(room.id); @@ -286,7 +258,7 @@ Kirigami.Page { id: messageDelegateContextMenu MessageDelegateContextMenu { room: root.currentRoom - connection: root.connection + connection: root.currentRoom.connection } } @@ -294,7 +266,7 @@ Kirigami.Page { id: fileDelegateContextMenu FileDelegateContextMenu { room: root.currentRoom - connection: root.connection + connection: root.currentRoom.connection } } diff --git a/src/timeline/TimelineView.qml b/src/timeline/TimelineView.qml index 5532a8292..d0e81e946 100644 --- a/src/timeline/TimelineView.qml +++ b/src/timeline/TimelineView.qml @@ -13,11 +13,6 @@ import org.kde.neochat.libneochat as LibNeoChat QQC2.ScrollView { id: root - /** - * @brief The NeoChatRoom the delegate is being displayed in. - */ - required property LibNeoChat.NeoChatRoom room - /** * @brief The MessageFilterModel to use. * @@ -25,11 +20,6 @@ QQC2.ScrollView { */ required property MessageFilterModel messageFilterModel - /** - * @brief Whether the timeline is scrolled to the end. - */ - readonly property bool atYEnd: messageListView.atYEnd - /** * @brief Whether the timeline ListView is interactive. */ @@ -64,7 +54,7 @@ QQC2.ScrollView { * All messages will be marked as read. */ function goToLastMessage() { - room.markAllMessagesAsRead(); + _private.room.markAllMessagesAsRead(); messageListView.positionViewAtBeginning(); } @@ -137,6 +127,17 @@ QQC2.ScrollView { clip: true interactive: Kirigami.Settings.isMobile + Shortcut { + sequence: StandardKey.Cancel + onActivated: { + if (!messageListView.atYEnd || !_private.room.partiallyReadStats.empty()) { + messageListView.positionViewAtBeginning(); + } else { + applicationWindow().pageStack.get(0).forceActiveFocus(); + } + } + } + Component.onCompleted: { positionViewAtBeginning(); } @@ -154,19 +155,19 @@ QQC2.ScrollView { function onReadMarkerAdded() { if (messageListView.allUnreadVisible()) { - root.room.markAllMessagesAsRead(); + _private.room.markAllMessagesAsRead(); } } function onNewLocalUserEventAdded() { messageListView.positionViewAtBeginning(); - root.room.markAllMessagesAsRead(); + _private.room.markAllMessagesAsRead(); } } onAtYEndChanged: if (atYEnd && _private.hasScrolledUpBefore) { if (QQC2.ApplicationWindow.window && (QQC2.ApplicationWindow.window.visibility !== QQC2.ApplicationWindow.Hidden)) { - root.room.markAllMessagesAsRead(); + _private.room.markAllMessagesAsRead(); } _private.hasScrolledUpBefore = false; } else if (!atYEnd) { @@ -175,7 +176,7 @@ QQC2.ScrollView { model: root.messageFilterModel delegate: EventDelegate { - room: root.room + room: _private.room } KirigamiComponents.FloatingButton { @@ -194,13 +195,13 @@ QQC2.ScrollView { padding: Kirigami.Units.largeSpacing z: 2 - visible: (!root.room?.partiallyReadStats.empty()) + visible: (!_private.room?.partiallyReadStats.empty()) - text: root.room.readMarkerLoaded ? i18n("Jump to first unread message") : i18n("Jump to oldest loaded message") + text: _private.room.readMarkerLoaded ? i18n("Jump to first unread message") : i18n("Jump to oldest loaded message") action: Kirigami.Action { onTriggered: { goReadMarkerFab.textChanged() - root.goToEvent(root.room.lastFullyReadEventId); + root.goToEvent(_private.room.lastFullyReadEventId); } icon.name: "go-up" shortcut: "Shift+PgUp" @@ -232,7 +233,7 @@ QQC2.ScrollView { action: Kirigami.Action { onTriggered: { messageListView.positionViewAtBeginning(); - root.room.markAllMessagesAsRead(); + _private.room.markAllMessagesAsRead(); } icon.name: "go-down" shortcut: "Shift+PgDown" @@ -248,7 +249,7 @@ QQC2.ScrollView { DropArea { id: dropAreaFile anchors.fill: parent - onDropped: drop => { root.room.mainCache.attachmentPath = drop.urls[0] } + onDropped: drop => { _private.room.mainCache.attachmentPath = drop.urls[0] } enabled: root.fileDropEnabled } @@ -268,7 +269,7 @@ QQC2.ScrollView { RowLayout { id: typingPaneContainer - visible: root.room && root.room.otherMembersTyping.length > 0 + visible: _private.room && _private.room.otherMembersTyping.length > 0 anchors.left: parent.left anchors.right: parent.right anchors.bottom: parent.bottom @@ -287,7 +288,7 @@ QQC2.ScrollView { Layout.maximumWidth: typingPaneSizeHelper.availableWidth TypingPane { id: typingPane - labelText: visible ? i18ncp("Message displayed when some users are typing", "%2 is typing", "%2 are typing", root.room.otherMembersTyping.length, root.room.otherMembersTyping.map(member => member.displayName).join(", ")) : "" + labelText: visible ? i18ncp("Message displayed when some users are typing", "%2 is typing", "%2 are typing", _private.room.otherMembersTyping.length, _private.room.otherMembersTyping.map(member => member.displayName).join(", ")) : "" } } @@ -313,11 +314,17 @@ QQC2.ScrollView { } repeat: true } + } - QtObject { - id: _private - // Used to determine if scrolling to the bottom should mark the message as unread - property bool hasScrolledUpBefore: false - } + QtObject { + id: _private + + // Get the room from the model so we always have the one its using (this + // may not be the case just after RoomManager.currentRoom changes while + // the model does the switch over). + readonly property LibNeoChat.NeoChatRoom room: messageListView.model.sourceModel.timelineMessageModel.room + + // Used to determine if scrolling to the bottom should mark the message as unread + property bool hasScrolledUpBefore: false } }