From 13decc668a4018dadbf71d599d8390a85353f33d Mon Sep 17 00:00:00 2001 From: David Langley Date: Sun, 25 Jun 2023 13:24:48 +0100 Subject: [PATCH] Reaction highlights (#1145) * Highlight own reactions - Highlight own reactions on timeline and improve style. - Highlight reactions on message bottom sheet only when it our own. * Improve overlay background colours * pin to new sdk version * Remove the red used for debug. * Fix padding, overlap and only display count if > 1. - Fix reaction padding and overlap of reaction and bubble. - Only display count if greater than 1. - Return EmptyView for TimelineDeliveryStatus.cancelled. * Fix colors as per design recommendations. * Add Snapshot tests for reactions and documentation. - Add UITests for the timeline including reactions. - Add the reference snapshots. - Add documentation on how to run snapshot tests and update the reference snapshots. * Fix Package.resolved --- CONTRIBUTING.md | 8 +++++ .../Style/TimelineItemBubbledStylerView.swift | 5 ++-- .../Supplementary/TimelineReactionsView.swift | 29 ++++++++++++++----- .../RoomScreen/View/TimelineItemMenu.swift | 3 +- .../RoomTimelineItemFactory.swift | 2 +- .../UITests/UITestsAppCoordinator.swift | 11 +++++++ .../UITests/UITestsScreenIdentifier.swift | 1 + UITests/Sources/RoomScreenUITests.swift | 6 ++++ ...eration.roomSmallTimelineWithReactions.png | 3 ++ ...hone-14.roomSmallTimelineWithReactions.png | 3 ++ ...eration.roomSmallTimelineWithReactions.png | 3 ++ ...hone-14.roomSmallTimelineWithReactions.png | 3 ++ 12 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 UITests/Sources/__Snapshots__/Application/en-GB-iPad-9th-generation.roomSmallTimelineWithReactions.png create mode 100644 UITests/Sources/__Snapshots__/Application/en-GB-iPhone-14.roomSmallTimelineWithReactions.png create mode 100644 UITests/Sources/__Snapshots__/Application/pseudo-iPad-9th-generation.roomSmallTimelineWithReactions.png create mode 100644 UITests/Sources/__Snapshots__/Application/pseudo-iPhone-14.roomSmallTimelineWithReactions.png diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 612dbea78..d81e9f867 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,6 +57,14 @@ Git LFS is used to store UI test snapshots. `swift run tools setup-project` will git lfs install ``` +### Snapshot Tests + +If you make changes to the UI you may cause existing UI Snapshot tests to fail. You can run the snapshot tests using `UITests` target. To update the reference snapshots, delete them from `element-x-ios/UITests/Sources/__Snapshots__/Application` and run the tests again. +These are the devices we store snapshots for that you will need to run against: +- iPhone 14 +- iPad (9th generation) + + ### Githooks The project uses its own shared githooks stored in the .githooks folder, you will need to configure git to use such folder, this is already done if you have run the setup tool with `swift run tools setup-project` otherwise you would need to run: diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift index f38af73e6..82e8e45c3 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift @@ -86,9 +86,8 @@ struct TimelineItemBubbledStylerView: View { } private var messageBubbleWithReactions: some View { - // Figma has a spacing of -4 but it doesn't take into account - // the centre aligned stroke width so we use -5 here - VStack(alignment: alignment, spacing: -5) { + // Figma overlaps reactions by 3 + VStack(alignment: alignment, spacing: -3) { messageBubble .accessibilityElement(children: .combine) diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift index 77690b348..98a834e9b 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift @@ -44,23 +44,36 @@ struct TimelineReactionButton: View { HStack(spacing: 4) { Text(reaction.key) .font(.compound.bodyMD) - Text(String(reaction.count)) - .font(.compound.bodyMD) - .foregroundColor(.compound.textSecondary) + if reaction.count > 1 { + Text(String(reaction.count)) + .font(.compound.bodyMD) + .foregroundColor(textColor) + } } .padding(.vertical, 6) .padding(.horizontal, 8) - .background( - backgroundShape - .strokeBorder(reaction.isHighlighted ? Color.compound.textSecondary : .compound.bgCanvasDefault, lineWidth: 2) - .background(reaction.isHighlighted ? Color.compound.textPrimary.opacity(0.1) : .compound._bgReactionButton, in: backgroundShape) - ) + .background(backgroundShape.fill(overlayBackgroundColor)) + .overlay(backgroundShape.strokeBorder(overlayBorderColor)) + .padding(2) + .overlay(backgroundShape.strokeBorder(Color.compound.bgCanvasDefault, lineWidth: 2)) .accessibilityElement(children: .combine) } var backgroundShape: some InsettableShape { RoundedRectangle(cornerRadius: 12, style: .continuous) } + + var textColor: Color { + reaction.isHighlighted ? Color.compound.textPrimary : .compound.textSecondary + } + + var overlayBackgroundColor: Color { + reaction.isHighlighted ? Color.compound.bgSubtlePrimary : .compound.bgSubtleSecondary + } + + var overlayBorderColor: Color { + reaction.isHighlighted ? Color.compound.borderInteractivePrimary : .clear + } } struct TimelineReactionView_Previews: PreviewProvider { diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineItemMenu.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineItemMenu.swift index 3fdbaf4eb..82ac03084 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineItemMenu.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineItemMenu.swift @@ -163,7 +163,8 @@ public struct TimelineItemMenu: View { } private func reactionBackgroundColor(for emoji: String) -> Color { - if item.properties.reactions.first(where: { $0.key == emoji }) != nil { + if let reaction = item.properties.reactions.first(where: { $0.key == emoji }), + reaction.isHighlighted { return .compound.bgActionPrimaryRest } else { return .clear diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift index e9be1b576..4bb97eee9 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift @@ -322,7 +322,7 @@ struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol { private func aggregateReactions(_ reactions: [Reaction]) -> [AggregatedReaction] { reactions.map { reaction in - let isHighlighted = false // reaction.details.contains(where: { $0.sender.id == userID }) + let isHighlighted = reaction.senders.contains(where: { $0 == userID }) return AggregatedReaction(key: reaction.key, count: Int(reaction.count), isHighlighted: isHighlighted) } } diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index 1b03a01d0..e7f032b7c 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -198,6 +198,17 @@ class MockScreen: Identifiable { let coordinator = RoomScreenCoordinator(parameters: parameters) navigationStackCoordinator.setRootCoordinator(coordinator) return navigationStackCoordinator + case .roomSmallTimelineWithReactions: + let navigationStackCoordinator = NavigationStackCoordinator() + let timelineController = MockRoomTimelineController() + timelineController.timelineItems = RoomTimelineItemFixtures.default + let parameters = RoomScreenCoordinatorParameters(roomProxy: RoomProxyMock(with: .init(displayName: "New room", avatarURL: URL.picturesDirectory)), + timelineController: timelineController, + mediaProvider: MockMediaProvider(), + emojiProvider: EmojiProvider()) + let coordinator = RoomScreenCoordinator(parameters: parameters) + navigationStackCoordinator.setRootCoordinator(coordinator) + return navigationStackCoordinator case .roomSmallTimelineWithReadReceipts: ServiceLocator.shared.settings.readReceiptsEnabled = true let navigationStackCoordinator = NavigationStackCoordinator() diff --git a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift index f707c63ca..0107c71bd 100644 --- a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift +++ b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift @@ -36,6 +36,7 @@ enum UITestsScreenIdentifier: String { case roomPlainNoAvatar case roomEncryptedWithAvatar case roomSmallTimeline + case roomSmallTimelineWithReactions case roomSmallTimelineWithReadReceipts case roomSmallTimelineIncomingAndSmallPagination case roomSmallTimelineLargePagination diff --git a/UITests/Sources/RoomScreenUITests.swift b/UITests/Sources/RoomScreenUITests.swift index 5039989ca..501f6cc1a 100644 --- a/UITests/Sources/RoomScreenUITests.swift +++ b/UITests/Sources/RoomScreenUITests.swift @@ -161,6 +161,12 @@ class RoomScreenUITests: XCTestCase { try await app.assertScreenshot(.roomSmallTimelineWithReadReceipts) } + func testTimelineReactions() async throws { + let app = Application.launch(.roomSmallTimelineWithReactions) + + try await app.assertScreenshot(.roomSmallTimelineWithReactions) + } + // MARK: - Helper Methods private func performOperation(_ operation: UITestsSignal, using client: UITestsSignalling.Client) async throws { diff --git a/UITests/Sources/__Snapshots__/Application/en-GB-iPad-9th-generation.roomSmallTimelineWithReactions.png b/UITests/Sources/__Snapshots__/Application/en-GB-iPad-9th-generation.roomSmallTimelineWithReactions.png new file mode 100644 index 000000000..dcb0a806e --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/en-GB-iPad-9th-generation.roomSmallTimelineWithReactions.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6b77e85ff9c5f5b1d888c97af89a511f788aa9e74eb0d609659c7d81e2b89b94 +size 231654 diff --git a/UITests/Sources/__Snapshots__/Application/en-GB-iPhone-14.roomSmallTimelineWithReactions.png b/UITests/Sources/__Snapshots__/Application/en-GB-iPhone-14.roomSmallTimelineWithReactions.png new file mode 100644 index 000000000..9c06d6854 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/en-GB-iPhone-14.roomSmallTimelineWithReactions.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2c6127923272006fe70a164fca243c1683d32d0227c8066f6794957567724a41 +size 328078 diff --git a/UITests/Sources/__Snapshots__/Application/pseudo-iPad-9th-generation.roomSmallTimelineWithReactions.png b/UITests/Sources/__Snapshots__/Application/pseudo-iPad-9th-generation.roomSmallTimelineWithReactions.png new file mode 100644 index 000000000..31e171ad0 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/pseudo-iPad-9th-generation.roomSmallTimelineWithReactions.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bfc0399661f579abcba48d2d003e710e1f54b235186d26d5fb7f6d3ffcbe235a +size 232688 diff --git a/UITests/Sources/__Snapshots__/Application/pseudo-iPhone-14.roomSmallTimelineWithReactions.png b/UITests/Sources/__Snapshots__/Application/pseudo-iPhone-14.roomSmallTimelineWithReactions.png new file mode 100644 index 000000000..6cd17d0e9 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/pseudo-iPhone-14.roomSmallTimelineWithReactions.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ae1da33e7d2bdddd25617f63d797c2b14c0d646645f19348d2aea2a1729a0950 +size 328669