Reactions - Design review tweaks (#1417)

* Design review tweaks - truncate reactions as per spec - add haptic feedback on reacting - Remove all collapse/expand animations on reactions - tweak reaction and summary padding - Fix alignment of time on reaction summary view - Made sure reaction summary button in horizontal scroller is on screen on appear - update screenshots * Update TimelineReactionsView.swift * Remove dark add more button and check long rtl reactions * Set colour of templated add more button * Fix padding and only inset from the bubble on the leading side as per designs - Fix padding - Only inset from the bubble on the leading side as per designs * Update screenshots * Fix line length warning.
2026-11-03 06:02:10 +00:00 · 2023-07-31 12:16:52 +01:00
parent d38b6f74b1
commit beaf9a8dbf
18 changed files with 90 additions and 46 deletions
@@ -10,6 +10,7 @@
    "version" : 1
  },
  "properties" : {
-    "preserves-vector-representation" : true
+    "preserves-vector-representation" : true,
+    "template-rendering-intent" : "template"
  }
 }
@@ -69,4 +69,12 @@ extension AggregatedReaction {
            mockReaction(key: "🐼", senderIDs: mockIds(1))
        ]
    }
+    
+    static var mockReactionWithLongText: AggregatedReaction {
+        mockReaction(key: "Matrix defines a set of open APIs for decentralised communication.", senderIDs: [alice])
+    }
+    
+    static var mockReactionWithLongTextRTL: AggregatedReaction {
+        mockReaction(key: "تحدد المصفوفة مجموعة من واجهات برمجة التطبيقات المفتوحة للتواصل اللامركزي.", senderIDs: [alice])
+    }
 }
@@ -78,3 +78,12 @@ extension String {
        return whiteSpaces + "\u{2800}"
    }
 }
