Refactor EventContent

Use proper layouts and get rid of multiple annoyances/bugs like text-wrapping randomly happening in the middle of words, background rectangles being too large, a certain binding loop, etc
2019-09-19 15:49:29 -04:00 · 2019-09-19 15:49:29 -04:00 · d20ab5a348
commit d20ab5a348
parent 4c4d747ecf
11 changed files with 183 additions and 139 deletions
--- a/TODO.md
+++ b/TODO.md
@ -6,8 +6,8 @@
    - Bottom/top bar
  - Uploading (+local echo)
    - Deduplicate uploads
-  - Files, links, video, audio
-  - File thumbnails, ask matrix API?
+  - EventLink
+  - File thumbnails + ask matrix API?
  - Encrypted media
  - Loading animation
  - GIF thumbnails: load the real animated image
@ -19,8 +19,13 @@
  - Video: missing buttons and small size problems
  - Audio: online playback is buggy, must download+play file

-  - Refactor EventContent
-    - No background/padding around medias
+  - With this as eventText: `https://0x0.st/ztXe.png`, shrinking the window
+    near its minimum size (seen at 262px) makes the image preview
+    left padding wrong
+  - In the "Leave me" room, "join > Hi > left" aren't combined
+  - GIF glitchy border
+  - Combined pure media events time looks bad
+  - Avatars shouldn't be vertically centered

  - Copy to X11 selection with new CppUtils class

@ -41,7 +46,7 @@
  - When qml syntax highlighting supports ES6 string interpolation, use that

 - Fixes
-  - GIF glitchy border
+  - Event delegates changing height don't scroll the list
  - When selecting text and scrolling up, selection stops working after a while
    - Ensure all the text that should be copied is copied

--- a/src/python/utils.py
+++ b/src/python/utils.py
@ -44,6 +44,5 @@ def guess_mime(file: IO) -> Optional[str]:

 def plain2html(text: str) -> str:
    return html.escape(text)\
-               .replace(" ", "&nbsp;")\
               .replace("\n", "<br>")\
               .replace("\t", "&nbsp;" * 4)
