From d2541926cc5ddfc76fe360b3c7cdd7d4781e1381 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 21 Sep 2023 17:49:38 +0200 Subject: [PATCH] Consider tests for animations (#1779) --- .../SwiftUI/Animation/ElementAnimations.swift | 22 ++++++++++++++----- .../SwiftUI/Animation/ShimmerModifier.swift | 5 ++++- ElementX/Sources/Other/Tests.swift | 5 +++++ .../View/ComposerToolbar.swift | 2 +- .../HomeScreen/View/HomeScreenRoomCell.swift | 2 +- .../View/InviteUsersScreen.swift | 2 +- .../View/Style/LongPressWithFeedback.swift | 2 +- .../Supplementary/TimelineReactionsView.swift | 2 +- 8 files changed, 30 insertions(+), 12 deletions(-) diff --git a/ElementX/Sources/Other/SwiftUI/Animation/ElementAnimations.swift b/ElementX/Sources/Other/SwiftUI/Animation/ElementAnimations.swift index 3bb350b5f..a47799654 100644 --- a/ElementX/Sources/Other/SwiftUI/Animation/ElementAnimations.swift +++ b/ElementX/Sources/Other/SwiftUI/Animation/ElementAnimations.swift @@ -21,9 +21,22 @@ public extension Animation { /// Animation to be used to disable animations. static let noAnimation: Animation = .linear(duration: 0) - /// `noAnimation` if running UI tests, otherwise `default` animation. + /// `noAnimation` if running tests, otherwise `default` animation if `UIAccessibility.isReduceMotionEnabled` is false static var elementDefault: Animation { - Tests.isRunningUITests ? .noAnimation : (UIAccessibility.isReduceMotionEnabled ? .noAnimation : .default) + let animation: Animation = Tests.isRunningTests ? .noAnimation : .default + return animation.disabledIfReduceMotionEnabled() + } + + // `noAnimation` if running tests, otherwise `self` if `UIAccessibility.isReduceMotionEnabled` is false + func disabledDuringTests() -> Self { + let animation: Animation = Tests.isRunningTests ? .noAnimation : self + return animation.disabledIfReduceMotionEnabled() + } + + // MARK: - Private + + private func disabledIfReduceMotionEnabled() -> Self { + UIAccessibility.isReduceMotionEnabled ? .noAnimation : self } } @@ -33,8 +46,5 @@ public extension Animation { /// - animation: Animation /// - body: operations to be animated public func withElementAnimation(_ animation: Animation? = .default, _ body: () throws -> Result) rethrows -> Result { - if Tests.isRunningUITests { - return try withAnimation(.noAnimation, body) - } - return try withAnimation(animation, body) + try withAnimation(animation?.disabledDuringTests(), body) } diff --git a/ElementX/Sources/Other/SwiftUI/Animation/ShimmerModifier.swift b/ElementX/Sources/Other/SwiftUI/Animation/ShimmerModifier.swift index c0465945a..7c7e195da 100644 --- a/ElementX/Sources/Other/SwiftUI/Animation/ShimmerModifier.swift +++ b/ElementX/Sources/Other/SwiftUI/Animation/ShimmerModifier.swift @@ -41,7 +41,10 @@ struct ShimmerModifier: ViewModifier { private let regularColor = Color.white /// A slow linear animation which auto-repeats after a delay. - private let animation: Animation = Tests.isRunningUITests ? .noAnimation : .linear(duration: 1.75).delay(0.5).repeatForever(autoreverses: false) + private let animation: Animation = .linear(duration: 1.75) + .delay(0.5) + .repeatForever(autoreverses: false) + .disabledDuringTests() func body(content: Content) -> some View { content diff --git a/ElementX/Sources/Other/Tests.swift b/ElementX/Sources/Other/Tests.swift index 74d90cc01..fddda70b5 100644 --- a/ElementX/Sources/Other/Tests.swift +++ b/ElementX/Sources/Other/Tests.swift @@ -34,6 +34,11 @@ public enum Tests { false #endif } + + /// Flag indicating whether the app is running the UI tests or unit tests. + static var isRunningTests: Bool { + isRunningUITests || isRunningUnitTests + } /// The identifier of the screen to be loaded when running UI tests. static var screenID: UITestsScreenIdentifier? { diff --git a/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift b/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift index 0e8b4353b..f48ecf9c2 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift @@ -102,7 +102,7 @@ struct ComposerToolbar: View { .padding(4) } .disabled(context.viewState.sendButtonDisabled) - .animation(.linear(duration: 0.1), value: context.viewState.sendButtonDisabled) + .animation(.linear(duration: 0.1).disabledDuringTests(), value: context.viewState.sendButtonDisabled) .keyboardShortcut(.return, modifiers: [.command]) } diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomCell.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomCell.swift index a67c97510..b9092a63d 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomCell.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomCell.swift @@ -167,7 +167,7 @@ struct HomeScreenRoomCellButtonStyle: ButtonStyle { configuration.label .background(isSelected ? Color.compound.bgSubtleSecondary : Color.compound.bgCanvasDefault) .contentShape(Rectangle()) - .animation(isSelected ? .none : .easeOut(duration: 0.1), value: isSelected) + .animation(isSelected ? .none : .easeOut(duration: 0.1).disabledDuringTests(), value: isSelected) } } diff --git a/ElementX/Sources/Screens/InviteUsersScreen/View/InviteUsersScreen.swift b/ElementX/Sources/Screens/InviteUsersScreen/View/InviteUsersScreen.swift index c20991091..a66740135 100644 --- a/ElementX/Sources/Screens/InviteUsersScreen/View/InviteUsersScreen.swift +++ b/ElementX/Sources/Screens/InviteUsersScreen/View/InviteUsersScreen.swift @@ -106,7 +106,7 @@ struct InviteUsersScreen: View { } .onChange(of: context.viewState.scrollToLastID) { lastAddedID in guard let id = lastAddedID else { return } - withAnimation(.easeInOut) { + withElementAnimation(.easeInOut) { scrollView.scrollTo(id) } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/LongPressWithFeedback.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/LongPressWithFeedback.swift index 69ea10b2f..8a676a1ec 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/LongPressWithFeedback.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/LongPressWithFeedback.swift @@ -31,7 +31,7 @@ struct LongPressWithFeedback: ViewModifier { .shadow(color: .black.opacity(isLongPressing ? 0.1 : 0.0), radius: isLongPressing ? 3 : 0) .scaleEffect(x: isLongPressing ? 1.05 : 1, y: isLongPressing ? 1.05 : 1) - .animation(isLongPressing ? .spring(response: 0.5).delay(0.1) : .spring(response: 0.5), + .animation(.spring(response: 0.5).delay(isLongPressing ? 0.1 : 0).disabledDuringTests(), value: isLongPressing) // The minimum duration here doesn't actually invoke the perform block when elapsed (thus // the implementation below) but it does cancel other system gestures e.g. swipe to reply diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift index 051f3cccf..8b489bf9b 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift @@ -61,7 +61,7 @@ struct TimelineReactionsView: View { .reactionLayoutItem(.addMore) } .environment(\.layoutDirection, reactionsLayoutDirection) - .animation(.easeInOut(duration: 0.1), value: reactions) + .animation(.easeInOut(duration: 0.1).disabledDuringTests(), value: reactions) .padding(.leading, 4) } }