Rework how messages and events are handled

- No more translatable, content_type, show_name_line attrs for TimelineEventReceived. Since they are UI concerns, they are handled directly in QML. - Refactor the EventDelegate and get rid of errors when new items are added to the timeline - Messages, events and emotes all combine correctly. - No more 28px wide avatars for events, to make them uniform with messages.
2019-07-19 23:07:26 -04:00 · 2019-07-19 23:07:26 -04:00 · cea586120e
commit cea586120e
parent ecc2c099f1
9 changed files with 148 additions and 160 deletions
--- a/TODO.md
+++ b/TODO.md
@ -18,7 +18,7 @@
  - Horrible performance for big rooms

 - UI
-  - Need to make events and messages avatars the same size
+  - Don't show typing bar for any of our users
  - Show error box if uploading avatar fails
  - EditAccount page:
    - Device settings
--- a/src/python/events/rooms.py
+++ b/src/python/events/rooms.py
@ -2,14 +2,13 @@
 # This file is part of harmonyqml, licensed under LGPLv3.

 from datetime import datetime
-from typing import Any, Dict, List, Sequence, Type, Union
+from typing import Any, Dict, List, Sequence, Type

 from dataclasses import dataclass, field

 import nio
 from nio.rooms import MatrixRoom

-from ..utils import AutoStrEnum, auto
 from .event import Event


@ -83,15 +82,6 @@ class RoomMemberDeleted(Event):

 # Timeline

-class ContentType(AutoStrEnum):
-    html     = auto()
-    image    = auto()
-    audio    = auto()
-    video    = auto()
-    file     = auto()
-    location = auto()
-
-
@dataclass
 class TimelineEventReceived(Event):
    event_type:    Type[nio.Event] = field()
@ -100,12 +90,8 @@ class TimelineEventReceived(Event):
    sender_id:     str             = field()
    date:          datetime        = field()
    content:       str             = field()
-    content_type:  ContentType     = ContentType.html
    is_local_echo: bool            = False

-    show_name_line: bool                       = False
-    translatable:   Union[bool, Sequence[str]] = True
-
    target_user_id: str = ""

    @classmethod
@ -120,9 +106,3 @@ class TimelineEventReceived(Event):
            target_user_id = getattr(ev, "state_key", "") or "",
            **fields
        )
-
-
-@dataclass
-class TimelineMessageReceived(TimelineEventReceived):
-    show_name_line: bool                       = True
-    translatable:   Union[bool, Sequence[str]] = False
--- a/src/python/matrix_client.py
+++ b/src/python/matrix_client.py
@ -22,7 +22,7 @@ from nio.rooms import MatrixRoom

 from . import __about__
 from .events import rooms, users
-from .events.rooms import TimelineEventReceived, TimelineMessageReceived
+from .events.rooms import TimelineEventReceived
 from .html_filter import HTML_FILTER


@ -174,7 +174,7 @@ class MatrixClient(nio.AsyncClient):
            content["format"]         = "org.matrix.custom.html"
            content["formatted_body"] = to_html

