From 0f72ccd00c51083742a1ce4b50b7c8d399d764e8 Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 23 Jul 2024 10:52:52 +0100 Subject: [PATCH] Mamange the creation of NeochatRoomMembers and only create one per member rather than event. --- src/models/messageeventmodel.cpp | 23 ++--- src/models/messageeventmodel.h | 2 + src/neochatroommember.h | 158 ++++--------------------------- src/timeline/StateComponent.qml | 3 +- 4 files changed, 30 insertions(+), 156 deletions(-) diff --git a/src/models/messageeventmodel.cpp b/src/models/messageeventmodel.cpp index d1a8bf640..e2444732e 100644 --- a/src/models/messageeventmodel.cpp +++ b/src/models/messageeventmodel.cpp @@ -88,6 +88,7 @@ void MessageEventModel::setRoom(NeoChatRoom *room) // HACK: Reset the model to a null room first to make sure QML dismantles // last room's objects before the room is actually changed beginResetModel(); + m_memberObjects.clear(); m_readMarkerModels.clear(); m_currentRoom->disconnect(this); m_currentRoom = nullptr; @@ -216,22 +217,6 @@ void MessageEventModel::setRoom(NeoChatRoom *room) beginResetModel(); endResetModel(); }); - connect(m_currentRoom, &Room::memberNameUpdated, this, [this](RoomMember member) { - for (auto it = m_currentRoom->messageEvents().rbegin(); it != m_currentRoom->messageEvents().rend(); ++it) { - auto event = it->event(); - if (event->senderId() == member.id()) { - refreshEventRoles(event->id(), {AuthorRole}); - } - } - }); - connect(m_currentRoom, &Room::memberAvatarUpdated, this, [this](RoomMember member) { - for (auto it = m_currentRoom->messageEvents().rbegin(); it != m_currentRoom->messageEvents().rend(); ++it) { - auto event = it->event(); - if (event->senderId() == member.id()) { - refreshEventRoles(event->id(), {AuthorRole}); - } - } - }); qCDebug(MessageEvent) << "Connected to room" << room->id() << "as" << room->localMember().id(); } else { @@ -477,7 +462,7 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const } else { mId = evt.senderId(); } - return QVariant::fromValue(new NeochatRoomMember(m_currentRoom, mId)); + return QVariant::fromValue(m_memberObjects.at(mId).get()); } if (role == HighlightRole) { @@ -628,6 +613,10 @@ void MessageEventModel::createEventObjects(const Quotient::RoomEvent *event) auto eventId = event->id(); + if (!m_memberObjects.contains(event->senderId())) { + m_memberObjects[event->senderId()] = std::unique_ptr(new NeochatRoomMember(m_currentRoom, event->senderId())); + } + // ReadMarkerModel handles updates to add and remove markers, we only need to // handle adding and removing whole models here. if (m_readMarkerModels.contains(eventId)) { diff --git a/src/models/messageeventmodel.h b/src/models/messageeventmodel.h index cdf7cb685..ef895f4ee 100644 --- a/src/models/messageeventmodel.h +++ b/src/models/messageeventmodel.h @@ -9,6 +9,7 @@ #include "linkpreviewer.h" #include "neochatroom.h" +#include "neochatroommember.h" #include "pollhandler.h" #include "readmarkermodel.h" @@ -115,6 +116,7 @@ private: bool movingEvent = false; KFormat m_format; + std::map> m_memberObjects; QMap> m_readMarkerModels; QMap> m_reactionModels; diff --git a/src/neochatroommember.h b/src/neochatroommember.h index bb8d8c60b..655ec74fa 100644 --- a/src/neochatroommember.h +++ b/src/neochatroommember.h @@ -12,20 +12,26 @@ class NeoChatRoom; -//! This class is for visualizing a user in a room context. -//! -//! The class is intentionally a read-only data object that is effectively a wrapper -//! around an m.room.member event for the desired user. This is designed provide the -//! data in a format ready for visualizing a user (avatar or name) in the context -//! of the room it was generated in. This means that if a user has set a unique -//! name or avatar for a particular room that is what will be returned. -//! -//! \note The RoomMember class is not intended for interacting with a User's profile. -//! For that a Quotient::User object should be obtained from a -//! Quotient::Connection as that has the support functions for modifying profile -//! information. -//! -//! \sa Quotient::User +/** + * @class NeochatRoomMember + * + * This class is a shim around RoomMember that can be safety passed to QML. + * + * Because RoomMember has an internal pointer to a RoomMemberEvent it is + * designed to be created used then quickly discarded as the stste event is changed + * every time the member updates. Passing these to QML which will hold onto them + * can lead to accessing an already deleted Quotient::RoomMemberEvent relatively easily. + * + * This class instead holds a member ID and can therefore always safely create and + * access Quotient::RoomMember objects while being used as long as needed by QML. + * + * @note This is only needed to pass to QML if only accessing in CPP RoomMmeber can + * be used safely. + * + * @note The interface is the same as Quotient::RoomMember. + * + * @sa Quotient::RoomMember + */ class NeochatRoomMember : public QObject { Q_OBJECT @@ -51,145 +57,21 @@ public: explicit NeochatRoomMember(NeoChatRoom *room, const QString &memberId); - /** - * @brief Get unique stable user id - * - * The Matrix user ID is generated by the server and is never changed. - */ QString id() const; - - /** - * @brief The matrix.to URI for the user - * - * Typically used when you want to visit a user (see - * Quotient::UriResolverBase::visitResource()). - * - * @sa Quotient::UriResolverBase::visitResource() - */ Quotient::Uri uri() const; - - //! Whether this member is the local user bool isLocalMember() const; - - //! The membership state of the member Quotient::Membership membershipState() const; - - //! \brief The raw unmodified display name for the user in the given room - //! - //! The value will be empty if no display name has been set. - //! - //! \warning This value is not sanitized or HTML escape so use appropriately. - //! For ready to display values use displayName() or fullName() for - //! plain text and htmlSafeDisplayName() or htmlSafeFullName() fo - //! rich text. - //! - //! \sa displayName(), htmlSafeDisplayName(), fullName(), htmlSafeFullName() QString name() const; - - //! \brief Get the user display name ready for display - //! - //! This function always aims to return something that can be displayed in a - //! UI, so if no display name is set the user's Matrix ID will be returned. - //! - //! The output is sanitized and suitable for a plain text field. For a rich - //! field use htmlSafeDisplayName(). - //! - //! \sa htmlSafeDisplayName() QString displayName() const; - - //! \brief Get the user display name ready for display - //! - //! This function always aims to return something that can be displayed in a - //! UI, so if no display name is set the user's Matrix ID will be returned. - //! - //! The output is sanitized and html escaped ready for a rich text field. For - //! a plain field use displayName(). - //! - //! \sa displayName() QString htmlSafeDisplayName() const; - - //! \brief Get user name and id in a single string - //! - //! This function always aims to return something that can be displayed in a - //! UI, so if no display name is set the just user's Matrix ID will be returned. - //! The constructed string follows the format 'name (id)' which the spec - //! recommends for users disambiguation in a room context and in other places. - //! - //! The output is sanitized and suitable for a plain text field. For a rich - //! field use htmlSafeFullName(). - //! - //! \sa htmlSafeFullName() QString fullName() const; - - //! \brief Get user name and id in a single string - //! - //! This function always aims to return something that can be displayed in a - //! UI, so if no display name is set the just user's Matrix ID will be returned. - //! The constructed string follows the format 'name (id)' which the spec - //! recommends for users disambiguation in a room context and in other places. - //! - //! The output is sanitized and html escaped ready for a rich text field. For - //! a plain field use fullName(). - //! - //! \sa fullName() QString htmlSafeFullName() const; - - //! \brief Get the disambiguated user name - //! - //! This function always aims to return something that can be displayed in a - //! UI, so if no display name is set the just user's Matrix ID will be returned. - //! The output is equivalent to fullName() if there is another user in the room - //! with the same name. Otherwise it is equivalent to displayName(). - //! - //! The output is sanitized and suitable for a plain text field. For a rich - //! field use htmlSafeDisambiguatedName(). - //! - //! \sa htmlSafeDisambiguatedName(), fullName(), displayName() QString disambiguatedName() const; - - //! \brief Get the disambiguated user name - //! - //! This function always aims to return something that can be displayed in a - //! UI, so if no display name is set the just user's Matrix ID will be returned. - //! The output is equivalent to htmlSafeFullName() if there is another user in the room - //! with the same name. Otherwise it is equivalent to htmlSafeDisplayName(). - //! - //! The output is sanitized and html escaped ready for a rich text field. For - //! a plain field use disambiguatedName(). - //! - //! \sa disambiguatedName(), htmlSafeFullName(), htmlSafeDisplayName() QString htmlSafeDisambiguatedName() const; - - //! \brief Hue color component of this user based on the user's Matrix ID - //! - //! The implementation is based on XEP-0392: - //! https://xmpp.org/extensions/xep-0392.html - //! Naming and ranges are the same as QColor's hue methods: - //! https://doc.qt.io/qt-5/qcolor.html#integer-vs-floating-point-precision int hue() const; - - //! \brief HueF color component of this user based on the user's Matrix ID - //! - //! The implementation is based on XEP-0392: - //! https://xmpp.org/extensions/xep-0392.html - //! Naming and ranges are the same as QColor's hue methods: - //! https://doc.qt.io/qt-5/qcolor.html#integer-vs-floating-point-precision qreal hueF() const; - - //! \brief Color based on the user's Matrix ID - //! - //! See https://github.com/quotient-im/libQuotient/wiki/User-color-coding-standard-draft-proposal - //! for the methodology. QColor color() const; - - //! \brief The mxc URL as a string for the user avatar in the room - //! - //! This can be empty if none set. QString avatarMediaId() const; - - //! \brief The mxc URL for the user avatar in the room - //! - //! This can be empty if none set. QUrl avatarUrl() const; Q_SIGNALS: diff --git a/src/timeline/StateComponent.qml b/src/timeline/StateComponent.qml index 0aca69e86..fe75715fc 100644 --- a/src/timeline/StateComponent.qml +++ b/src/timeline/StateComponent.qml @@ -48,10 +48,11 @@ RowLayout { source: root.author?.avatarUrl ?? "" name: root.author?.displayName ?? "" - color: root.author?.color ?? undefined + color: root.author?.color ?? Kirigami.Theme.highlightColor MouseArea { anchors.fill: parent + enabled: root.author cursorShape: Qt.PointingHandCursor onClicked: RoomManager.resolveResource("https://matrix.to/#/" + root.author.id) }