--- a/src/qml/Base/HRepeater.qml
+++ b/src/qml/Base/HRepeater.qml
@ -9,7 +9,19 @@ Repeater {
        let total = 0

        for (let i = 0;  i < repeater.count; i++) {
-            total += repeater.itemAt(i).implicitWidth
+            let item = repeater.itemAt(i)
+            if (item && item.width) total += item.width
+        }
+
+        return total
+    }
+
+    readonly property int childrenWidth: {
+        let total = 0
+
+        for (let i = 0;  i < repeater.count; i++) {
+            let item = repeater.itemAt(i)
+            if (item && item.width) total += item.width
        }

        return total
--- a/src/qml/Chat/Timeline/EventAudio.qml
+++ b/src/qml/Chat/Timeline/EventAudio.qml
@ -7,10 +7,6 @@ import "../../utils.js" as Utils

 AudioPlayer {
    id: audio
-    width: Math.min(
-        mainColumn.width - eventContent.spacing * 2,
-        theme.chat.message.audioWidth,
-    )

    HoverHandler {
        onHoveredChanged:
--- a/src/qml/Chat/Timeline/EventContent.qml
+++ b/src/qml/Chat/Timeline/EventContent.qml
@ -3,149 +3,173 @@ import QtQuick.Layouts 1.12
 import "../../Base"
 import "../../utils.js" as Utils

-Row {
+HRowLayout {
    id: eventContent
    spacing: theme.spacing / 1.25
+    layoutDirection: onRight ? Qt.RightToLeft: Qt.LeftToRight


    readonly property string eventText: Utils.processedEventText(model)
    readonly property string eventTime: Utils.formatTime(model.date, false)
+    readonly property bool pureMedia: ! eventText && previewLinksRepeater.count

-    readonly property string hoveredLink:
-        nameLabel.hoveredLink || contentLabel.hoveredLink
+    readonly property string hoveredLink: contentLabel.hoveredLink
+    readonly property bool hoveredSelectable: contentHover.hovered

-    readonly property bool hoveredSelectable:
-        nameHover.hovered || contentHover.hovered
+    readonly property int messageBodyWidth:
+        width - (avatarWrapper.visible ? avatarWrapper.width : 0) -
+        totalSpacing


    Item {
-        width: hideAvatar ? 0 : 58
-        height: hideAvatar ? 0 : collapseAvatar ? 1 : smallAvatar ? 28 : 58
-        opacity: hideAvatar || collapseAvatar ? 0 : 1
-        visible: width > 0
+        id: avatarWrapper
+        opacity: collapseAvatar ? 0 : 1
+        visible: ! hideAvatar
+
+        Layout.minimumWidth: 58
+        Layout.minimumHeight: collapseAvatar ? 1 : smallAvatar ? 28 : 58
+        Layout.maximumWidth: Layout.minimumWidth
+        Layout.maximumHeight: Layout.minimumHeight

        HUserAvatar {
            id: avatar
            userId: model.sender_id
            displayName: model.sender_name
            avatarUrl: model.sender_avatar
-            width: hideAvatar ? 0 : 58
-            height: hideAvatar ? 0 : collapseAvatar ? 1 : 58
+            width: parent.width
+            height: collapseAvatar ? 1 : 58
        }
    }

-    Rectangle {
-        color: isOwn?
-               theme.chat.message.ownBackground :
-               theme.chat.message.background
+    HColumnLayout {
+        id: contentColumn
+        Layout.alignment: Qt.AlignVCenter

-        //width: nameLabel.implicitWidth
-        width: Math.min(
-            eventList.width - avatar.width - eventContent.spacing,
-            theme.fontSize.normal * 0.5 * 75,  // 600 with 16px font
+        HSelectableLabel {
+            id: contentLabel
+            container: selectableLabelContainer
+            index: model.index

-            Math.max(
-                nameLabel.visible ? (nameLabel.implicitWidth + 1) : 0,
-
-                contentLabel.implicitWidth + 1,
-
-                previewLinksRepeater.count > 0 ?
-                theme.chat.message.thumbnailWidth : 0,
-            )
-        )
-        height: childrenRect.height
-        y: parent.height / 2 - height / 2
-
-        Column {
-            id: mainColumn
-            width: parent.width
-            spacing: theme.spacing / 1.75
            topPadding: theme.spacing / 1.75
            bottomPadding: topPadding
+            leftPadding: eventContent.spacing
+            rightPadding: leftPadding

-            HSelectableLabel {
-                id: nameLabel
-                width: parent.width
-                visible: ! hideNameLine
-                container: selectableLabelContainer
-                selectable: ! unselectableNameLine
-                leftPadding: eventContent.spacing
-                rightPadding: leftPadding
+            color: theme.chat.message.body
+            wrapMode: TextEdit.Wrap
+            textFormat: Text.RichText
+            text:
+                // CSS
+                theme.chat.message.styleInclude +

-                // This is +0.1 and content is +0 instead of the opposite,
-                // because the eventList is reversed
-                index: model.index + 0.1
+                // Sender name
+                (hideNameLine ? "" : (
+                "<div class='sender'>" +
+                Utils.coloredNameHtml(model.sender_name, model.sender_id) +
+                "</div>")) +

-                text: Utils.coloredNameHtml(model.sender_name, model.sender_id)
-                textFormat: Text.RichText
-                wrapMode: Text.Wrap
-                // horizontalAlignment: onRight ? Text.AlignRight : Text.AlignLeft
-                horizontalAlignment: Text.AlignLeft
+                // Message body
+                eventContent.eventText +

-                function selectAllText() {
-                    // select the sender name, body and date
-                    container.clearSelection()
-                    nameLabel.selectAll()
-                    contentLabel.selectAll()
-                    contentLabel.updateContainerSelectedTexts()
-                }
+                // Time
+                // For some reason, if there's only one space,
+                // times will be on their own lines most of the time.
+                "  " +
+                "<font size=" + theme.fontSize.small +
+                "px color=" + theme.chat.message.date + '>' +
+                eventTime +
+                "</font>" +

-                HoverHandler { id: nameHover }
+                // Local echo icon
+                (model.is_local_echo ?
+                 "&nbsp;<font size=" + theme.fontSize.small +
+                 "px>⏳</font>" : "")
+
+            transform: Translate {
+                x: onRight ?
+                   contentLabel.width - contentLabel.paintedWidth -
+                   contentLabel.leftPadding - contentLabel.rightPadding :
+                   0
            }

-            HSelectableLabel {
-                id: contentLabel
-                visible: Boolean(eventContent.eventText)
-                width: parent.width
-                container: selectableLabelContainer
-                index: model.index
-                leftPadding: eventContent.spacing
-                rightPadding: leftPadding
-                bottomPadding: previewLinksRepeater.count > 0 ?
-                               mainColumn.bottomPadding : 0
+            Layout.maximumWidth: Math.min(
+                // 600px with 16px font
+                theme.fontSize.normal * 0.5 * 75,
+                messageBodyWidth - leftPadding - rightPadding,
+            )

-                text: theme.chat.message.styleInclude +
-                      eventContent.eventText +
-                      // time
-                      // for some reason, if there's only one space,
-                      // times will be on their own lines most of the time.
-                      "  " +
-                      "<font size=" + theme.fontSize.small +
-                      "px color=" + theme.chat.message.date + '>' +
-                      eventTime +
-                      "</font>" +
-                      // local echo icon
-                      (model.is_local_echo ?
-                       "&nbsp;<font size=" + theme.fontSize.small +
-                       "px>⏳</font>" : "")
-
-                color: theme.chat.message.body
-                wrapMode: Text.Wrap
-                textFormat: Text.RichText
-
-                function selectAllText() {
-                    // Select the message body without the date or name
-                    container.clearSelection()
-                    contentLabel.select(
-                        0,
-                        contentLabel.length -
-                        eventTime.length - 1  // - 1: separating space
-                    )
-                    contentLabel.updateContainerSelectedTexts()
-                }
-
-                HoverHandler { id: contentHover }
+            function selectAllText() {
+                // Select the message body without the date or name
+                container.clearSelection()
+                contentLabel.select(
+                    0,
+                    contentLabel.length -
+                    eventTime.length - 1  // - 1: separating space
+                )
+                contentLabel.updateContainerSelectedTexts()
            }

-            Repeater {
-                id: previewLinksRepeater
-                model: eventDelegate.currentItem.links
+            HoverHandler { id: contentHover }

-                EventMediaLoader {
-                    info: eventDelegate.currentItem
-                    mediaUrl: modelData
-                }
+            Rectangle {
+                width: Math.max(
+                    parent.paintedWidth +
+                    parent.leftPadding + parent.rightPadding,
+
+                    previewLinksRepeater.childrenWidth +
+                    (pureMedia ? 0 : parent.leftPadding + parent.rightPadding),
+                )
+                height: contentColumn.height
+                z: -1
+                color: isOwn?
+                       theme.chat.message.ownBackground :
+                       theme.chat.message.background
+            }
+        }
+
+        HRepeater {
+            id: previewLinksRepeater
+            model: eventDelegate.currentItem.links
+
+            EventMediaLoader {
+                info: eventDelegate.currentItem
+                mediaUrl: modelData
+
+                Layout.bottomMargin: contentLabel.bottomPadding * multiply
+                Layout.leftMargin: contentLabel.leftPadding * multiply
+                Layout.rightMargin: contentLabel.rightPadding * multiply
+
+                Layout.minimumWidth:
+                    type === EventDelegate.Media.File ?
+                    theme.chat.message.fileMinWidth : -1
+
+                Layout.preferredWidth:
+                    type === EventDelegate.Media.Image ?
+                    (item ? item.fitSize.width : 0) :
+
+                    type === EventDelegate.Media.Video ?
+                    theme.chat.message.videoWidth :
+
+                    type === EventDelegate.Media.Audio ?
+                    theme.chat.message.audioWidth :
+
+                    -1
+
+                Layout.maximumWidth:
+                    messageBodyWidth - Layout.leftMargin - Layout.rightMargin
+
+                Layout.maximumHeight:
+                    type === EventDelegate.Media.Image && item ?
+                    Utils.fitSize(
+                        Layout.maximumWidth,
+                        item.fitSize.height,
+                        Layout.maximumWidth
+                    ).height : -1
+
+                readonly property int multiply: pureMedia ? 0 : 1
            }
        }
    }
+
+    HSpacer {}
 }
--- a/src/qml/Chat/Timeline/EventDelegate.qml
+++ b/src/qml/Chat/Timeline/EventDelegate.qml
@ -48,9 +48,6 @@ Column {
        onRight ||
        combine

-    readonly property bool unselectableNameLine:
-        hideNameLine && ! (onRight && ! combine)
-
    readonly property int cursorShape:
        eventContent.hoveredLink || hoveredMediaTypeUrl.length > 0 ?
        Qt.PointingHandCursor :
@ -96,7 +93,7 @@ Column {

    EventContent {
        id: eventContent
-        x: onRight ? parent.width - width : 0
+        width: parent.width

        Behavior on x { HNumberAnimation {} }
    }
--- a/src/qml/Chat/Timeline/EventFile.qml
+++ b/src/qml/Chat/Timeline/EventFile.qml
@ -3,11 +3,6 @@ import QtQuick.Layouts 1.12
 import "../../Base"

 HTile {
-    width: Math.min(
-        mainColumn.width - eventContent.spacing * 2,
-        theme.chat.message.thumbnailWidth,
-    )
-
    onLeftClicked: Qt.openUrlExternally(fileUrl)
    onRightClicked: eventDelegate.openContextMenu()

--- a/src/qml/Chat/Timeline/EventImage.qml
+++ b/src/qml/Chat/Timeline/EventImage.qml
@ -1,20 +1,26 @@
 import QtQuick 2.12
 import "../../Base"
+import "../../utils.js" as Utils

 HImage {
    id: image
    sourceSize.width: theme.chat.message.thumbnailWidth
    sourceSize.height: theme.chat.message.thumbnailWidth
-    width: Math.min(
-        mainColumn.width - eventContent.spacing * 2,
-        implicitWidth,
-        theme.chat.message.thumbnailWidth,
-    )
+    width: fitSize.width
+    height: fitSize.height
+
+    // Leaving PreserveAspectFit creates a binding loop, and is uneeded
+    // since we calculate ourself the right size.
+    fillMode: Image.Pad


    // source = thumbnail, fullSource = full original image
    property url fullSource: source

+    readonly property size fitSize: Utils.fitSize(
+        implicitWidth, implicitHeight, theme.chat.message.thumbnailWidth,
+    )
+

    TapHandler {
        onTapped: if (! image.animated) Qt.openUrlExternally(fullSource)
--- a/src/qml/Chat/Timeline/EventVideo.qml
+++ b/src/qml/Chat/Timeline/EventVideo.qml
@ -7,10 +7,6 @@ import "../../utils.js" as Utils

 VideoPlayer {
    id: video
-    width: fullScreen ? implicitWidth : Math.min(
-        mainColumn.width - eventContent.spacing * 2,
-        theme.chat.message.videoWidth,
-    )

    onHoveredChanged:
        eventDelegate.hoveredMediaTypeUrl =
--- a/src/qml/utils.js
+++ b/src/qml/utils.js
@ -167,6 +167,16 @@ function thumbnailParametersFor(width, height) {
 }


+function fitSize(width, height, max) {
+    if (width >= height) {
+        let new_width = Math.min(width, max)
+        return Qt.size(new_width, height / (width / new_width))
+    }
+    let new_height = Math.min(height, max)
+    return Qt.size(width / (height / new_height), new_height)
+}
+
+
 function minutesBetween(date1, date2) {
    return ((date2 - date1) / 1000) / 60
 }
--- a/src/themes/Default.qpl
+++ b/src/themes/Default.qpl
@ -315,6 +315,7 @@ chat:
        string styleSheet:
            "* { white-space: pre-wrap }" +
            "a { color: " + link  + " }" +
+            "p { margin-top: 0 }" +

            "code { font-family: " + fontFamily.mono + "; " +
                   "color: "       + code            + " }" +
@ -327,14 +328,17 @@ chat:
            "h5 { font-size: " + fontSize.small + "px }" +
            "h6 { font-size: " + fontSize.smaller + "px }" +

-            ".quote { color: " + quote + " }"
+            ".sender { margin-bottom: " + spacing / 2 + " }" +
+            ".quote  { color: " + quote + " }"

        string styleInclude:
            '<style type"text/css">\n' + styleSheet + '\n</style>\n'

+        // TODO rename
+        int fileMinWidth: 256
        int thumbnailWidth: 256
-        int videoWidth: 512
-        int audioWidth: 512
+        int videoWidth: 640
+        int audioWidth: 320

    daybreak:
        color background: colors.strongBackground