-        TimelineMessageReceived(
+        TimelineEventReceived(
            event_type    = event_type,
            room_id       = room_id,
            event_id      = f"local_echo.{uuid4()}",
@ -306,9 +306,7 @@ class MatrixClient(nio.AsyncClient):

    # Callbacks for nio events

-    # Special %tokens for event contents:
-    # %S = sender's displayname
-    # %T = target (ev.state_key)'s displayname
+    # Content: %1 is the sender, %2 the target (ev.state_key).
    # pylint: disable=unused-argument

    async def onRoomMessageText(self, room, ev, from_past=False) -> None:
@ -316,14 +314,14 @@ class MatrixClient(nio.AsyncClient):
            ev.formatted_body
            if ev.format == "org.matrix.custom.html" else html.escape(ev.body)
        )
-        TimelineMessageReceived.from_nio(room, ev, content=co)
+        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onRoomMessageEmote(self, room, ev, from_past=False) -> None:
-        co = "%S {}".format(HTML_FILTER.filter_inline(
+        co = HTML_FILTER.filter_inline(
            ev.formatted_body
            if ev.format == "org.matrix.custom.html" else html.escape(ev.body)
-        ))
+        )
        TimelineEventReceived.from_nio(room, ev, content=co)


@ -335,21 +333,21 @@ class MatrixClient(nio.AsyncClient):


    async def onRoomCreateEvent(self, room, ev, from_past=False) -> None:
-        co = "%S allowed users on other matrix servers to join this room." \
+        co = "%1 allowed users on other matrix servers to join this room." \
             if ev.federate else \
-             "%S blocked users on other matrix servers from joining this room."
+             "%1 blocked users on other matrix servers from joining this room."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onRoomGuestAccessEvent(self, room, ev, from_past=False) -> None:
        allowed = "allowed" if ev.guest_access else "forbad"
-        co      = f"%S {allowed} guests to join the room."
+        co      = f"%1 {allowed} guests to join the room."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onRoomJoinRulesEvent(self, room, ev, from_past=False) -> None:
        access = "public" if ev.join_rule == "public" else "invite-only"
-        co     = f"%S made the room {access}."
+        co     = f"%1 made the room {access}."
        TimelineEventReceived.from_nio(room, ev, content=co)


@ -368,12 +366,12 @@ class MatrixClient(nio.AsyncClient):
            log.warning("Invalid visibility - %s",
                        json.dumps(ev.__dict__, indent=4))

-        co = f"%S made future room history visible to {to}."
+        co = f"%1 made future room history visible to {to}."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onPowerLevelsEvent(self, room, ev, from_past=False) -> None:
-        co = "%S changed the room's permissions."  # TODO: improve
+        co = "%1 changed the room's permissions."  # TODO: improve
        TimelineEventReceived.from_nio(room, ev, content=co)


@ -388,34 +386,34 @@ class MatrixClient(nio.AsyncClient):

            if membership == "join":
                return (
-                    "%S accepted their invitation."
+                    "%1 accepted their invitation."
                    if prev and prev_membership == "invite" else
-                    "%S joined the room."
+                    "%1 joined the room."
                )

            if membership == "invite":
-                return "%S invited %T to the room."
+                return "%1 invited %2 to the room."

            if membership == "leave":
                if ev.state_key == ev.sender:
                    return (
-                        f"%S declined their invitation.{reason}"
+                        f"%1 declined their invitation.{reason}"
                        if prev and prev_membership == "invite" else
-                        f"%S left the room.{reason}"
+                        f"%1 left the room.{reason}"
                    )

                return (
-                    f"%S withdrew %T's invitation.{reason}"
+                    f"%1 withdrew %2's invitation.{reason}"
                    if prev and prev_membership == "invite" else

-                    f"%S unbanned %T from the room.{reason}"
+                    f"%1 unbanned %2 from the room.{reason}"
                    if prev and prev_membership == "ban" else

-                    f"%S kicked out %T from the room.{reason}"
+                    f"%1 kicked out %2 from the room.{reason}"
                )

            if membership == "ban":
-                return f"%S banned %T from the room.{reason}"
+                return f"%1 banned %2 from the room.{reason}"


        if ev.sender in self.backend.clients:
@ -435,7 +433,7 @@ class MatrixClient(nio.AsyncClient):
            ))

        if changed:
-            return "%S changed their {}.".format(" and ".join(changed))
+            return "%1 changed their {}.".format(" and ".join(changed))

        log.warning("Invalid member event - %s",
                    json.dumps(ev.__dict__, indent=4))
@ -450,40 +448,40 @@ class MatrixClient(nio.AsyncClient):


    async def onRoomAliasEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S set the room's main address to {ev.canonical_alias}."
+        co = f"%1 set the room's main address to {ev.canonical_alias}."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onRoomNameEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S changed the room's name to \"{ev.name}\"."
+        co = f"%1 changed the room's name to \"{ev.name}\"."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onRoomTopicEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S changed the room's topic to \"{ev.topic}\"."
+        co = f"%1 changed the room's topic to \"{ev.topic}\"."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onRoomEncryptionEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S turned on encryption for this room."
+        co = f"%1 turned on encryption for this room."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onOlmEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S sent an undecryptable olm message."
+        co = f"%1 sent an undecryptable olm message."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onMegolmEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S sent an undecryptable message."
+        co = f"%1 sent an undecryptable message."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onBadEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S sent a malformed event."
+        co = f"%1 sent a malformed event."
        TimelineEventReceived.from_nio(room, ev, content=co)


    async def onUnknownBadEvent(self, room, ev, from_past=False) -> None:
