fix: Show all reactions of a message and keep their order stable

NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift

@@ -14,6 +14,22 @@ import UIKit
     public weak var reactionsDelegate: ReactionsViewDelegate?
     var reactions: [NCChatReaction] = []
 
+    /// Spacing between two reactions
+    private static let itemSpacing: CGFloat = 8
+
+    /// Width of the fade shown at an edge that has more reactions behind it
+    private static let scrollFadeWidth: CGFloat = 16
+
+    private let feedbackGenerator = UIImpactFeedbackGenerator(style: .light)
+
+    private lazy var scrollFadeLayer: CAGradientLayer = {
+        let fadeLayer = CAGradientLayer()
+        fadeLayer.startPoint = .init(x: 0, y: 0.5)
+        fadeLayer.endPoint = .init(x: 1, y: 0.5)
+
+        return fadeLayer
+    }()
+
     /// Tracks the touch start time to differentiate quick taps from long presses
     private var touchBeganTime: Date?
 
@@ -42,6 +58,7 @@ import UIKit
 
     override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) {
         touchBeganTime = Date()
+        feedbackGenerator.prepare()
         super.touchesBegan(touches, with: event)
     }
 
@@ -53,6 +70,50 @@ import UIKit
     func updateReactions(reactions: [NCChatReaction]) {
         self.reactions = reactions
         self.reloadData()
+
+        // Cells keep their ReactionsView across reuse, so without this it stays at the width of the
+        // reactions it showed before and silently clips the new ones
+        self.invalidateIntrinsicContentSize()
+
+        // A reused view might still be scrolled to where the previous message's reactions were
+        self.setContentOffset(.zero, animated: false)
+    }
+
+    // MARK: - Scroll fade
+
+    override func layoutSubviews() {
+        super.layoutSubviews()
+
+        // Also called while scrolling, so the fade follows the content offset
+        self.updateScrollFade()
+    }
+
+    /// Fades out an edge that can be scrolled towards, so it is visible that there are more reactions
+    private func updateScrollFade() {
+        guard self.bounds.width > 0 else { return }
+
+        let canScrollToLeading = self.contentOffset.x > 1
+        let canScrollToTrailing = self.contentOffset.x + self.bounds.width < self.contentSize.width - 1
+
+        guard canScrollToLeading || canScrollToTrailing else {
+            self.layer.mask = nil
+            return
+        }
+
+        let fadeLayer = self.scrollFadeLayer
+        self.layer.mask = fadeLayer
+
+        let opaque = UIColor.white.cgColor
+        let clear = UIColor.clear.cgColor
+        let fade = min(ReactionsView.scrollFadeWidth, self.bounds.width / 3) / self.bounds.width
+
+        // The mask is part of the scroll view's layer, so it has to be moved along with the content
+        CATransaction.begin()
+        CATransaction.setDisableActions(true)
+        fadeLayer.frame = .init(origin: self.contentOffset, size: self.bounds.size)
+        fadeLayer.colors = [canScrollToLeading ? clear : opaque, opaque, opaque, canScrollToTrailing ? clear : opaque]
+        fadeLayer.locations = [0, NSNumber(value: fade), NSNumber(value: 1 - fade), 1]
+        CATransaction.commit()
     }
 
     func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
@@ -64,16 +125,16 @@ import UIKit
     }
 
     func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, minimumInteritemSpacingForSectionAt section: Int) -> CGFloat {
-        return 8
+        return ReactionsView.itemSpacing
     }
 
     func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, minimumLineSpacingForSectionAt section: Int) -> CGFloat {
-        return 8
+        return ReactionsView.itemSpacing
     }
 
     func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize {
         if indexPath.row < reactions.count {
-            return ReactionsViewCell().sizeForReaction(reaction: reactions[indexPath.row])
+            return ReactionsViewCell.sizeForReaction(reaction: reactions[indexPath.row])
         }
         return CGSize(width: 50, height: 30)
     }
