From 37de1ec5832599b3f8f15277a00f68f5e116425d Mon Sep 17 00:00:00 2001 From: James Graham Date: Sat, 11 Jan 2025 13:16:14 +0000 Subject: [PATCH] Move the storage of MessageContentModels to the room Move the storage of MessageContentModels to the room in the same manner as memeber objects to prevent duplication but mainly to make the system easier to maintain going forward with things like threads for example. This requires the creation of a MessageContentFilterModel as the same model may be used in multiple places, sometimes with the author showning sometimes not. --- src/CMakeLists.txt | 2 ++ src/models/mediamessagefiltermodel.cpp | 8 ----- src/models/messagecontentfiltermodel.cpp | 37 ++++++++++++++++++++ src/models/messagecontentfiltermodel.h | 43 ++++++++++++++++++++++++ src/models/messagecontentmodel.cpp | 30 +---------------- src/models/messagecontentmodel.h | 9 ----- src/models/messagefiltermodel.cpp | 9 ++--- src/models/messagefiltermodel.h | 1 + src/models/messagemodel.cpp | 20 +++-------- src/models/messagemodel.h | 2 -- src/models/threadmodel.cpp | 29 +++++++++++----- src/models/threadmodel.h | 7 +--- src/neochatroom.cpp | 43 ++++++++++++++++++++++++ src/neochatroom.h | 4 +++ src/timeline/Bubble.qml | 10 +++++- src/timeline/MessageDelegate.qml | 12 +++++-- 16 files changed, 177 insertions(+), 89 deletions(-) create mode 100644 src/models/messagecontentfiltermodel.cpp create mode 100644 src/models/messagecontentfiltermodel.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1828bcdd0..e0a09b3d8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -192,6 +192,8 @@ add_library(neochat STATIC models/roomsortparametermodel.h models/messagemodel.cpp models/messagemodel.h + models/messagecontentfiltermodel.cpp + models/messagecontentfiltermodel.h ) set_source_files_properties(qml/OsmLocationPlugin.qml PROPERTIES diff --git a/src/models/mediamessagefiltermodel.cpp b/src/models/mediamessagefiltermodel.cpp index 2caf8b9f6..46c9b8feb 100644 --- a/src/models/mediamessagefiltermodel.cpp +++ b/src/models/mediamessagefiltermodel.cpp @@ -39,14 +39,6 @@ QVariant MediaMessageFilterModel::data(const QModelIndex &index, int role) const const auto previousEventDay = mapToSource(this->index(index.row() + 1, 0)).data(TimelineMessageModel::TimeRole).toDateTime().toLocalTime().date(); return day != previousEventDay; } - // Catch and force the author to be shown for all rows - if (role == TimelineMessageModel::ContentModelRole) { - const auto model = qvariant_cast(mapToSource(index).data(TimelineMessageModel::ContentModelRole)); - if (model != nullptr) { - model->setShowAuthor(true); - } - return QVariant::fromValue(model); - } QVariantMap mediaInfo = mapToSource(index).data(TimelineMessageModel::MediaInfoRole).toMap(); diff --git a/src/models/messagecontentfiltermodel.cpp b/src/models/messagecontentfiltermodel.cpp new file mode 100644 index 000000000..e3d259c37 --- /dev/null +++ b/src/models/messagecontentfiltermodel.cpp @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: 2025 James Graham +// SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL + +#include "messagecontentfiltermodel.h" +#include "enums/messagecomponenttype.h" +#include "models/messagecontentmodel.h" + +MessageContentFilterModel::MessageContentFilterModel(QObject *parent) + : QSortFilterProxyModel(parent) +{ +} + +bool MessageContentFilterModel::showAuthor() const +{ + return m_showAuthor; +} + +void MessageContentFilterModel::setShowAuthor(bool showAuthor) +{ + if (showAuthor == m_showAuthor) { + return; + } + + m_showAuthor = showAuthor; + Q_EMIT showAuthorChanged(); +} + +bool MessageContentFilterModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const +{ + if (m_showAuthor) { + return true; + } + + const auto index = sourceModel()->index(source_row, 0, source_parent); + auto contentType = static_cast(index.data(MessageContentModel::ComponentTypeRole).toInt()); + return contentType != MessageComponentType::Author; +} diff --git a/src/models/messagecontentfiltermodel.h b/src/models/messagecontentfiltermodel.h new file mode 100644 index 000000000..6f943f078 --- /dev/null +++ b/src/models/messagecontentfiltermodel.h @@ -0,0 +1,43 @@ +// SPDX-FileCopyrightText: 2025 James Graham +// SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL + +#pragma once + +#include +#include + +/** + * @class MessageContentFilterModel + * + * This model filters a message's contents. + */ +class MessageContentFilterModel : public QSortFilterProxyModel +{ + Q_OBJECT + QML_ELEMENT + + /** + * @brief Whether the author component should be shown. + */ + Q_PROPERTY(bool showAuthor READ showAuthor WRITE setShowAuthor NOTIFY showAuthorChanged) + +public: + explicit MessageContentFilterModel(QObject *parent = nullptr); + + bool showAuthor() const; + void setShowAuthor(bool showAuthor); + +Q_SIGNALS: + void showAuthorChanged(); + +protected: + /** + * @brief Whether a row should be shown out or not. + * + * @sa QSortFilterProxyModel::filterAcceptsRow + */ + [[nodiscard]] bool filterAcceptsRow(int source_row, const QModelIndex &source_parent) const override; + +private: + bool m_showAuthor = true; +}; diff --git a/src/models/messagecontentmodel.cpp b/src/models/messagecontentmodel.cpp index 968d51683..a55caed7a 100644 --- a/src/models/messagecontentmodel.cpp +++ b/src/models/messagecontentmodel.cpp @@ -233,32 +233,6 @@ NeochatRoomMember *MessageContentModel::senderObject() const return m_room->qmlSafeMember(eventResult.first->senderId()); } -bool MessageContentModel::showAuthor() const -{ - return m_showAuthor; -} - -void MessageContentModel::setShowAuthor(bool showAuthor) -{ - if (showAuthor == m_showAuthor) { - return; - } - m_showAuthor = showAuthor; - - if (m_room->connection()->isIgnored(senderId())) { - if (showAuthor) { - beginInsertRows({}, 0, 0); - m_components.prepend(MessageComponent{MessageComponentType::Author, QString(), {}}); - endInsertRows(); - } else { - beginRemoveRows({}, 0, 0); - m_components.remove(0, 1); - endRemoveRows(); - } - } - Q_EMIT showAuthorChanged(); -} - static LinkPreviewer *emptyLinkPreview = new LinkPreviewer; QVariant MessageContentModel::data(const QModelIndex &index, int role) const @@ -434,9 +408,7 @@ void MessageContentModel::resetModel() return; } - if (m_showAuthor) { - m_components += MessageComponent{MessageComponentType::Author, QString(), {}}; - } + m_components += MessageComponent{MessageComponentType::Author, QString(), {}}; m_components += messageContentComponents(); endResetModel(); diff --git a/src/models/messagecontentmodel.h b/src/models/messagecontentmodel.h index 9db059b40..a33257775 100644 --- a/src/models/messagecontentmodel.h +++ b/src/models/messagecontentmodel.h @@ -25,11 +25,6 @@ class MessageContentModel : public QAbstractListModel QML_ELEMENT QML_UNCREATABLE("") - /** - * @brief Whether the author component is being shown. - */ - Q_PROPERTY(bool showAuthor READ showAuthor WRITE setShowAuthor NOTIFY showAuthorChanged) - public: enum MessageState { Unknown, /**< The message state is unknown. */ @@ -75,9 +70,6 @@ public: bool isPending = false, MessageContentModel *parent = nullptr); - bool showAuthor() const; - void setShowAuthor(bool showAuthor); - /** * @brief Get the given role value at the given index. * @@ -117,7 +109,6 @@ private: NeochatRoomMember *senderObject() const; MessageState m_currentState = Unknown; - bool m_showAuthor = true; bool m_isReply; void initializeModel(); diff --git a/src/models/messagefiltermodel.cpp b/src/models/messagefiltermodel.cpp index e7be90890..75bb5fbec 100644 --- a/src/models/messagefiltermodel.cpp +++ b/src/models/messagefiltermodel.cpp @@ -92,12 +92,8 @@ QVariant MessageFilterModel::data(const QModelIndex &index, int role) const return authorList(mapToSource(index).row()); } else if (role == ExcessAuthorsRole) { return excessAuthors(mapToSource(index).row()); - } else if (role == TimelineMessageModel::ContentModelRole) { - const auto model = qvariant_cast(mapToSource(index).data(TimelineMessageModel::ContentModelRole)); - if (model != nullptr && !showAuthor(index)) { - model->setShowAuthor(false); - } - return QVariant::fromValue(model); + } else if (role == ShowAuthorRole) { + return showAuthor(index); } return QSortFilterProxyModel::data(index, role); } @@ -109,6 +105,7 @@ QHash MessageFilterModel::roleNames() const roles[StateEventsRole] = "stateEvents"; roles[AuthorListRole] = "authorList"; roles[ExcessAuthorsRole] = "excessAuthors"; + roles[ShowAuthorRole] = "showAuthor"; return roles; } diff --git a/src/models/messagefiltermodel.h b/src/models/messagefiltermodel.h index de8fcdb5d..0b9ec6112 100644 --- a/src/models/messagefiltermodel.h +++ b/src/models/messagefiltermodel.h @@ -34,6 +34,7 @@ public: StateEventsRole, /**< List of state events in the aggregated state. */ AuthorListRole, /**< List of the first 5 unique authors of the aggregated state event. */ ExcessAuthorsRole, /**< The number of unique authors beyond the first 5. */ + ShowAuthorRole, /**< Whether the author of a message should be shown. */ LastRole, // Keep this last }; diff --git a/src/models/messagemodel.cpp b/src/models/messagemodel.cpp index 761afa7a7..e6ac0af39 100644 --- a/src/models/messagemodel.cpp +++ b/src/models/messagemodel.cpp @@ -120,16 +120,11 @@ QVariant MessageModel::data(const QModelIndex &idx, int role) const } if (role == ContentModelRole) { - QString modelId; - if (!event->get().id().isEmpty() && m_contentModels.contains(event->get().id())) { - modelId = event.value().get().id(); - } else if (!event.value().get().transactionId().isEmpty() && m_contentModels.contains(event.value().get().transactionId())) { - modelId = event.value().get().transactionId(); + auto evtOrTxnId = event->get().id(); + if (evtOrTxnId.isEmpty()) { + evtOrTxnId = event->get().transactionId(); } - if (!modelId.isEmpty()) { - return QVariant::fromValue(m_contentModels.at(modelId).get()); - } - return {}; + return QVariant::fromValue(m_room->contentModelForEvent(evtOrTxnId)); } if (role == GenericDisplayRole) { @@ -430,12 +425,6 @@ void MessageModel::createEventObjects(const Quotient::RoomEvent *event, bool isP senderId = m_room->localMember().id(); } - if (!m_contentModels.contains(eventId) && !m_contentModels.contains(event->transactionId())) { - if (!event->isStateEvent() || event->matrixType() == u"org.matrix.msc3672.beacon_info"_s) { - m_contentModels[eventId] = std::unique_ptr(new MessageContentModel(m_room, eventId, false, isPending)); - } - } - const auto roomMessageEvent = eventCast(event); if (roomMessageEvent && roomMessageEvent->isThreaded() && !m_threadModels.contains(roomMessageEvent->threadRootEventId())) { m_threadModels[roomMessageEvent->threadRootEventId()] = QSharedPointer(new ThreadModel(roomMessageEvent->threadRootEventId(), m_room)); @@ -515,7 +504,6 @@ void MessageModel::clearModel() void MessageModel::clearEventObjects() { - m_contentModels.clear(); m_reactionModels.clear(); m_readMarkerModels.clear(); } diff --git a/src/models/messagemodel.h b/src/models/messagemodel.h index 6d55a6544..0583a7e3a 100644 --- a/src/models/messagemodel.h +++ b/src/models/messagemodel.h @@ -7,7 +7,6 @@ #include #include -#include "messagecontentmodel.h" #include "neochatroom.h" #include "pollhandler.h" #include "readmarkermodel.h" @@ -153,7 +152,6 @@ private: bool resetting = false; bool movingEvent = false; - std::map> m_contentModels; QMap> m_readMarkerModels; QMap> m_threadModels; QMap> m_reactionModels; diff --git a/src/models/threadmodel.cpp b/src/models/threadmodel.cpp index 1afb13dce..23ae4e8b2 100644 --- a/src/models/threadmodel.cpp +++ b/src/models/threadmodel.cpp @@ -97,10 +97,9 @@ void ThreadModel::fetchMore(const QModelIndex &parent) const auto connection = room->connection(); m_currentJob = connection->callApi(room->id(), m_threadRootId, u"m.thread"_s, *m_nextBatch, QString(), 5); connect(m_currentJob, &Quotient::BaseJob::success, this, [this]() { - const auto room = dynamic_cast(QObject::parent()); auto newEvents = m_currentJob->chunk(); for (auto &event : newEvents) { - m_contentModels.push_back(new MessageContentModel(room, event->id())); + m_events.push_back(event->id()); } addModels(); @@ -122,12 +121,11 @@ void ThreadModel::fetchMore(const QModelIndex &parent) void ThreadModel::addNewEvent(const Quotient::RoomEvent *event) { - const auto room = dynamic_cast(QObject::parent()); auto eventId = event->id(); if (eventId.isEmpty()) { eventId = event->transactionId(); } - m_contentModels.push_front(new MessageContentModel(room, eventId)); + m_events.push_front(eventId); } void ThreadModel::addModels() @@ -137,8 +135,15 @@ void ThreadModel::addModels() } addSourceModel(m_threadRootContentModel.get()); - for (auto it = m_contentModels.crbegin(); it != m_contentModels.crend(); ++it) { - addSourceModel(*it); + const auto room = dynamic_cast(QObject::parent()); + if (room == nullptr) { + return; + } + for (auto it = m_events.crbegin(); it != m_events.crend(); ++it) { + const auto contentModel = room->contentModelForEvent(*it); + if (contentModel != nullptr) { + addSourceModel(room->contentModelForEvent(*it)); + } } addSourceModel(m_threadChatBarModel); @@ -149,9 +154,15 @@ void ThreadModel::addModels() void ThreadModel::clearModels() { removeSourceModel(m_threadRootContentModel.get()); - for (const auto &model : m_contentModels) { - if (sourceModels().contains(model)) { - removeSourceModel(model); + + const auto room = dynamic_cast(QObject::parent()); + if (room == nullptr) { + return; + } + for (const auto &model : m_events) { + const auto contentModel = room->contentModelForEvent(model); + if (sourceModels().contains(contentModel)) { + removeSourceModel(contentModel); } } removeSourceModel(m_threadChatBarModel); diff --git a/src/models/threadmodel.h b/src/models/threadmodel.h index 1ec29cb53..9f844357f 100644 --- a/src/models/threadmodel.h +++ b/src/models/threadmodel.h @@ -122,14 +122,9 @@ private: std::unique_ptr m_threadRootContentModel; - std::deque m_contentModels; + std::deque m_events; ThreadChatBarModel *m_threadChatBarModel; - QList m_events; - QList m_pendingEvents; - - std::unordered_map> m_unloadedEvents; - QMap> m_reactionModels; QPointer m_currentJob = nullptr; diff --git a/src/neochatroom.cpp b/src/neochatroom.cpp index f68405f21..9f8772fe1 100644 --- a/src/neochatroom.cpp +++ b/src/neochatroom.cpp @@ -173,6 +173,7 @@ void NeoChatRoom::setVisible(bool visible) if (!visible) { m_memberObjects.clear(); + m_eventContentModels.clear(); } } @@ -1754,4 +1755,46 @@ NeochatRoomMember *NeoChatRoom::qmlSafeMember(const QString &memberId) return m_memberObjects[memberId].get(); } +MessageContentModel *NeoChatRoom::contentModelForEvent(const QString &evtOrTxnId) +{ + const auto event = getEvent(evtOrTxnId); + if (event.first == nullptr) { + // If for some reason a model is there remove. + if (m_eventContentModels.contains(evtOrTxnId)) { + m_eventContentModels.erase(evtOrTxnId); + } + return nullptr; + } + + if (event.first->isStateEvent() || event.first->matrixType() == u"org.matrix.msc3672.beacon_info"_s) { + return nullptr; + } + + auto eventId = event.first->id(); + const auto txnId = event.first->transactionId(); + if (!m_eventContentModels.contains(eventId) && !m_eventContentModels.contains(txnId)) { + if (eventId.isEmpty()) { + eventId = txnId; + } + return m_eventContentModels.emplace(eventId, std::make_unique(this, eventId, false, event.second)).first->second.get(); + } + + if (!eventId.isEmpty() && m_eventContentModels.contains(eventId)) { + return m_eventContentModels[eventId].get(); + } + + if (!txnId.isEmpty() && m_eventContentModels.contains(txnId)) { + if (eventId.isEmpty()) { + return m_eventContentModels[txnId].get(); + } + + // If we now have an event ID use that as the map key instead of transaction ID. + auto txnModel = std::move(m_eventContentModels[txnId]); + m_eventContentModels.erase(txnId); + return m_eventContentModels.emplace(eventId, std::move(txnModel)).first->second.get(); + } + + return nullptr; +} + #include "moc_neochatroom.cpp" diff --git a/src/neochatroom.h b/src/neochatroom.h index 6e985119b..d7ac384ca 100644 --- a/src/neochatroom.h +++ b/src/neochatroom.h @@ -14,6 +14,7 @@ #include "enums/messagetype.h" #include "enums/pushrule.h" +#include "models/messagecontentmodel.h" #include "neochatroommember.h" #include "pollhandler.h" @@ -595,6 +596,8 @@ public: NeochatRoomMember *qmlSafeMember(const QString &memberId); + MessageContentModel *contentModelForEvent(const QString &evtOrTxnId); + private: bool m_visible = false; @@ -627,6 +630,7 @@ private: void cleanupExtraEvent(const QString &eventId); std::unordered_map> m_memberObjects; + std::unordered_map> m_eventContentModels; private Q_SLOTS: void updatePushNotificationState(QString type); diff --git a/src/timeline/Bubble.qml b/src/timeline/Bubble.qml index ae3e4317e..a41cffd43 100644 --- a/src/timeline/Bubble.qml +++ b/src/timeline/Bubble.qml @@ -41,6 +41,11 @@ QQC2.Control { */ property var author + /** + * @brief Whether the message author should be shown. + */ + required property bool showAuthor + /** * @brief Whether the message should be highlighted. */ @@ -105,7 +110,10 @@ QQC2.Control { spacing: Kirigami.Units.smallSpacing Repeater { id: contentRepeater - model: root.contentModel + model: MessageContentFilterModel { + showAuthor: root.showAuthor + sourceModel: root.contentModel + } delegate: MessageComponentChooser { room: root.room index: root.index diff --git a/src/timeline/MessageDelegate.qml b/src/timeline/MessageDelegate.qml index 2d11f7b33..36775f67b 100644 --- a/src/timeline/MessageDelegate.qml +++ b/src/timeline/MessageDelegate.qml @@ -52,6 +52,11 @@ TimelineDelegate { */ required property NeochatRoomMember author + /** + * @brief Whether the message author should be shown. + */ + required property bool showAuthor + /** * @brief The model to visualise the content of the message. */ @@ -222,11 +227,11 @@ TimelineDelegate { id: mainContainer Layout.fillWidth: true - Layout.topMargin: root.contentModel?.showAuthor ? Kirigami.Units.largeSpacing : (NeoChatConfig.compactLayout ? 1 : Kirigami.Units.smallSpacing) + Layout.topMargin: root.showAuthor ? Kirigami.Units.largeSpacing : (NeoChatConfig.compactLayout ? 1 : Kirigami.Units.smallSpacing) Layout.leftMargin: Kirigami.Units.smallSpacing Layout.rightMargin: Kirigami.Units.smallSpacing - implicitHeight: Math.max(root.contentModel?.showAuthor ? avatar.implicitHeight : 0, bubble.height) + implicitHeight: Math.max(root.showAuthor ? avatar.implicitHeight : 0, bubble.height) // show hover actions onHoveredChanged: { @@ -246,7 +251,7 @@ TimelineDelegate { topMargin: Kirigami.Units.smallSpacing } - visible: ((root.contentModel?.showAuthor ?? false) || root.isThreaded) && NeoChatConfig.showAvatarInTimeline && (NeoChatConfig.compactLayout || !_private.showUserMessageOnRight) + visible: ((root.showAuthor ?? false) || root.isThreaded) && NeoChatConfig.showAvatarInTimeline && (NeoChatConfig.compactLayout || !_private.showUserMessageOnRight) name: root.author.displayName source: root.author.avatarUrl color: root.author.color @@ -292,6 +297,7 @@ TimelineDelegate { room: root.room index: root.index author: root.author + showAuthor: root.showAuthor isThreaded: root.isThreaded // HACK: This is stupid but seemingly QConcatenateTablesProxyModel