-        co = f"%S sent an event this client doesn't understand."
+        co = f"%1 sent an event this client doesn't understand."
        TimelineEventReceived.from_nio(room, ev, content=co)
--- a/src/qml/Chat/Timeline/EventContent.qml
+++ b/src/qml/Chat/Timeline/EventContent.qml
@ -11,15 +11,22 @@ Row {
    spacing: theme.spacing / 2
    // layoutDirection: onRight ? Qt.RightToLeft : Qt.LeftToRight

+    Item {
+        width: hideAvatar ? 0 : 48
+        height: hideAvatar ? 0 : collapseAvatar ? 1 : smallAvatar ? 28 : 48
+        opacity: hideAvatar || collapseAvatar ? 0 : 1
+        visible: width > 0
+
+        // Don't animate w/h of avatar itself! It might affect the sourceSize
+        Behavior on width { HNumberAnimation {} }
+        Behavior on height { HNumberAnimation {} }
+
        HUserAvatar {
            id: avatar
            userId: model.senderId
-        width: onRight ? 0 : model.showNameLine ? 48 : 28
-        height: onRight ? 0 : combine ? 1 : model.showNameLine ? 48 : 28
-        opacity: combine ? 0 : 1
-        visible: width > 0
-        Behavior on width { HNumberAnimation {} }
-        Behavior on height { HNumberAnimation {} }
+            width: hideAvatar ? 0 : 48
+            height: hideAvatar ? 0 : collapseAvatar ? 1 : 48
+        }
    }

    Rectangle {
@ -37,34 +44,34 @@ Row {
            )
        )
        height: nameLabel.height + contentLabel.implicitHeight
+        y: parent.height / 2 - height / 2

        Column {
            spacing: 0
            anchors.fill: parent

            HLabel {
-                width: parent.width
-                height: model.showNameLine && ! onRight && ! combine ?
-                        implicitHeight : 0
-                Behavior on height { HNumberAnimation {} }
-                visible: height > 0
-
                id: nameLabel
+                visible: height > 0
+                width: parent.width
+                height: hideNameLine ? 0 : implicitHeight
+                Behavior on height { HNumberAnimation {} }
+
                text: senderInfo.displayName || model.senderId
                color: Utils.nameColor(avatar.name)
                elide: Text.ElideRight
                horizontalAlignment: onRight ? Text.AlignRight : Text.AlignLeft

-                leftPadding: horizontalPadding
-                rightPadding: horizontalPadding
-                topPadding: verticalPadding
+                leftPadding: theme.spacing
+                rightPadding: leftPadding
+                topPadding: theme.spacing / 2
            }

            HRichLabel {
+                id: contentLabel
                width: parent.width

-                id: contentLabel
-                text: Utils.translatedEventContent(model) +
+                text: Utils.processedEventText(model) +
                      // time
                      "&nbsp;&nbsp;<font size=" + theme.fontSize.small +
                      "px color=" + theme.chat.message.date + ">" +
@ -78,10 +85,10 @@ Row {
                color: theme.chat.message.body
                wrapMode: Text.Wrap

-                leftPadding: horizontalPadding
-                rightPadding: horizontalPadding
-                topPadding: nameLabel.visible ? 0 : verticalPadding
-                bottomPadding: verticalPadding
+                leftPadding: theme.spacing
+                rightPadding: leftPadding
+                topPadding: nameLabel.visible ? 0 : bottomPadding
+                bottomPadding: theme.spacing / 2
            }
        }
    }
--- a/src/qml/Chat/Timeline/EventDelegate.qml
+++ b/src/qml/Chat/Timeline/EventDelegate.qml
@ -4,76 +4,46 @@
 import QtQuick 2.12
 import QtQuick.Layouts 1.12
 import "../../Base"
-import "../../utils.js" as Utils

 Column {
-    id: roomEventDelegate
+    id: eventDelegate

-    function minsBetween(date1, date2) {
-        return Math.round((((date2 - date1) % 86400000) % 3600000) / 60000)
+    // Remember timeline goes from newest message at index 0 to oldest
+    property var previousItem: eventList.model.get(model.index + 1)
+    property var nextItem: eventList.model.get(model.index - 1)
+
+    property int modelCount: eventList.model.count
+    onModelCountChanged: {
+        previousItem = eventList.model.get(model.index + 1)
+        nextItem     = eventList.model.get(model.index - 1)
    }

-    function getPreviousItem(nth=1) {
-        // Remember, index 0 = newest bottomest message
-        return eventList.model.count - 1 > model.index + nth ?
-                eventList.model.get(model.index + nth) : null
-    }
+    property var senderInfo: senderInfo = users.find(model.senderId)

-    property var previousItem: getPreviousItem()
-    signal reloadPreviousItem()
-    onReloadPreviousItem: previousItem = getPreviousItem()
+    property bool isOwn: chatPage.userId === model.senderId
+    property bool onRight: eventList.ownEventsOnRight && isOwn
+    property bool combine: eventList.canCombine(previousItem, model)
+    property bool talkBreak: eventList.canTalkBreak(previousItem, model)
+    property bool dayBreak: eventList.canDayBreak(previousItem, model)

-    property var senderInfo: null
-    Component.onCompleted: senderInfo = users.find(model.senderId)
+    readonly property bool smallAvatar:
+        eventList.canCombine(model, nextItem) &&
+        (model.eventType == "RoomMessageEmote" ||
+         ! model.eventType.startsWith("RoomMessage"))

-    readonly property bool isOwn: chatPage.userId === model.senderId
-    readonly property bool onRight: eventList.ownEventsOnRight && isOwn
+    readonly property bool collapseAvatar: combine
+    readonly property bool hideAvatar: onRight

-    readonly property bool isFirstEvent: model.eventType == "RoomCreateEvent"
-
-    // Item roles may not be loaded yet, reason for all these checks
-    readonly property bool combine: Boolean(
-        model.date &&
-        previousItem && previousItem.eventType && previousItem.date &&
-        Utils.eventIsMessage(previousItem) == Utils.eventIsMessage(model) &&
-
-        // RoomMessageEmote are shown inline-style
-        ! (previousItem.eventType == "RoomMessageEmote" &&
-           model.eventType != "RoomMessageEmote") &&
-        ! (previousItem.eventType != "RoomMessageEmote" &&
-           model.eventType == "RoomMessageEmote") &&
-
-        ! talkBreak &&
-        ! dayBreak &&
-        previousItem.senderId === model.senderId &&
-        minsBetween(previousItem.date, model.date) <= 5
-    )
-
-    readonly property bool dayBreak: Boolean(
-        isFirstEvent ||
-        model.date && previousItem && previousItem.date &&
-        model.date.getDate() != previousItem.date.getDate()
-    )
-
-    readonly property bool talkBreak: Boolean(
-        model.date && previousItem && previousItem.date &&
-        ! dayBreak &&
-        minsBetween(previousItem.date, model.date) >= 20
-    )
-
-
-    readonly property int horizontalPadding: theme.spacing
-    readonly property int verticalPadding: theme.spacing / 2
-
-    ListView.onAdd: {
-        let nextDelegate = eventList.contentItem.children[index]
-        if (nextDelegate) { nextDelegate.reloadPreviousItem() }
-    }
+    readonly property bool hideNameLine:
+        model.eventType == "RoomMessageEmote" ||
+        ! model.eventType.startsWith("RoomMessage") ||
+        onRight ||
+        combine

    width: eventList.width

    topPadding:
-        isFirstEvent ? 0 :
+        model.eventType == "RoomCreateEvent" ? 0 :
        dayBreak  ? theme.spacing * 4 :
        talkBreak ? theme.spacing * 6 :
        combine   ? theme.spacing / 2 :
@ -81,7 +51,7 @@ Column {

    Loader {
        source: dayBreak ? "Daybreak.qml" : ""
-        width: roomEventDelegate.width
+        width: eventDelegate.width
    }

    EventContent {
--- a/src/qml/Chat/Timeline/EventList.qml
+++ b/src/qml/Chat/Timeline/EventList.qml
@ -4,6 +4,7 @@
 import QtQuick 2.12
 import SortFilterProxyModel 0.2
 import "../../Base"
+import "../../utils.js" as Utils

 HRectangle {
    property alias listView: eventList
@ -14,6 +15,37 @@ HRectangle {
        id: eventList
        clip: true

+        function canCombine(item, itemAfter) {
+            if (! item || ! itemAfter) { return false }
+
+            return Boolean(
+                ! canTalkBreak(item, itemAfter) &&
+                ! canDayBreak(item, itemAfter) &&
+                item.senderId === itemAfter.senderId &&
+                Utils.minutesBetween(item.date, itemAfter.date) <= 5
+            )
+        }
+
+        function canTalkBreak(item, itemAfter) {
+            if (! item || ! itemAfter) { return false }
+
+            return Boolean(
+                ! canDayBreak(item, itemAfter) &&
+                Utils.minutesBetween(item.date, itemAfter.date) >= 20
+            )
+        }
+
+        function canDayBreak(item, itemAfter) {
+            if (! item || ! itemAfter || ! item.date || ! itemAfter.date) {
+                return false
+            }
+
+            return Boolean(
+                itemAfter.eventType == "RoomCreateEvent" ||
+                item.date.getDate() != itemAfter.date.getDate()
+            )
+        }
+
        model: HListModel {
            sourceModel: timelines

--- a/src/qml/EventHandlers/rooms.js
+++ b/src/qml/EventHandlers/rooms.js
@ -71,14 +71,12 @@ function onRoomForgotten(userId, roomId) {


 function onTimelineEventReceived(
-    eventType, roomId, eventId, senderId, date, content,
-    contentType, isLocalEcho, showNameLine, translatable, targetUserId
+    eventType, roomId, eventId, senderId, date, content, isLocalEcho,
+    targetUserId
 ) {
    let item = {
        eventType: py.getattr(eventType, "__name__"),
-
-        roomId, eventId, senderId, date, content, contentType, isLocalEcho,
-        showNameLine, translatable, targetUserId
+        roomId, eventId, senderId, date, content, isLocalEcho, targetUserId
    }

    if (isLocalEcho) {
--- a/src/qml/SidePane/RoomDelegate.qml
+++ b/src/qml/SidePane/RoomDelegate.qml
@ -52,8 +52,9 @@ HInteractiveRectangle {
                        if (! ev) { return "" }

                        if (ev.eventType == "RoomMessageEmote" ||
-                            ! Utils.eventIsMessage(ev)) {
-                            return Utils.translatedEventContent(ev)
+                            ! ev.eventType.startsWith("RoomMessage"))
+                        {
+                            return Utils.processedEventText(ev)
                        }

                        return Utils.coloredNameHtml(
--- a/src/qml/utils.js
+++ b/src/qml/utils.js
@ -77,26 +77,23 @@ function escapeHtml(string) {
 }


-function eventIsMessage(ev) {
-    return /^RoomMessage($|[A-Z])/.test(ev.eventType)
-}
-
-
-function translatedEventContent(ev) {
-    // ev: timelines item
-    if (ev.translatable == false) { return ev.content }
-
-    // %S → sender display name
+function processedEventText(ev) {
+    if (ev.eventType == "RoomMessageEmote") {
        let name = users.find(ev.senderId).displayName
-    let text = ev.content.replace("%S", coloredNameHtml(name, ev.senderId))
-
-    // %T → target (event state_key) display name
-    if (ev.targetUserId) {
-        let tname = users.find(ev.targetUserId).displayName
-        text = text.replace("%T", coloredNameHtml(tname, ev.targetUserId))
+        return "<i>" + coloredNameHtml(name) + " " + ev.content + "</i>"
    }

-    return qsTr(text)
+    if (ev.eventType.startsWith("RoomMessage")) { return ev.content }
+
+    let name = users.find(ev.senderId).displayName
+    let text = qsTr(ev.content).arg(coloredNameHtml(name, ev.senderId))
+
+    if (text.includes("%2") && ev.targetUserId) {
+        let tname = users.find(ev.targetUserId).displayName
+        text = text.arg(coloredNameHtml(tname, ev.targetUserId))
+    }
+
+    return text
 }


@ -132,3 +129,8 @@ function thumbnailParametersFor(width, height) {

    return {width: 32, height: 32, fillMode: Image.PreserveAspectCrop}
 }
+
+
+function minutesBetween(date1, date2) {
+    return Math.round((((date2 - date1) % 86400000) % 3600000) / 60000)
+}