@@ -99,11 +160,17 @@ import UIKit
         }
 
         if indexPath.row < reactions.count {
+            self.feedbackGenerator.impactOccurred()
             self.reactionsDelegate?.didSelectReaction(reaction: reactions[indexPath.row])
         }
     }
 
     override var intrinsicContentSize: CGSize {
-        return .init(width: self.collectionViewLayout.collectionViewContentSize.width, height: UICollectionView.noIntrinsicMetric)
+        // Not collectionViewContentSize: the flow layout only recomputes that while laying out, so right
+        // after reloadData() it still reports the width of the previous reactions
+        let width = self.reactions.reduce(0) { $0 + ReactionsViewCell.sizeForReaction(reaction: $1).width }
+            + CGFloat(max(self.reactions.count - 1, 0)) * ReactionsView.itemSpacing
+
+        return .init(width: width, height: UICollectionView.noIntrinsicMetric)
     }
 }

NextcloudTalk/Chat/Chat views/Reactions/ReactionsViewCell.swift

@@ -40,19 +40,19 @@ import UIKit
         return NCAppBranding.elementColor()
     }
 
-    func sizeForReaction(reaction: NCChatReaction) -> CGSize {
+    static func sizeForReaction(reaction: NCChatReaction) -> CGSize {
         let text = textForReaction(reaction: reaction)
         var size = CGSize(width: text.width(withConstrainedHeight: 30, font: .systemFont(ofSize: 13.0)), height: 30)
         size.width += 20
         return size
     }
 
-    func textForReaction(reaction: NCChatReaction) -> String {
+    static func textForReaction(reaction: NCChatReaction) -> String {
         return reaction.reaction + " " + String(reaction.count)
     }
 
     func setReaction(reaction: NCChatReaction) {
-        label.text = textForReaction(reaction: reaction)
+        label.text = ReactionsViewCell.textForReaction(reaction: reaction)
         label.textColor = reaction.userReacted ? highlightedTextColor() : defaultTextColor()
         label.backgroundColor = reaction.userReacted ? highlightedBackgroundColor() : defaultBackgroundColor()
     }

NextcloudTalk/Chat/NCChatMessage.swift

@@ -456,18 +456,8 @@ import SwiftyAttributes
     // MARK: - Reactions
 
     public func reactionsArray() -> [NCChatReaction] {
-        var reactionsArray: [NCChatReaction] = []
-
-        // Grab message reactions
-        let reactionsDict = self.reactionsDictionary()
-        for reactionKey in reactionsDict.keys {
-            // We need to keep this check for users who installed v14.0 (beta 1)
-            if reactionKey == "self" { continue }
-
-            let count = (reactionsDict[reactionKey] as? NSNumber)?.intValue ?? 0
-            let reaction = NCChatReaction(reaction: reactionKey, count: count, userReacted: false, state: .set)
-            reactionsArray.append(reaction)
-        }
+        // Already in the order the reactions were first used
+        var reactionsArray = self.storedReactions()
 
         // Set flag for own reactions
         for ownReaction in self.reactionsSelfArray() {
@@ -479,10 +469,11 @@ import SwiftyAttributes
         // Merge with temporary reactions
         self.mergeTemporaryReactions(into: &reactionsArray)
 
-        // Sort by reactions count
-        reactionsArray.sort { $0.count > $1.count }
-
-        return reactionsArray
+        // Highest count first, keeping the order for equal counts so only a reaction whose count changed
+        // moves. sorted(by:) is not guaranteed to be stable, so the position is the tie-break.
+        return reactionsArray.enumerated()
+            .sorted { $0.element.count != $1.element.count ? $0.element.count > $1.element.count : $0.offset < $1.offset }
+            .map { $0.element }
     }
 
     // MARK: - Updating
@@ -522,7 +513,9 @@ import SwiftyAttributes
         managedChatMessage.systemMessage = chatMessage.systemMessage
         managedChatMessage.isReplyable = chatMessage.isReplyable
         managedChatMessage.messageType = chatMessage.messageType
-        managedChatMessage.reactionsJSONString = chatMessage.reactionsJSONString
+        // Reactions we already know keep their position, new ones are appended
+        managedChatMessage.reactionsJSONString = NCChatMessage.reactionsJSONString(for: chatMessage.storedReactions(),
+                                                                                  keepingOrderOf: managedChatMessage.storedReactions())
         managedChatMessage.expirationTimestamp = chatMessage.expirationTimestamp
         managedChatMessage.isMarkdownMessage = chatMessage.isMarkdownMessage
         managedChatMessage.lastEditActorId = chatMessage.lastEditActorId
@@ -575,12 +568,49 @@ import SwiftyAttributes
 
 extension NCChatMessage {
 
-    @nonobjc private func reactionsDictionary() -> [String: Any] {
+    /// The stored reactions, in the order they were first used.
+    ///
+    /// Stored as `[emoji, count]` pairs so the order survives. Older messages hold a JSON object, whose
+    /// key order is lost while parsing, so those are ordered by emoji to give them a fixed order too.
+    @nonobjc internal func storedReactions() -> [NCChatReaction] {
         guard let data = self.reactionsJSONString?.data(using: .utf8),
-              let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any]
-        else { return [:] }
+              let json = try? JSONSerialization.jsonObject(with: data)
+        else { return [] }
 
-        return dict
+        if let pairs = json as? [[Any]] {
+            return pairs.compactMap { NCChatMessage.reaction(fromEmoji: $0.first, count: $0.last) }
+        }
+
+        if let dictionary = json as? [String: Any] {
+            return dictionary
+                .compactMap { NCChatMessage.reaction(fromEmoji: $0.key, count: $0.value) }
+                .sorted { $0.reaction < $1.reaction }
+        }
+
+        return []
+    }
+
+    /// Serialises reactions, keeping the position of the ones in `storedReactions` and appending the rest
+    @nonobjc internal static func reactionsJSONString(for reactions: [NCChatReaction], keepingOrderOf storedReactions: [NCChatReaction]) -> String? {
+        guard !reactions.isEmpty else { return nil }
+
+        let counts = Dictionary(reactions.map { ($0.reaction, $0.count) }, uniquingKeysWith: { first, _ in first })
+        let knownEmoji = storedReactions.map { $0.reaction }.filter { counts[$0] != nil }
+        let newEmoji = reactions.map { $0.reaction }.filter { !knownEmoji.contains($0) }
+        let pairs = (knownEmoji + newEmoji).map { [$0, counts[$0] ?? 0] as [Any] }
+
+        guard let data = try? JSONSerialization.data(withJSONObject: pairs) else { return nil }
+
+        return String(data: data, encoding: .utf8)
+    }
+
+    @nonobjc private static func reaction(fromEmoji emoji: Any?, count: Any?) -> NCChatReaction? {
+        // The "self" key needs to be skipped for users who installed v14.0 (beta 1)
+        guard let emoji = emoji as? String, emoji != "self",
+              let reactionCount = (count as? NSNumber)?.intValue, reactionCount > 0
+        else { return nil }
+
+        return NCChatReaction(reaction: emoji, count: reactionCount, userReacted: false, state: .set)
     }
 
     @nonobjc private func reactionsSelfArray() -> [String] {

NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift

@@ -8,6 +8,101 @@ import XCTest
 
 final class UnitBaseChatTableViewCellTest: TestBaseRealm {
 
+    // MARK: - Reactions
+
+    private func makeReactionsView(inContainerOfWidth width: CGFloat) -> (container: UIView, reactionsView: ReactionsView) {
+        // Mirrors how BaseChatTableViewCell.showReactionsPart() builds and constrains the view
+        let container = UIView(frame: .init(x: 0, y: 0, width: width, height: 40))
+        let flowLayout = UICollectionViewFlowLayout()
+        flowLayout.scrollDirection = .horizontal
+
+        let reactionsView = ReactionsView(frame: .init(x: 0, y: 0, width: 50, height: 30), collectionViewLayout: flowLayout)
+        reactionsView.translatesAutoresizingMaskIntoConstraints = false
+        container.addSubview(reactionsView)
+
+        NSLayoutConstraint.activate([
+            reactionsView.leadingAnchor.constraint(equalTo: container.leadingAnchor),
+            reactionsView.topAnchor.constraint(equalTo: container.topAnchor),
+            reactionsView.heightAnchor.constraint(equalToConstant: 30),
+            reactionsView.trailingAnchor.constraint(lessThanOrEqualTo: container.trailingAnchor)
+        ])
+
+        return (container, reactionsView)
+    }
+
+    private func reactions(_ emojis: [String]) -> [NCChatReaction] {
+        return emojis.map { NCChatReaction(reaction: $0, count: 1, userReacted: false, state: .set) }
+    }
+
+    // A cell keeps its ReactionsView across reuse, so more reactions than the previous message had
+    // must widen it again instead of being clipped to the width it showed before
+    func testReactionsViewWidthFollowsTheReactionsItShows() throws {
+        let (container, reactionsView) = makeReactionsView(inContainerOfWidth: 1000)
+
+        reactionsView.updateReactions(reactions: reactions(["👍", "❤️"]))
+        container.setNeedsLayout()
+        container.layoutIfNeeded()
+        let widthForTwo = reactionsView.frame.width
+
+        reactionsView.updateReactions(reactions: reactions(["👍", "❤️", "😀", "🎉", "🚀"]))
+        container.setNeedsLayout()
+        container.layoutIfNeeded()
+        let widthForFive = reactionsView.frame.width
+
+        XCTAssertGreaterThan(widthForFive, widthForTwo,
+                             "Showing more reactions must widen the view, not clip them")
+        XCTAssertEqual(widthForFive, reactionsView.collectionViewLayout.collectionViewContentSize.width, accuracy: 0.5,
+                       "With enough room available, all reactions must fit without scrolling")
+
+        reactionsView.updateReactions(reactions: reactions(["👍"]))
+        container.setNeedsLayout()
+        container.layoutIfNeeded()
+        XCTAssertLessThan(reactionsView.frame.width, widthForTwo,
+                          "Showing fewer reactions must shrink the view again")
+    }
+
+    // Reactions that don't fit are only reachable by scrolling, which the fade is what makes visible
+    func testReactionsViewFadesTheEdgeThatCanBeScrolledTowards() throws {
+        let (container, reactionsView) = makeReactionsView(inContainerOfWidth: 120)
+
+        reactionsView.updateReactions(reactions: reactions(["👍", "❤️", "😀", "🎉", "🚀"]))
+        container.setNeedsLayout()
+        container.layoutIfNeeded()
+
+        XCTAssertGreaterThan(reactionsView.contentSize.width, reactionsView.bounds.width,
+                             "This case is only meaningful when the reactions don't fit")
+        XCTAssertNotNil(reactionsView.layer.mask,
+                        "Reactions that don't fit must be faded out at the edge they can be scrolled towards")
+
+        let (roomyContainer, roomyReactionsView) = makeReactionsView(inContainerOfWidth: 1000)
+
+        roomyReactionsView.updateReactions(reactions: reactions(["👍", "❤️"]))
+        roomyContainer.setNeedsLayout()
+        roomyContainer.layoutIfNeeded()
+
+        XCTAssertNil(roomyReactionsView.layer.mask,
+                     "Reactions that all fit must not be faded out, there is nothing to scroll to")
+    }
+
+    // A reused view must not keep the scroll position, or the new reactions start off screen
+    func testReactionsViewResetsScrollPositionOnReuse() throws {
+        let (container, reactionsView) = makeReactionsView(inContainerOfWidth: 120)
+
+        reactionsView.updateReactions(reactions: reactions(["👍", "❤️", "😀", "🎉", "🚀"]))
+        container.setNeedsLayout()
+        container.layoutIfNeeded()
+
+        reactionsView.setContentOffset(.init(x: 80, y: 0), animated: false)
+        XCTAssertEqual(reactionsView.contentOffset.x, 80)
+
+        reactionsView.updateReactions(reactions: reactions(["🚀", "🎉", "😀"]))
+        container.setNeedsLayout()
+        container.layoutIfNeeded()
+
+        XCTAssertEqual(reactionsView.contentOffset.x, 0,
+                       "A view showing another message's reactions must start at the first one")
+    }
+
     func testSharedDeckCardQuote() throws {
         let deckObject = """
 {