From 1d95cd6977c5f889f52d5edd094d25fe3b666303 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Mar 2018 10:16:18 -0400 Subject: [PATCH] Improve video scrubbing UX in PageView // FREEBIE --- .../MediaDetailViewController.h | 2 ++ .../MediaDetailViewController.m | 22 +++++++++++++--- .../MediaPageViewController.swift | 22 ++++++---------- .../attachments/VideoPlayerView.swift | 25 ++++++++++++++++++- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/Signal/src/ViewControllers/MediaDetailViewController.h b/Signal/src/ViewControllers/MediaDetailViewController.h index 1337e602a6..5626cb92d4 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.h +++ b/Signal/src/ViewControllers/MediaDetailViewController.h @@ -23,6 +23,8 @@ typedef NS_OPTIONS(NSInteger, MediaGalleryOption) { - (void)mediaDetailViewController:(MediaDetailViewController *)mediaDetailViewController isPlayingVideo:(BOOL)isPlayingVideo; +- (void)mediaDetailViewControllerDidTapMedia:(MediaDetailViewController *)mediaDetailViewController; + @end @interface MediaDetailViewController : OWSViewController diff --git a/Signal/src/ViewControllers/MediaDetailViewController.m b/Signal/src/ViewControllers/MediaDetailViewController.m index 98c251a1b2..d5dca42f34 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.m +++ b/Signal/src/ViewControllers/MediaDetailViewController.m @@ -143,7 +143,6 @@ NS_ASSUME_NONNULL_BEGIN [super viewDidLoad]; [self createContents]; - [self initializeGestureRecognizers]; // Even though bars are opaque, we want content to be layed out behind them. // The bars might obscure part of the content, but they can easily be hidden by tapping @@ -244,6 +243,11 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(self.mediaView); + // We add these gestures to mediaView rather than + // the root view so that interacting with the video player + // progres bar doesn't trigger any of these gestures. + [self addGestureRecognizersToView:self.mediaView]; + [scrollView addSubview:self.mediaView]; self.mediaViewLeadingConstraint = [self.mediaView autoPinEdgeToSuperviewEdge:ALEdgeLeading]; self.mediaViewTopConstraint = [self.mediaView autoPinEdgeToSuperviewEdge:ALEdgeTop]; @@ -323,21 +327,31 @@ NS_ASSUME_NONNULL_BEGIN self.videoProgressBar.hidden = shouldHideToolbars; } -- (void)initializeGestureRecognizers +- (void)addGestureRecognizersToView:(UIView *)view { UITapGestureRecognizer *doubleTap = [[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(didDoubleTapImage:)]; doubleTap.numberOfTapsRequired = 2; - [self.view addGestureRecognizer:doubleTap]; + [view addGestureRecognizer:doubleTap]; + + UITapGestureRecognizer *singleTap = + [[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(didSingleTapImage:)]; + [singleTap requireGestureRecognizerToFail:doubleTap]; + [view addGestureRecognizer:singleTap]; UILongPressGestureRecognizer *longPress = [[UILongPressGestureRecognizer alloc] initWithTarget:self action:@selector(longPressGesture:)]; longPress.delegate = self; - [self.view addGestureRecognizer:longPress]; + [view addGestureRecognizer:longPress]; } #pragma mark - Gesture Recognizers +- (void)didSingleTapImage:(UITapGestureRecognizer *)gesture +{ + [self.delegate mediaDetailViewControllerDidTapMedia:self]; +} + - (void)didDoubleTapImage:(UITapGestureRecognizer *)gesture { DDLogVerbose(@"%@ did double tap image.", self.logTag); diff --git a/Signal/src/ViewControllers/MediaPageViewController.swift b/Signal/src/ViewControllers/MediaPageViewController.swift index 20add8558c..37b19b8690 100644 --- a/Signal/src/ViewControllers/MediaPageViewController.swift +++ b/Signal/src/ViewControllers/MediaPageViewController.swift @@ -175,14 +175,6 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou // Gestures - let doubleTap = UITapGestureRecognizer(target: nil, action: nil) - doubleTap.numberOfTapsRequired = 2 - view.addGestureRecognizer(doubleTap) - - let singleTap = UITapGestureRecognizer(target: self, action: #selector(didTapView)) - singleTap.require(toFail: doubleTap) - view.addGestureRecognizer(singleTap) - let verticalSwipe = UISwipeGestureRecognizer(target: self, action: #selector(didSwipeView)) verticalSwipe.direction = [.up, .down] view.addGestureRecognizer(verticalSwipe) @@ -229,13 +221,6 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou self.dismissSelf(animated: true) } - @objc - public func didTapView(sender: Any) { - Logger.debug("\(logTag) in \(#function)") - - self.shouldHideToolbars = !self.shouldHideToolbars - } - private var shouldHideToolbars: Bool = false { didSet { if (oldValue == shouldHideToolbars) { @@ -515,6 +500,13 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou // MARK: MediaDetailViewControllerDelegate + @objc + public func mediaDetailViewControllerDidTapMedia(_ mediaDetailViewController: MediaDetailViewController) { + Logger.debug("\(logTag) in \(#function)") + + self.shouldHideToolbars = !self.shouldHideToolbars + } + public func mediaDetailViewController(_ mediaDetailViewController: MediaDetailViewController, requestDelete conversationViewItem: ConversationViewItem) { guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") diff --git a/SignalMessaging/attachments/VideoPlayerView.swift b/SignalMessaging/attachments/VideoPlayerView.swift index bef91c818d..ea9f0468c2 100644 --- a/SignalMessaging/attachments/VideoPlayerView.swift +++ b/SignalMessaging/attachments/VideoPlayerView.swift @@ -33,6 +33,23 @@ public protocol PlayerProgressBarDelegate { func playerProgressBar(_ playerProgressBar: PlayerProgressBar, didFinishScrubbingAtTime time: CMTime, shouldResumePlayback: Bool) } +// Allows the user to tap anywhere on the slider to set it's position, +// without first having to grab the thumb. +class TrackingSlider: UISlider { + + override init(frame: CGRect) { + super.init(frame: frame) + } + + override func beginTracking(_ touch: UITouch, with event: UIEvent?) -> Bool { + return true + } + + required init?(coder aDecoder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } +} + @objc public class PlayerProgressBar: UIView { public let TAG = "[PlayerProgressBar]" @@ -52,7 +69,7 @@ public class PlayerProgressBar: UIView { // MARK: Subviews private let positionLabel = UILabel() private let remainingLabel = UILabel() - private let slider = UISlider() + private let slider = TrackingSlider() private let blurEffectView = UIVisualEffectView(effect: UIBlurEffect(style: .light)) weak private var progressObserver: AnyObject? @@ -109,6 +126,12 @@ public class PlayerProgressBar: UIView { slider.addTarget(self, action: #selector(handleSliderTouchUp), for: .touchUpOutside) slider.addTarget(self, action: #selector(handleSliderValueChanged), for: .valueChanged) + // Panning is a no-op. We just absorb pan gesture's originating in the video controls + // from propogating so we don't inadvertently change pages while trying to scrub in + // the MediaPageView. + let panAbsorber = UIPanGestureRecognizer(target: self, action: nil) + self.addGestureRecognizer(panAbsorber) + // Layout Subviews addSubview(positionLabel)