+
+extension String {
+    func ellipsize(length: Int) -> String {
+        guard count > length else {
+            return self
+        }
+        return "\(prefix(length))…"
+    }
+}
@@ -25,6 +25,8 @@ struct EmojiPickerScreen: View {
    
    @ScaledMetric(relativeTo: .title) var minimumWidth: Double = 64
    
+    private let feedbackGenerator = UIImpactFeedbackGenerator(style: .heavy)
+    
    var body: some View {
        NavigationStack {
            ScrollView {
@@ -33,6 +35,7 @@ struct EmojiPickerScreen: View {
                        Section {
                            ForEach(category.emojis) { emoji in
                                Button {
+                                    feedbackGenerator.impactOccurred()
                                    context.send(viewAction: .emojiTapped(emoji: emoji))
                                } label: {
                                    Text(emoji.value)
@@ -21,8 +21,6 @@ struct TimelineItemPlainStylerView<Content: View>: View {
    @EnvironmentObject private var context: RoomScreenViewModel.Context
    @Environment(\.timelineGroupStyle) private var timelineGroupStyle
    
-    private let feedbackGenerator = UIImpactFeedbackGenerator(style: .heavy)
-    
    let timelineItem: EventBasedTimelineItemProtocol
    @ViewBuilder let content: () -> Content
    
@@ -40,7 +40,7 @@ struct ReactionsSummaryView: View {
    private var reactionButtons: some View {
        ScrollView(.horizontal, showsIndicators: false) {
            ScrollViewReader { scrollView in
-                HStack {
+                HStack(spacing: 8) {
                    ForEach(reactions, id: \.self) { reaction in
                        ReactionSummaryButton(reaction: reaction, highlighted: selectedReactionKey == reaction.key) { key in
                            selectedReactionKey = key
@@ -50,7 +50,9 @@ struct ReactionsSummaryView: View {
                }
                .padding(.horizontal, 20)
                .onAppear {
-                    scrollView.scrollTo(selectedReactionKey, anchor: .leading)
+                    DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
+                        scrollView.scrollTo(selectedReactionKey)
+                    }
                }
                .onChange(of: selectedReactionKey) { _ in
                    scrollView.scrollTo(selectedReactionKey)
@@ -91,7 +93,7 @@ private struct ReactionSummaryButton: View {
    
    var label: some View {
        HStack(spacing: 4) {
-            Text(reaction.key)
+            Text(reaction.displayKey)
                .font(.compound.headingSM)
            if reaction.count > 1 {
                Text(String(reaction.count))
@@ -99,8 +101,8 @@ private struct ReactionSummaryButton: View {
                    .foregroundColor(highlighted ? Color.compound.textOnSolidPrimary : Color.compound.textSecondary)
            }
        }
-        .padding(.vertical, 6)
-        .padding(.horizontal, 8)
+        .padding(.vertical, 8)
+        .padding(.horizontal, 12)
        .background(highlighted ? Color.compound.bgActionPrimaryRest : .clear, in: Capsule())
        .accessibilityElement(children: .combine)
    }
@@ -124,19 +126,20 @@ private struct ReactionSummarySenderView: View {
                                imageProvider: imageProvider)
            
            VStack(alignment: .leading) {
-                Text(displayName)
-                    .font(.compound.bodyMDSemibold)
+                HStack {
+                    Text(displayName)
+                        .font(.compound.bodyMDSemibold)
+                        .frame(maxWidth: .infinity, alignment: .leading)
+                    Text(sender.timestamp.formattedMinimal())
+                        .font(.compound.bodyXS)
+                        .foregroundColor(.compound.textSecondary)
+                }
                Text(sender.senderID)
                    .font(.compound.bodySM)
                    .foregroundColor(.compound.textSecondary)
            }
-            .frame(maxWidth: .infinity, alignment: .leading)
-            
-            Text(sender.timestamp.formattedMinimal())
-                .font(.compound.bodyXS)
-                .foregroundColor(.compound.textSecondary)
        }
-        .frame(maxWidth: .infinity, alignment: .leading)
+        
        .padding(.vertical, 8)
    }
 }
@@ -17,11 +17,9 @@
 import SwiftUI

 struct TimelineReactionsView: View {
-    /// We use a coordinate space for measuring the reactions within their container.
-    /// For some reason when using .local the origin of reactions always shown as (0, 0)
-    private static let flowCoordinateSpace = "flowCoordinateSpace"
    private static let horizontalSpacing: CGFloat = 4
    private static let verticalSpacing: CGFloat = 4
+    private let feedbackGenerator = UIImpactFeedbackGenerator(style: .heavy)
    @EnvironmentObject private var context: RoomScreenViewModel.Context
    @Environment(\.layoutDirection) private var layoutDirection: LayoutDirection
    @Namespace private var animation
@@ -34,6 +32,7 @@ struct TimelineReactionsView: View {
        CollapsibleReactionLayout(itemSpacing: 4, rowSpacing: 4, collapsed: collapsed, rowsBeforeCollapsible: 2) {
            ForEach(reactions, id: \.self) { reaction in
                TimelineReactionButton(reaction: reaction) { key in
+                    feedbackGenerator.impactOccurred()
                    context.send(viewAction: .toggleReaction(key: key, itemID: itemID))
                } showReactionSummary: { key in
                    context.send(viewAction: .reactionSummary(itemID: itemID, key: key))
@@ -44,18 +43,18 @@ struct TimelineReactionsView: View {
                collapsed.toggle()
            } label: {
                TimelineCollapseButtonLabel(collapsed: collapsed)
+                    .transaction { $0.animation = nil }
            }
            .reactionLayoutItem(.expandCollapse)
-            .animation(.easeOut, value: collapsed)
            Button {
                context.send(viewAction: .displayEmojiPicker(itemID: itemID))
            } label: {
                TimelineReactionAddMoreButtonLabel()
            }
-            .animation(.easeOut, value: collapsed)
            .reactionLayoutItem(.addMore)
        }
-        .coordinateSpace(name: Self.flowCoordinateSpace)
+        .animation(.easeInOut(duration: 0.1), value: reactions)
+        .padding(.leading, 4)
    }
 }

@@ -92,9 +91,7 @@ struct TimelineCollapseButtonLabel: View {
        TimelineReactionButtonLabel {
            Text(collapsed ? L10n.screenRoomReactionsShowMore : L10n.screenRoomReactionsShowLess)
                .padding(.vertical, 6)
-                .padding(.horizontal, 8)
-                .layoutPriority(1)
-                .drawingGroup()
+                .padding(.horizontal, 12)
                .font(.compound.bodyMD)
                .foregroundColor(.compound.textPrimary)
        }
@@ -119,7 +116,7 @@ struct TimelineReactionButton: View {
    var label: some View {
        TimelineReactionButtonLabel(isHighlighted: reaction.isHighlighted) {
            HStack(spacing: 4) {
-                Text(reaction.key)
+                Text(reaction.displayKey)
                    .font(.compound.bodyMD)
                if reaction.count > 1 {
                    Text(String(reaction.count))
@@ -128,7 +125,7 @@ struct TimelineReactionButton: View {
                }
            }
            .padding(.vertical, 6)
-            .padding(.horizontal, 8)
+            .padding(.horizontal, 12)
        }
    }
    
@@ -149,6 +146,7 @@ struct TimelineReactionAddMoreButtonLabel: View {
                // matches the height of the text based buttons.
                .padding(.horizontal, 10)
                .frame(maxHeight: .infinity, alignment: .center)
+                .foregroundColor(.compound.iconSecondary)
        }
    }
 }
@@ -159,11 +157,15 @@ struct TimelineReactionViewPreviewsContainer: View {

    var body: some View {
        VStack {
-            TimelineReactionsView(itemID: .init(timelineID: "1"), reactions: Array(AggregatedReaction.mockReactions.prefix(3)), collapsed: .constant(true))
+            TimelineReactionsView(itemID: .init(timelineID: "1"), reactions: [
+                AggregatedReaction.mockReactionWithLongText,
+                AggregatedReaction.mockReactionWithLongTextRTL
+            ], collapsed: .constant(true))
+            TimelineReactionsView(itemID: .init(timelineID: "2"), reactions: Array(AggregatedReaction.mockReactions.prefix(3)), collapsed: .constant(true))
            Divider()
-            TimelineReactionsView(itemID: .init(timelineID: "2"), reactions: AggregatedReaction.mockReactions, collapsed: $collapseState1)
+            TimelineReactionsView(itemID: .init(timelineID: "3"), reactions: AggregatedReaction.mockReactions, collapsed: $collapseState1)
            Divider()
-            TimelineReactionsView(itemID: .init(timelineID: "3"), reactions: AggregatedReaction.mockReactions, collapsed: $collapseState2)
+            TimelineReactionsView(itemID: .init(timelineID: "4"), reactions: AggregatedReaction.mockReactions, collapsed: $collapseState2)
                .environment(\.layoutDirection, .rightToLeft)
        }
        .background(Color.red)
@@ -124,6 +124,7 @@ public struct TimelineItemMenu: View {
    
    let item: EventBasedTimelineItemProtocol
    let actions: TimelineItemMenuActions
+    private let feedbackGenerator = UIImpactFeedbackGenerator(style: .heavy)
    
    public var body: some View {
        VStack {
@@ -218,6 +219,7 @@ public struct TimelineItemMenu: View {
    
    private func reactionButton(for emoji: String) -> some View {
        Button {
+            feedbackGenerator.impactOccurred()
            dismiss()
            context.send(viewAction: .toggleReaction(key: emoji, itemID: item.id))
        } label: {
@@ -18,6 +18,11 @@ import Foundation

 /// Represents all reactions of the same type for a single event.
 struct AggregatedReaction: Hashable {
+    /// Length at which we ellipsize a reaction key for display
+    /// Reactions can be free text, so we need to limit the length
+    /// displayed on screen.
+    private static let maxDisplayChars = 16
+    
    /// The id of the account owner
    let accountOwnerID: String
    /// The reaction that was sent.
@@ -44,4 +49,9 @@ extension AggregatedReaction {
    var isHighlighted: Bool {
        senders.contains(where: { $0.senderID == accountOwnerID })
    }
+    
+    /// The key to be displayed on screen. See `maxDisplayChars`.
+    var displayKey: String {
+        key.ellipsize(length: Self.maxDisplayChars)
+    }
 }
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:c23c7cdf9392e0a9c6d4506147ea724240273c99b7ef8f94bb9e14e3c9e533ba
-size 297394
+oid sha256:8538909b4eb58f736fb4be966726cdf0c000caf3b9e285f98b97326f8cbb5964
+size 297160
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:859eed113d4847d36d2e57a40a3cf725aa10cf04f3aed279f96718aec96148e7
-size 297139
+oid sha256:359a17ae2ab3c7f11348eeab34bb416097704dedbf6ff95d198fbd420e243a02
+size 297042
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:004096089c03d37c0806c889e12cb6d3b48327c7d37cca55e69d3c0410202e8c
-size 392571
+oid sha256:57441eaeba880a5cccab827ac5b9817b2a3605a219553ad37bb67ea78b871d3a
+size 364717
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:20d5d19b75a2f0ac721bb73128c147381a5d76b0cee15e3c0f4c218537384c94
-size 392078
+oid sha256:8b1788c514b01a82938458a571d52550eea9552575a0c85d2b62a0a4bb19fde2
+size 364426
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:2f3c7e90abe271e563a99ebfcb108d6896553ab00599013695cfe265939e9821
-size 298482
+oid sha256:05d9d894f8c655acd034615af8fc5098bf6adb64b475219d1a58da9eea9697e6
+size 298240
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:587bda739d2b923f4d760136520e43ca38911ff93829d7d9aee92a281da69d06
-size 298223
+oid sha256:ef52847f31d85009e37e06340fc4ad21317f907be2fbf4a873a43f4b4740d5c6
+size 298123
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:02e573a04518ef8ae2a2fc05a8b767c8aae2b1f826df324aab28f367663e8f7d
-size 385149
+oid sha256:af701156aa31a33ba3787cdcf29e9c9d0e96bedc017308c88acedae08c80f12c
+size 357151
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:d92f99aa7aacc07b86cd05cff5c4336c037580ca5e45b2dc385154832448826b
-size 384656
+oid sha256:ea0906e449a16419f404bf56233440193dfc071cac5fb668c711eedca657ed76
+size 356860
@@ -80,4 +80,12 @@ class StringTests: XCTestCase {
        result = ""
        XCTAssertEqual(String.generateBreakableWhitespaceEnd(whitespaceCount: count, layoutDirection: .rightToLeft), result)
    }
+    
+    func testEllipsizeWorks() {
+        XCTAssertEqual("ellipsize".ellipsize(length: 5), "ellip…")
+    }
+    
+    func testEllipsizeNotNeeded() {
+        XCTAssertEqual("ellipsize".ellipsize(length: 15), "ellipsize")
+    }
 }