From e4bc4b5004033a2a4498fb7d5e69d8c34510f7fc Mon Sep 17 00:00:00 2001 From: s0ph0s Date: Mon, 24 Feb 2025 16:51:54 -0500 Subject: [PATCH 1/2] Do not play animated GIFs, stickers when Reduce Motion is on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes https://github.com/signalapp/Signal-iOS/issues/5949 by: - Disabling autoplay for GIFs, animated images, and animated stickers when Reduce Motion is on - Changing the `handleTap` behavior for stickers to play/pause the animation when the sticker is animated and Reduce Motion is on - Adding a new “Show Sticker Pack” item to the context menu when long-pressing a sticker, so that it’s still possible to see the pack when Reduce motion is enabled - Adding play button icons on top of GIFs and animated stickers to indicate that tapping will play the animation This also adds 2 new translation strings (for the “Show Sticker Pack” menu item). Unfortunately, I only know English, so that’s the only language I’ve added the translations to. This is also my first contribution to the Signal codebase, so please let me know if I should’ve done something a different way! I tried to make my changes as minimally disruptive as possible, and I don’t know whether I’ve done that at the expense of following existing style/conventions. --- .../CVItemViewModelImpl.swift | 4 ++ .../CellViews/CVMediaView.swift | 2 +- .../CellViews/ReusableMediaView.swift | 19 +++++ .../Components/CVComponentSticker.swift | 69 +++++++++++++++++-- ...rsationViewController+MessageActions.swift | 1 + ...iewController+MessageActionsDelegate.swift | 6 ++ Signal/ConversationView/MessageActions.swift | 19 +++++ .../MediaItemViewController.swift | 4 +- .../MessageActionsToolbar.swift | 3 + .../translations/en.lproj/Localizable.strings | 6 ++ SignalUI/Appearance/Theme+Icons.swift | 3 + SignalUI/Stickers/StickerView.swift | 1 + SignalUI/Views/LoopingVideoView.swift | 4 +- 13 files changed, 133 insertions(+), 8 deletions(-) diff --git a/Signal/ConversationView/CVItemViewModelImpl.swift b/Signal/ConversationView/CVItemViewModelImpl.swift index 68a0d83bcf7..4c97813205f 100644 --- a/Signal/ConversationView/CVItemViewModelImpl.swift +++ b/Signal/ConversationView/CVItemViewModelImpl.swift @@ -236,6 +236,10 @@ extension CVItemViewModelImpl { AttachmentSharing.showShareUI(for: attachments, sender: sender) } + var isSticker: Bool { + return messageCellType == .stickerMessage + } + var canForwardMessage: Bool { guard !isViewOnce else { return false diff --git a/Signal/ConversationView/CellViews/CVMediaView.swift b/Signal/ConversationView/CellViews/CVMediaView.swift index 47559862691..9d5eaf0f9fa 100644 --- a/Signal/ConversationView/CellViews/CVMediaView.swift +++ b/Signal/ConversationView/CellViews/CVMediaView.swift @@ -155,7 +155,7 @@ public class CVMediaView: ManualLayoutViewWithLayer { mediaView.backgroundColor = isBorderless ? .clear : Theme.washColor if !addProgressIfNecessary() { - if reusableMediaView.isVideo { + if reusableMediaView.needsPlayButton { addVideoPlayButton() } } diff --git a/Signal/ConversationView/CellViews/ReusableMediaView.swift b/Signal/ConversationView/CellViews/ReusableMediaView.swift index b0a89408eea..32f41fa9d41 100644 --- a/Signal/ConversationView/CellViews/ReusableMediaView.swift +++ b/Signal/ConversationView/CellViews/ReusableMediaView.swift @@ -65,6 +65,21 @@ public class ReusableMediaView: NSObject { mediaViewAdapter is MediaViewAdapterVideo } + var needsPlayButton: Bool { + mediaViewAdapter is MediaViewAdapterVideo + || ( + UIAccessibility.isReduceMotionEnabled + && ( + mediaViewAdapter is MediaViewAdapterLoopingVideo + || ( + mediaViewAdapter is MediaViewAdapterSticker + && mediaViewAdapter.shouldBeRenderedByYY + ) + || mediaViewAdapter is MediaViewAdapterAnimated + ) + ) + } + // MARK: - LoadState // Thread-safe access to load state. @@ -332,6 +347,7 @@ class MediaViewAdapterAnimated: MediaViewAdapterSwift { return } imageView.image = image + imageView.autoPlayAnimatedImage = !UIAccessibility.isReduceMotionEnabled } func unloadMedia() { @@ -568,6 +584,9 @@ public class MediaViewAdapterSticker: NSObject, MediaViewAdapterSwift { owsFailDebug("Media has unexpected type: \(type(of: media))") return } + if let yyView = imageView as? CVAnimatedImageView { + yyView.autoPlayAnimatedImage = !UIAccessibility.isReduceMotionEnabled + } imageView.image = image } else { guard let image = media as? UIImage else { diff --git a/Signal/ConversationView/Components/CVComponentSticker.swift b/Signal/ConversationView/Components/CVComponentSticker.swift index abb2c47a44f..cef503d9480 100644 --- a/Signal/ConversationView/Components/CVComponentSticker.swift +++ b/Signal/ConversationView/Components/CVComponentSticker.swift @@ -158,6 +158,18 @@ public class CVComponentSticker: CVComponentBase, CVComponent { // MARK: - Events + private func toggleStickerAnimation(_ view: CVComponentView) { + if let stickerView = view as? CVComponentViewSticker, let rmv = stickerView.reusableMediaView, let yyView = rmv.mediaView as? CVAnimatedImageView { + if yyView.isAnimating { + yyView.stopAnimating() + stickerView.togglePlayButton() + } else { + stickerView.togglePlayButton() + yyView.startAnimating() + } + } + } + public override func handleTap(sender: UIGestureRecognizer, componentDelegate: CVComponentDelegate, componentView: CVComponentView, @@ -168,7 +180,15 @@ public class CVComponentSticker: CVComponentBase, CVComponent { // Not yet downloaded. return false } - componentDelegate.didTapStickerPack(stickerMetadata.packInfo) + var isAnimated = false + if let stickerComponent = componentView as? CVComponentViewSticker { + isAnimated = stickerComponent.isAnimated + } + if UIAccessibility.isReduceMotionEnabled && isAnimated { + toggleStickerAnimation(componentView) + } else { + componentDelegate.didTapStickerPack(stickerMetadata.packInfo) + } return true } @@ -179,6 +199,7 @@ public class CVComponentSticker: CVComponentBase, CVComponent { public class CVComponentViewSticker: NSObject, CVComponentView { fileprivate let stackView = ManualStackView(name: "sticker.container") + fileprivate var playButtonView: UIView? = nil fileprivate var reusableMediaView: ReusableMediaView? @@ -188,11 +209,21 @@ public class CVComponentSticker: CVComponentBase, CVComponent { stackView } + public var isAnimated: Bool { + get { + reusableMediaView?.needsPlayButton != nil && (reusableMediaView?.needsPlayButton)! || false + } + } + public func setIsCellVisible(_ isCellVisible: Bool) { if isCellVisible { - if let reusableMediaView = reusableMediaView, - reusableMediaView.owner == self { - reusableMediaView.load() + if let reusableMediaView = reusableMediaView { + if reusableMediaView.owner == self { + reusableMediaView.load() + } + if reusableMediaView.needsPlayButton { + addPlayButton() + } } } else { if let reusableMediaView = reusableMediaView, @@ -202,6 +233,36 @@ public class CVComponentSticker: CVComponentBase, CVComponent { } } + private func addPlayButton() { + if playButtonView != nil { + return + } + let playButtonWidth: CGFloat = 44 + let playIconWidth: CGFloat = 20 + + let playButton = UIView.transparentContainer() + playButtonView = playButton + stackView.addSubviewToCenterOnSuperview(playButton, size: CGSize(square: playButtonWidth)) + + let playCircleView = OWSLayerView.circleView() + playCircleView.backgroundColor = UIColor.ows_black.withAlphaComponent(0.7) + playCircleView.isUserInteractionEnabled = false + playButton.addSubview(playCircleView) + stackView.layoutSubviewToFillSuperviewEdges(playCircleView) + + let playIconView = CVImageView() + playIconView.setTemplateImageName("play-fill-32", tintColor: UIColor.ows_white) + playIconView.isUserInteractionEnabled = false + stackView.addSubviewToCenterOnSuperview(playIconView, + size: CGSize(square: playIconWidth)) + } + + fileprivate func togglePlayButton() { + if let playButton = playButtonView { + playButton.isHidden = !playButton.isHidden + } + } + public func reset() { stackView.reset() diff --git a/Signal/ConversationView/ConversationViewController+MessageActions.swift b/Signal/ConversationView/ConversationViewController+MessageActions.swift index 15e1b5e380b..6d2ba7b08c6 100644 --- a/Signal/ConversationView/ConversationViewController+MessageActions.swift +++ b/Signal/ConversationView/ConversationViewController+MessageActions.swift @@ -122,6 +122,7 @@ extension ConversationViewController: ContextMenuInteractionDelegate { .showPaymentDetails, .speak, .stopSpeaking, + .showStickerPack, .info, .delete ] diff --git a/Signal/ConversationView/ConversationViewController+MessageActionsDelegate.swift b/Signal/ConversationView/ConversationViewController+MessageActionsDelegate.swift index de4a9952e30..48626b20c9f 100644 --- a/Signal/ConversationView/ConversationViewController+MessageActionsDelegate.swift +++ b/Signal/ConversationView/ConversationViewController+MessageActionsDelegate.swift @@ -278,4 +278,10 @@ extension ConversationViewController: MessageActionsDelegate { let paymentsDetailViewController = PaymentsDetailViewController(paymentItem: paymentHistoryItem) navigationController?.pushViewController(paymentsDetailViewController, animated: true) } + + func messageActionsShowStickerPack(_ itemViewModel: CVItemViewModelImpl) { + if let stickerMetadata = itemViewModel.stickerMetadata { + didTapStickerPack(stickerMetadata.packInfo) + } + } } diff --git a/Signal/ConversationView/MessageActions.swift b/Signal/ConversationView/MessageActions.swift index 1752b41e9ac..2753b832e40 100644 --- a/Signal/ConversationView/MessageActions.swift +++ b/Signal/ConversationView/MessageActions.swift @@ -15,6 +15,7 @@ protocol MessageActionsDelegate: AnyObject { func messageActionsStopSpeakingItem(_ itemViewModel: CVItemViewModelImpl) func messageActionsEditItem(_ itemViewModel: CVItemViewModelImpl) func messageActionsShowPaymentDetails(_ itemViewModel: CVItemViewModelImpl) + func messageActionsShowStickerPack(_ itemViewModel: CVItemViewModelImpl) } // MARK: - @@ -151,6 +152,19 @@ struct MessageActionBuilder { } ) } + + static func showStickerPack(itemViewModel: CVItemViewModelImpl, delegate: MessageActionsDelegate) -> MessageAction { + MessageAction( + .showStickerPack, + accessibilityLabel: OWSLocalizedString("MESSAGE_ACTION_SHOW_STICKER_PACK", comment: "Action sheet accessibility label"), + accessibilityIdentifier: UIView.accessibilityIdentifier(containerName: "message_action", name: "show_sticker_pack"), + contextMenuTitle: OWSLocalizedString("CONTEXT_MENU_SHOW_STICKER_PACK", comment: "Context menu button title"), + contextMenuAttributes: [], + block: { [weak delegate] _ in + delegate?.messageActionsShowStickerPack(itemViewModel) + } + ) + } } class MessageActions: NSObject { @@ -229,6 +243,11 @@ class MessageActions: NSObject { actions.append(editAction) } + if itemViewModel.isSticker { + let showPackAction = MessageActionBuilder.showStickerPack(itemViewModel: itemViewModel, delegate: delegate) + actions.append(showPackAction) + } + let selectAction = MessageActionBuilder.selectMessage(itemViewModel: itemViewModel, delegate: delegate) actions.append(selectAction) diff --git a/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift b/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift index de7e59bb45f..2e09e7cd1a3 100644 --- a/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift +++ b/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift @@ -206,7 +206,7 @@ class MediaItemViewController: OWSViewController, VideoPlaybackStatusProvider { guard mediaView == nil else { return } let view: UIView - if attachmentStream.contentType.isVideo, galleryItem.renderingFlag == .shouldLoop { + if attachmentStream.contentType.isVideo, galleryItem.renderingFlag == .shouldLoop, !UIAccessibility.isReduceMotionEnabled { if attachmentStream.contentType.isVideo, let loopingVideoPlayerView = buildLoopingVideoPlayerView() { loopingVideoPlayerView.delegate = self view = loopingVideoPlayerView @@ -331,7 +331,7 @@ class MediaItemViewController: OWSViewController, VideoPlaybackStatusProvider { private var hasAutoPlayedVideo = false private var isVideo: Bool { - galleryItem.isVideo + galleryItem.isVideo || galleryItem.isAnimated } private func playVideo() { diff --git a/Signal/src/ViewControllers/MessageActionsToolbar.swift b/Signal/src/ViewControllers/MessageActionsToolbar.swift index 9a5e8318be7..abeae7ee0d9 100644 --- a/Signal/src/ViewControllers/MessageActionsToolbar.swift +++ b/Signal/src/ViewControllers/MessageActionsToolbar.swift @@ -26,6 +26,7 @@ public class MessageAction: NSObject { case stopSpeaking case edit case showPaymentDetails + case showStickerPack } let actionType: MessageActionType @@ -70,6 +71,8 @@ public class MessageAction: NSObject { return .contextMenuEdit case .showPaymentDetails: return .settingsPayments + case .showStickerPack: + return .contextMenuSticker } }() return Theme.iconImage(icon) diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 5beb99f8ad7..251ae357fb7 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -1468,6 +1468,9 @@ /* Context menu button title */ "CONTEXT_MENU_SHARE_MEDIA" = "Share"; +/* Context menu button title */ +"CONTEXT_MENU_SHOW_STICKER_PACK" = "Show Sticker Pack"; + /* Context menu button title */ "CONTEXT_MENU_SPEAK_MESSAGE" = "Speak"; @@ -4399,6 +4402,9 @@ /* Action sheet button title */ "MESSAGE_ACTION_SHARE_MEDIA" = "Share Media"; +/* Aciton sheet button title */ +"MESSAGE_ACTION_SHOW_STICKER_PACK" = "Show Sticker Pack"; + /* Action sheet accessibility label */ "MESSAGE_ACTION_SPEAK_MESSAGE" = "Speak Message"; diff --git a/SignalUI/Appearance/Theme+Icons.swift b/SignalUI/Appearance/Theme+Icons.swift index 46ccf1a02b6..0583ac63b09 100644 --- a/SignalUI/Appearance/Theme+Icons.swift +++ b/SignalUI/Appearance/Theme+Icons.swift @@ -126,6 +126,7 @@ public enum ThemeIcon: UInt { case contextMenuVoiceCall case contextMenuVideoCall case contextMenuMessage + case contextMenuSticker case composeNewGroupLarge case composeFindByUsernameLarge @@ -444,6 +445,8 @@ public extension Theme { return "video-light" case .contextMenuMessage: return "chat-light" + case .contextMenuSticker: + return "sticker" // Empty chat list case .composeNewGroupLarge: diff --git a/SignalUI/Stickers/StickerView.swift b/SignalUI/Stickers/StickerView.swift index 77862183b2b..4d3ca8bd8dc 100644 --- a/SignalUI/Stickers/StickerView.swift +++ b/SignalUI/Stickers/StickerView.swift @@ -78,6 +78,7 @@ public class StickerView { let yyView = YYAnimatedImageView() yyView.alwaysInfiniteLoop = true yyView.contentMode = .scaleAspectFit + yyView.autoPlayAnimatedImage = !UIAccessibility.isReduceMotionEnabled yyView.image = stickerImage stickerView = yyView } diff --git a/SignalUI/Views/LoopingVideoView.swift b/SignalUI/Views/LoopingVideoView.swift index 1091ea564b1..757ef599a08 100644 --- a/SignalUI/Views/LoopingVideoView.swift +++ b/SignalUI/Views/LoopingVideoView.swift @@ -146,7 +146,9 @@ public class LoopingVideoView: UIView { } let playerItem = AVPlayerItem(asset: asset, automaticallyLoadedAssetKeys: ["tracks"]) self.player.replaceCurrentItem(with: playerItem) - self.player.play() + if !UIAccessibility.isReduceMotionEnabled { + self.player.play() + } }.done(on: DispatchQueue.main) { [weak self] in guard let self = self else { return From 3bb839a2a7b9ffeaee9c639e67f80364dd6ec327 Mon Sep 17 00:00:00 2001 From: s0ph0s Date: Sun, 9 Mar 2025 04:18:46 -0400 Subject: [PATCH 2/2] Fix crash when opening GIF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I discovered this while re-testing after merging in the recent changes. If Reduce Motion is *off*, opening a GIF would crash because the app couldn’t set up playback controls on the GIF player. This fix is starting to give me a whiff of code smell though, so I might need to refactor some of this… --- .../ViewControllers/MediaGallery/MediaItemViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift b/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift index 2e09e7cd1a3..7e4f57fcea7 100644 --- a/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift +++ b/Signal/src/ViewControllers/MediaGallery/MediaItemViewController.swift @@ -331,7 +331,7 @@ class MediaItemViewController: OWSViewController, VideoPlaybackStatusProvider { private var hasAutoPlayedVideo = false private var isVideo: Bool { - galleryItem.isVideo || galleryItem.isAnimated + galleryItem.isVideo || (UIAccessibility.isReduceMotionEnabled && galleryItem.isAnimated) } private func playVideo() {