From 6fdd5d10013fd85c7f149505e55dd0cf473c07f1 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 14:13:18 -0700 Subject: [PATCH 1/3] dont initializer pagerScrollView as sideEffect --- .../AttachmentApprovalViewController.swift | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index 191dcd61d..14e73e71c 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -174,7 +174,8 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC self.view.backgroundColor = .black - disablePagingIfNecessary() + // avoid an unpleasant "bounce" which doesn't make sense in the context of a single item. + pagerScrollView?.isScrollEnabled = attachmentItems.count > 1 // Bottom Toolbar galleryRailView.delegate = self @@ -395,25 +396,14 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC button.autoPinEdge(toSuperviewMargin: .trailing) } - var pagerScrollView: UIScrollView? - // This is kind of a hack. Since we don't have first class access to the superview's `scrollView` - // we traverse the view hierarchy until we find it, then disable scrolling if there's only one - // item. This avoids an unpleasant "bounce" which doesn't make sense in the context of a single item. - fileprivate func disablePagingIfNecessary() { - for view in self.view.subviews { - if let pagerScrollView = view as? UIScrollView { - self.pagerScrollView = pagerScrollView - break - } - } + lazy var pagerScrollView: UIScrollView? = { + // This is kind of a hack. Since we don't have first class access to the superview's `scrollView` + // we traverse the view hierarchy until we find it. + let pagerScrollView = view.subviews.first { $0 is UIScrollView } as? UIScrollView + assert(pagerScrollView != nil) - guard let pagerScrollView = self.pagerScrollView else { - owsFailDebug("pagerScrollView was unexpectedly nil") - return - } - - pagerScrollView.isScrollEnabled = attachmentItems.count > 1 - } + return pagerScrollView + }() // MARK: - UIPageViewControllerDelegate @@ -694,7 +684,7 @@ extension AttachmentApprovalViewController: AttachmentPrepViewControllerDelegate } func enablePaging() { - self.pagerScrollView?.panGestureRecognizer.isEnabled = true + pagerScrollView?.panGestureRecognizer.isEnabled = true } } From 0ac8f13c0b0a294ccd6906cf489b496d921cd2bd Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 14:36:35 -0700 Subject: [PATCH 2/3] remove redunant method, consolidate naming, adding array getter --- .../AttachmentApprovalViewController.swift | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index 14e73e71c..ad65eacce 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -314,7 +314,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC lastObservedKeyboardTop = keyboardEndFrame.size.height let keyboardScenario: KeyboardScenario = bottomToolView.isEditingMediaMessage ? .editingMessage : .editingCaption - currentPageController.updateCaptionViewBottomInset(keyboardScenario: keyboardScenario) + currentPageViewController.updateCaptionViewBottomInset(keyboardScenario: keyboardScenario) } @objc @@ -337,7 +337,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC Logger.debug("\(keyboardStartFrame) -> \(keyboardEndFrame)") lastObservedKeyboardTop = UIScreen.main.bounds.height - keyboardEndFrame.size.height - currentPageController.updateCaptionViewBottomInset(keyboardScenario: .hidden) + currentPageViewController.updateCaptionViewBottomInset(keyboardScenario: .hidden) } // MARK: - View Helpers @@ -483,13 +483,17 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC return nextPage } - public var currentViewController: AttachmentPrepViewController { - return viewControllers!.first as! AttachmentPrepViewController + public var currentPageViewController: AttachmentPrepViewController { + return pageViewControllers.first! + } + + public var pageViewControllers: [AttachmentPrepViewController] { + return super.viewControllers!.map { $0 as! AttachmentPrepViewController } } var currentItem: SignalAttachmentItem! { get { - return currentViewController.attachmentItem + return currentPageViewController.attachmentItem } set { setCurrentItem(newValue, direction: .forward, animated: false) @@ -586,23 +590,19 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC } extension AttachmentApprovalViewController: MediaMessageTextToolbarDelegate { - var currentPageController: AttachmentPrepViewController { - return viewControllers!.first as! AttachmentPrepViewController - } - func mediaMessageTextToolbarDidBeginEditing(_ mediaMessageTextToolbar: MediaMessageTextToolbar) { - currentPageController.setAttachmentViewScale(.compact, animated: true) + currentPageViewController.setAttachmentViewScale(.compact, animated: true) } func mediaMessageTextToolbarDidEndEditing(_ mediaMessageTextToolbar: MediaMessageTextToolbar) { - currentPageController.setAttachmentViewScale(.fullsize, animated: true) + currentPageViewController.setAttachmentViewScale(.fullsize, animated: true) } func mediaMessageTextToolbarDidTapSend(_ mediaMessageTextToolbar: MediaMessageTextToolbar) { // Toolbar flickers in and out if there are errors // and remains visible momentarily after share extension is dismissed. // It's easiest to just hide it at this point since we're done with it. - currentViewController.shouldAllowAttachmentViewResizing = false + currentPageViewController.shouldAllowAttachmentViewResizing = false mediaMessageTextToolbar.isUserInteractionEnabled = false mediaMessageTextToolbar.isHidden = true From 61758dcf051c7a9ef0244271e62e1cd27e90607a Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 14:55:46 -0700 Subject: [PATCH 3/3] Only show caption for multiple images --- .../AttachmentApprovalViewController.swift | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index ad65eacce..c4da32637 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -254,6 +254,8 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC return } navigationBar.overrideTheme(type: .clear) + + updateCaptionVisibility() } override public func viewDidAppear(_ animated: Bool) { @@ -405,6 +407,12 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC return pagerScrollView }() + func updateCaptionVisibility() { + for pageViewController in pageViewControllers { + pageViewController.updateCaptionVisibility(attachmentCount: attachments.count) + } + } + // MARK: - UIPageViewControllerDelegate public func pageViewController(_ pageViewController: UIPageViewController, willTransitionTo pendingViewControllers: [UIViewController]) { @@ -511,6 +519,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC Logger.debug("cache miss.") let viewController = AttachmentPrepViewController(attachmentItem: item) viewController.prepDelegate = self + viewController.updateCaptionVisibility(attachmentCount: attachments.count) cachedPages[item] = viewController return viewController @@ -780,6 +789,22 @@ public class AttachmentPrepViewController: OWSViewController, PlayerProgressBarD fatalError("init(coder:) has not been implemented") } + func updateCaptionVisibility(attachmentCount: Int) { + if attachmentCount > 1 { + captionView.isHidden = false + } + + // If we previously had multiple attachments, we'd have shown the caption fields. + // + // Subsequently, if the user had added caption text, then removed the other attachments + // we will continue to show this caption field, so as not to hide any already-entered text. + if let captionText = captionView.captionText, captionText.count > 0 { + captionView.isHidden = false + } + + captionView.isHidden = true + } + // MARK: - Subviews lazy var captionView: CaptionView = {