From ed8d73ff150dc8857abc81c36407550e098527d4 Mon Sep 17 00:00:00 2001 From: Ivan Sein Date: Wed, 26 Aug 2026 18:42:25 +0200 Subject: [PATCH 1/6] fix(chat): Size the reactions view to the reactions it shows Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ivan Sein --- .../Chat views/Reactions/ReactionsView.swift | 28 ++++++- .../Chat/UnitBaseChatTableViewCellTest.swift | 75 +++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift index 06e557c08..3e8b567b3 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift +++ b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift @@ -14,6 +14,9 @@ import UIKit public weak var reactionsDelegate: ReactionsViewDelegate? var reactions: [NCChatReaction] = [] + /// Spacing between two reactions + private static let itemSpacing: CGFloat = 8 + /// Tracks the touch start time to differentiate quick taps from long presses private var touchBeganTime: Date? @@ -53,6 +56,14 @@ import UIKit func updateReactions(reactions: [NCChatReaction]) { self.reactions = reactions self.reloadData() + + // Cells keep their ReactionsView across reuse, so without invalidating it here the view keeps + // the width of the reactions it showed before: too narrow silently clips the new ones (they are + // then only reachable by scrolling), too wide leaves a gap. + self.invalidateIntrinsicContentSize() + + // A reused view might still be scrolled to where the previous message's reactions were + self.setContentOffset(.zero, animated: false) } func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int { @@ -64,11 +75,11 @@ 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 { @@ -104,6 +115,17 @@ import UIKit } override var intrinsicContentSize: CGSize { - return .init(width: self.collectionViewLayout.collectionViewContentSize.width, height: UICollectionView.noIntrinsicMetric) + // Measured from the reactions themselves instead of from collectionViewContentSize: the flow + // layout only recomputes that while laying out, so right after reloadData() it would still + // report the width of the reactions this view showed before. + guard !self.reactions.isEmpty else { + return .init(width: 0, height: UICollectionView.noIntrinsicMetric) + } + + let sizingCell = ReactionsViewCell() + let width = self.reactions.reduce(0) { $0 + sizingCell.sizeForReaction(reaction: $1).width } + + CGFloat(self.reactions.count - 1) * ReactionsView.itemSpacing + + return .init(width: width, height: UICollectionView.noIntrinsicMetric) } } diff --git a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift index d7463476c..f30f9f037 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift @@ -8,6 +8,81 @@ 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: the + // collection view is sized by its own intrinsic content size, capped by the available width. + 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 showing a message with more reactions than the + // previous one must widen it again. Otherwise the collection view stays at the width of whatever + // it showed before and silently clips the rest, which is only reachable by scrolling. + 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") + } + + // A reused view must not keep the scroll position of the message it showed before, or the first + // reactions of the new message 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 = """ { From 63c7ae4b7f2738fbe0aed8cf6cd0de3170df2f40 Mon Sep 17 00:00:00 2001 From: Ivan Sein Date: Wed, 26 Aug 2026 18:45:18 +0200 Subject: [PATCH 2/6] feat(chat): Fade reactions that don't fit to show they can be scrolled Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ivan Sein --- .../Chat views/Reactions/ReactionsView.swift | 52 +++++++++++++++++++ .../Chat/UnitBaseChatTableViewCellTest.swift | 25 +++++++++ 2 files changed, 77 insertions(+) diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift index 3e8b567b3..71ce639c6 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift +++ b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift @@ -66,6 +66,58 @@ import UIKit self.setContentOffset(.zero, animated: false) } + // MARK: - Scroll fade + + /// Width of the fade shown at an edge that has more reactions behind it + private static let scrollFadeWidth: CGFloat = 16 + + private var scrollFadeLayer: CAGradientLayer? + + override func layoutSubviews() { + super.layoutSubviews() + + // Also called while scrolling, so the fade follows the content offset + self.updateScrollFade() + } + + /// Fades out the reactions at an edge that can still be scrolled towards, so it becomes visible + /// that a message has more reactions than there is room for. + 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 + self.scrollFadeLayer = nil + return + } + + let fadeLayer: CAGradientLayer + if let scrollFadeLayer { + fadeLayer = scrollFadeLayer + } else { + fadeLayer = CAGradientLayer() + fadeLayer.startPoint = .init(x: 0, y: 0.5) + fadeLayer.endPoint = .init(x: 1, y: 0.5) + self.scrollFadeLayer = fadeLayer + 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 { return reactions.count } diff --git a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift index f30f9f037..6d99a8544 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift @@ -63,6 +63,31 @@ final class UnitBaseChatTableViewCellTest: TestBaseRealm { "Showing fewer reactions must shrink the view again") } + // When a message has more reactions than the bubble has room for, the remaining ones are only + // reachable by scrolling. Fading out the edge that can be scrolled towards is what makes that + // visible instead of looking like the message simply has fewer reactions. + 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 of the message it showed before, or the first // reactions of the new message start off screen. func testReactionsViewResetsScrollPositionOnReuse() throws { From 30147a35be3ff533062f8a90ce16130e4cb251c1 Mon Sep 17 00:00:00 2001 From: Ivan Sein Date: Thu, 27 Aug 2026 12:43:07 +0200 Subject: [PATCH 3/6] fix(chat): Keep reactions in the order they were first used Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ivan Sein --- NextcloudTalk/Chat/NCChatMessage.swift | 78 +++++++++++++------ .../Unit/Chat/UnitNCChatMessageTest.swift | 71 +++++++++++++++++ 2 files changed, 127 insertions(+), 22 deletions(-) diff --git a/NextcloudTalk/Chat/NCChatMessage.swift b/NextcloudTalk/Chat/NCChatMessage.swift index eee32b580..73b867cb6 100644 --- a/NextcloudTalk/Chat/NCChatMessage.swift +++ b/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() { @@ -476,13 +466,16 @@ import SwiftyAttributes } } - // Merge with temporary reactions + // Merge with temporary reactions, appending a reaction we just added like any other new one self.mergeTemporaryReactions(into: &reactionsArray) - // Sort by reactions count - reactionsArray.sort { $0.count > $1.count } - - return reactionsArray + // Highest count first, keeping the order for reactions with the same count, so that only a + // reaction whose count changed moves and a new one is appended. sorted(by:) is not guaranteed to + // be stable, so the current position is the tie-break. The web client shows the same order: its + // reactions keep the insertion order of the store and Array.sort is stable there. + 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 +515,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 (see reactionsArray) + managedChatMessage.reactionsJSONString = NCChatMessage.reactionsJSONString(for: chatMessage.storedReactions(), + keepingOrderOf: managedChatMessage.storedReactions()) managedChatMessage.expirationTimestamp = chatMessage.expirationTimestamp managedChatMessage.isMarkdownMessage = chatMessage.isMarkdownMessage managedChatMessage.lastEditActorId = chatMessage.lastEditActorId @@ -575,12 +570,51 @@ import SwiftyAttributes extension NCChatMessage { - @nonobjc private func reactionsDictionary() -> [String: Any] { + /// The stored reactions, in the order they were first used. + /// + /// They are stored as an array of `[emoji, count]` pairs, so that the order survives being written + /// and read back. Messages stored by an older version hold a JSON object instead, whose key order is + /// lost while parsing, so those are ordered by emoji to give them a fixed order as well. + @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(for: $0.first, count: $0.last) } + } + + if let dictionary = json as? [String: Any] { + return dictionary + .compactMap { NCChatMessage.reaction(for: $0.key, count: $0.value) } + .sorted { $0.reaction < $1.reaction } + } + + return [] + } + + /// Serialises reactions so that the ones in `storedReactions` keep their position and the rest are + /// appended, which is what keeps the stored order the order they were first used in. + @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(for 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] { diff --git a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift index 2cbf29652..254cb1eab 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift @@ -8,6 +8,77 @@ import XCTest final class UnitNCChatMessageTest: TestBaseRealm { + // MARK: - Reactions + + /// Applies reactions the way the app does: a first store, then updates on top of it, so the stored + /// order is built up exactly as it is at runtime. + private func reactionOrder(_ reactions: [[String: Int]]) -> [String] { + var storedMessage: NCChatMessage? + + for counts in reactions { + let dict: [String: Any] = ["id": 1, "token": "orderToken", "message": "Hi", "reactions": counts] + guard let parsed = NCChatMessage(dictionary: dict, andAccountId: TestBaseRealm.fakeAccountId) else { continue } + + if let storedMessage { + NCChatMessage.update(storedMessage, with: parsed, isRoomLastMessage: false) + } else { + storedMessage = parsed + } + } + + return storedMessage?.reactionsArray().map { $0.reaction } ?? [] + } + + // Like the web client, where the reactions keep the insertion order of the store and Array.sort is + // stable, so only a reaction whose own count changed ever moves. + func testNewReactionIsAppendedInsteadOfPushingTheOthersAside() throws { + let order = reactionOrder([["👍": 1, "😀": 1], ["👍": 1, "😀": 1, "❤️": 1]]) + + XCTAssertEqual(order, ["👍", "😀", "❤️"], + "A new reaction must be appended, not sorted in front of the existing ones") + } + + func testOnlyTheReactionWhoseCountChangedMoves() throws { + let before = reactionOrder([["👍": 1, "😀": 1, "❤️": 1]]) + let after = reactionOrder([["👍": 1, "😀": 1, "❤️": 1], ["👍": 1, "😀": 3, "❤️": 1]]) + + XCTAssertEqual(after.first, "😀", "The reaction that gained counts moves to the front") + XCTAssertEqual(after.filter { $0 != "😀" }, before.filter { $0 != "😀" }, + "The reactions that did not change keep their order") + } + + func testReactionUsedAgainAfterBeingRemovedIsAppended() throws { + let order = reactionOrder([["👍": 1, "😀": 1, "❤️": 1], ["👍": 1, "❤️": 1], ["👍": 1, "❤️": 1, "😀": 1]]) + + XCTAssertEqual(order.last, "😀", "A reaction that is used again is appended, like any other new one") + } + + // Without a stored order the reactions come out of a Swift Dictionary, which iterates depending on a + // per-process hash seed, so the row was ordered differently on every app launch. + func testReactionOrderIsTheSameAcrossCalls() throws { + let reactions = [["👍": 2, "😀": 2, "❤️": 2, "🎉": 2, "🙏": 2]] + + let order = reactionOrder(reactions) + XCTAssertEqual(order.count, 5) + XCTAssertEqual(reactionOrder(reactions), order) + XCTAssertEqual(reactionOrder(reactions), order) + } + + // The very first time a message's reactions are stored there is no order to keep, and the server's + // order is already gone by then, so they get a fixed one instead of the dictionary's. + func testFirstStoredReactionsGetAFixedOrder() throws { + XCTAssertEqual(reactionOrder([["😀": 1, "👍": 1, "❤️": 1]]), ["❤️", "👍", "😀"]) + } + + // Messages stored before reactions were kept in order hold a JSON object instead of pairs + func testReactionsStoredInTheOldFormatAreStillRead() throws { + let message = NCChatMessage() + message.reactionsJSONString = "{\"👍\":3,\"😀\":1}" + + XCTAssertEqual(message.reactionsArray().map { $0.reaction }, ["👍", "😀"]) + XCTAssertEqual(message.reactionsArray().map { $0.count }, [3, 1]) + } + func testUnreadMessageSeparatorUrlCheck() throws { let message = NCChatMessage() message.messageId = MessageSeparatorTableViewCell.unreadMessagesSeparatorId From 203be37fde011c72be85d25b6f3c276fac0bb6d7 Mon Sep 17 00:00:00 2001 From: Ivan Sein Date: Thu, 27 Aug 2026 16:00:45 +0200 Subject: [PATCH 4/6] feat(chat): Add haptic feedback when tapping a reaction Signed-off-by: Ivan Sein --- NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift index 71ce639c6..da75ae05d 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift +++ b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift @@ -17,6 +17,8 @@ import UIKit /// Spacing between two reactions private static let itemSpacing: CGFloat = 8 + private let feedbackGenerator = UIImpactFeedbackGenerator(style: .light) + /// Tracks the touch start time to differentiate quick taps from long presses private var touchBeganTime: Date? @@ -45,6 +47,7 @@ import UIKit override func touchesBegan(_ touches: Set, with event: UIEvent?) { touchBeganTime = Date() + feedbackGenerator.prepare() super.touchesBegan(touches, with: event) } @@ -162,6 +165,7 @@ import UIKit } if indexPath.row < reactions.count { + self.feedbackGenerator.impactOccurred() self.reactionsDelegate?.didSelectReaction(reaction: reactions[indexPath.row]) } } From ed99fbf390d6643cff92448d500d8a95b2f8169e Mon Sep 17 00:00:00 2001 From: Ivan Sein Date: Fri, 28 Aug 2026 13:16:02 +0200 Subject: [PATCH 5/6] chore(chat): Tidy up the reactions code and comments Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ivan Sein --- .../Chat views/Reactions/ReactionsView.swift | 33 +++++++------------ .../Reactions/ReactionsViewCell.swift | 6 ++-- NextcloudTalk/Chat/NCChatMessage.swift | 24 ++++++-------- .../Chat/UnitBaseChatTableViewCellTest.swift | 15 +++------ .../Unit/Chat/UnitNCChatMessageTest.swift | 13 +++----- 5 files changed, 35 insertions(+), 56 deletions(-) diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift index da75ae05d..73713106e 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift +++ b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift @@ -17,7 +17,11 @@ import UIKit /// 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 var scrollFadeLayer: CAGradientLayer? /// Tracks the touch start time to differentiate quick taps from long presses private var touchBeganTime: Date? @@ -60,9 +64,8 @@ import UIKit self.reactions = reactions self.reloadData() - // Cells keep their ReactionsView across reuse, so without invalidating it here the view keeps - // the width of the reactions it showed before: too narrow silently clips the new ones (they are - // then only reachable by scrolling), too wide leaves a gap. + // 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 @@ -71,11 +74,6 @@ import UIKit // MARK: - Scroll fade - /// Width of the fade shown at an edge that has more reactions behind it - private static let scrollFadeWidth: CGFloat = 16 - - private var scrollFadeLayer: CAGradientLayer? - override func layoutSubviews() { super.layoutSubviews() @@ -83,8 +81,7 @@ import UIKit self.updateScrollFade() } - /// Fades out the reactions at an edge that can still be scrolled towards, so it becomes visible - /// that a message has more reactions than there is room for. + /// 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 } @@ -139,7 +136,7 @@ import UIKit 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) } @@ -171,16 +168,10 @@ import UIKit } override var intrinsicContentSize: CGSize { - // Measured from the reactions themselves instead of from collectionViewContentSize: the flow - // layout only recomputes that while laying out, so right after reloadData() it would still - // report the width of the reactions this view showed before. - guard !self.reactions.isEmpty else { - return .init(width: 0, height: UICollectionView.noIntrinsicMetric) - } - - let sizingCell = ReactionsViewCell() - let width = self.reactions.reduce(0) { $0 + sizingCell.sizeForReaction(reaction: $1).width } - + CGFloat(self.reactions.count - 1) * ReactionsView.itemSpacing + // 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) } diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsViewCell.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsViewCell.swift index 1dac8b0a9..e65d6eff3 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsViewCell.swift +++ b/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() } diff --git a/NextcloudTalk/Chat/NCChatMessage.swift b/NextcloudTalk/Chat/NCChatMessage.swift index 73b867cb6..c5a419c13 100644 --- a/NextcloudTalk/Chat/NCChatMessage.swift +++ b/NextcloudTalk/Chat/NCChatMessage.swift @@ -466,13 +466,11 @@ import SwiftyAttributes } } - // Merge with temporary reactions, appending a reaction we just added like any other new one + // Merge with temporary reactions self.mergeTemporaryReactions(into: &reactionsArray) - // Highest count first, keeping the order for reactions with the same count, so that only a - // reaction whose count changed moves and a new one is appended. sorted(by:) is not guaranteed to - // be stable, so the current position is the tie-break. The web client shows the same order: its - // reactions keep the insertion order of the store and Array.sort is stable there. + // 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 } @@ -515,7 +513,7 @@ import SwiftyAttributes managedChatMessage.systemMessage = chatMessage.systemMessage managedChatMessage.isReplyable = chatMessage.isReplyable managedChatMessage.messageType = chatMessage.messageType - // Reactions we already know keep their position, new ones are appended (see reactionsArray) + // 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 @@ -572,29 +570,27 @@ extension NCChatMessage { /// The stored reactions, in the order they were first used. /// - /// They are stored as an array of `[emoji, count]` pairs, so that the order survives being written - /// and read back. Messages stored by an older version hold a JSON object instead, whose key order is - /// lost while parsing, so those are ordered by emoji to give them a fixed order as well. + /// 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 json = try? JSONSerialization.jsonObject(with: data) else { return [] } if let pairs = json as? [[Any]] { - return pairs.compactMap { NCChatMessage.reaction(for: $0.first, count: $0.last) } + return pairs.compactMap { NCChatMessage.reaction(fromEmoji: $0.first, count: $0.last) } } if let dictionary = json as? [String: Any] { return dictionary - .compactMap { NCChatMessage.reaction(for: $0.key, count: $0.value) } + .compactMap { NCChatMessage.reaction(fromEmoji: $0.key, count: $0.value) } .sorted { $0.reaction < $1.reaction } } return [] } - /// Serialises reactions so that the ones in `storedReactions` keep their position and the rest are - /// appended, which is what keeps the stored order the order they were first used in. + /// 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 } @@ -608,7 +604,7 @@ extension NCChatMessage { return String(data: data, encoding: .utf8) } - @nonobjc private static func reaction(for emoji: Any?, count: Any?) -> NCChatReaction? { + @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 diff --git a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift index 6d99a8544..eb1e52457 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift @@ -11,8 +11,7 @@ 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: the - // collection view is sized by its own intrinsic content size, capped by the available width. + // 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 @@ -35,9 +34,8 @@ final class UnitBaseChatTableViewCellTest: TestBaseRealm { return emojis.map { NCChatReaction(reaction: $0, count: 1, userReacted: false, state: .set) } } - // A cell keeps its ReactionsView across reuse, so showing a message with more reactions than the - // previous one must widen it again. Otherwise the collection view stays at the width of whatever - // it showed before and silently clips the rest, which is only reachable by scrolling. + // 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) @@ -63,9 +61,7 @@ final class UnitBaseChatTableViewCellTest: TestBaseRealm { "Showing fewer reactions must shrink the view again") } - // When a message has more reactions than the bubble has room for, the remaining ones are only - // reachable by scrolling. Fading out the edge that can be scrolled towards is what makes that - // visible instead of looking like the message simply has fewer reactions. + // 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) @@ -88,8 +84,7 @@ final class UnitBaseChatTableViewCellTest: TestBaseRealm { "Reactions that all fit must not be faded out, there is nothing to scroll to") } - // A reused view must not keep the scroll position of the message it showed before, or the first - // reactions of the new message start off screen. + // 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) diff --git a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift index 254cb1eab..c9b841ce0 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift @@ -10,8 +10,7 @@ final class UnitNCChatMessageTest: TestBaseRealm { // MARK: - Reactions - /// Applies reactions the way the app does: a first store, then updates on top of it, so the stored - /// order is built up exactly as it is at runtime. + /// Applies reactions the way the app does: a first store, then updates on top of it private func reactionOrder(_ reactions: [[String: Int]]) -> [String] { var storedMessage: NCChatMessage? @@ -29,8 +28,7 @@ final class UnitNCChatMessageTest: TestBaseRealm { return storedMessage?.reactionsArray().map { $0.reaction } ?? [] } - // Like the web client, where the reactions keep the insertion order of the store and Array.sort is - // stable, so only a reaction whose own count changed ever moves. + // Like the web client, where only a reaction whose own count changed ever moves func testNewReactionIsAppendedInsteadOfPushingTheOthersAside() throws { let order = reactionOrder([["👍": 1, "😀": 1], ["👍": 1, "😀": 1, "❤️": 1]]) @@ -53,8 +51,8 @@ final class UnitNCChatMessageTest: TestBaseRealm { XCTAssertEqual(order.last, "😀", "A reaction that is used again is appended, like any other new one") } - // Without a stored order the reactions come out of a Swift Dictionary, which iterates depending on a - // per-process hash seed, so the row was ordered differently on every app launch. + // A Swift Dictionary iterates depending on a per-process hash seed, so without a stored order the + // row came out differently on every app launch func testReactionOrderIsTheSameAcrossCalls() throws { let reactions = [["👍": 2, "😀": 2, "❤️": 2, "🎉": 2, "🙏": 2]] @@ -64,8 +62,7 @@ final class UnitNCChatMessageTest: TestBaseRealm { XCTAssertEqual(reactionOrder(reactions), order) } - // The very first time a message's reactions are stored there is no order to keep, and the server's - // order is already gone by then, so they get a fixed one instead of the dictionary's. + // Nothing to keep the order of on the first store, so they get a fixed one instead func testFirstStoredReactionsGetAFixedOrder() throws { XCTAssertEqual(reactionOrder([["😀": 1, "👍": 1, "❤️": 1]]), ["❤️", "👍", "😀"]) } From a3d3626a7222f211599f1fc23ee58901d71939f1 Mon Sep 17 00:00:00 2001 From: Ivan Sein Date: Fri, 28 Aug 2026 16:00:42 +0200 Subject: [PATCH 6/6] chore(chat): Use a lazy var for the reactions scroll fade layer Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ivan Sein --- .../Chat views/Reactions/ReactionsView.swift | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift index 73713106e..063cd2380 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift +++ b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift @@ -21,7 +21,14 @@ import UIKit private static let scrollFadeWidth: CGFloat = 16 private let feedbackGenerator = UIImpactFeedbackGenerator(style: .light) - private var scrollFadeLayer: CAGradientLayer? + + 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? @@ -90,20 +97,11 @@ import UIKit guard canScrollToLeading || canScrollToTrailing else { self.layer.mask = nil - self.scrollFadeLayer = nil return } - let fadeLayer: CAGradientLayer - if let scrollFadeLayer { - fadeLayer = scrollFadeLayer - } else { - fadeLayer = CAGradientLayer() - fadeLayer.startPoint = .init(x: 0, y: 0.5) - fadeLayer.endPoint = .init(x: 1, y: 0.5) - self.scrollFadeLayer = fadeLayer - self.layer.mask = fadeLayer - } + let fadeLayer = self.scrollFadeLayer + self.layer.mask = fadeLayer let opaque = UIColor.white.cgColor let clear = UIColor.clear.cgColor