diff --git a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift b/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift index 06e557c08..063cd2380 100644 --- a/NextcloudTalk/Chat/Chat views/Reactions/ReactionsView.swift +++ b/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, 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) } } 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 eee32b580..c5a419c13 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() { @@ -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] { diff --git a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift b/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift index d7463476c..eb1e52457 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitBaseChatTableViewCellTest.swift +++ b/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 = """ { diff --git a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift index 2cbf29652..c9b841ce0 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift @@ -8,6 +8,74 @@ import XCTest final class UnitNCChatMessageTest: TestBaseRealm { + // MARK: - Reactions + + /// 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? + + 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 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") + } + + // 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]] + + let order = reactionOrder(reactions) + XCTAssertEqual(order.count, 5) + XCTAssertEqual(reactionOrder(reactions), order) + XCTAssertEqual(reactionOrder(reactions), order) + } + + // 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]]), ["❤️", "👍", "😀"]) + } + + // 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