From 517a550593ce7ad4ba1592d5fb11e02271e8cd9a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Mar 2019 16:29:57 -0400 Subject: [PATCH 1/2] Further improve "first responder" behavior in the image editor. --- ...AttachmentApprovalInputAccessoryView.swift | 21 +++++++++---- .../AttachmentApprovalViewController.swift | 31 ++++++++++--------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift index b983274e2..3f9907d86 100644 --- a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift +++ b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift @@ -101,11 +101,7 @@ class AttachmentApprovalInputAccessoryView: UIView { // MARK: - public var shouldHideControls = false { - didSet { - updateContents() - } - } + private var shouldHideControls = false private func updateContents() { var hasCurrentCaption = false @@ -135,6 +131,10 @@ class AttachmentApprovalInputAccessoryView: UIView { if !attachmentCaptionToolbar.textView.isFirstResponder { attachmentCaptionToolbar.textView.becomeFirstResponder() } + } else { + if attachmentCaptionToolbar.textView.isFirstResponder { + attachmentCaptionToolbar.textView.resignFirstResponder() + } } // NOTE: We don't automatically make attachmentTextToolbar.textView // first responder; @@ -143,9 +143,18 @@ class AttachmentApprovalInputAccessoryView: UIView { } public func update(isEditingCaptions: Bool, - currentAttachmentItem: SignalAttachmentItem?) { + currentAttachmentItem: SignalAttachmentItem?, + shouldHideControls: Bool) { + // De-bounce + guard self.isEditingCaptions != isEditingCaptions || + self.currentAttachmentItem != currentAttachmentItem || + self.shouldHideControls != shouldHideControls else { + return + } + self.isEditingCaptions = isEditingCaptions self.currentAttachmentItem = currentAttachmentItem + self.shouldHideControls = shouldHideControls updateContents() } diff --git a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift index cba9ceffa..569ca847a 100644 --- a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift @@ -184,7 +184,6 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC private func updateContents() { updateNavigationBar() updateInputAccessory() - updateControlVisibility() touchInterceptorView.isHidden = !isEditingCaptions } @@ -206,7 +205,10 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC currentPageViewController = pageViewControllers.first } let currentAttachmentItem: SignalAttachmentItem? = currentPageViewController?.attachmentItem - bottomToolView.update(isEditingCaptions: isEditingCaptions, currentAttachmentItem: currentAttachmentItem) + + bottomToolView.update(isEditingCaptions: isEditingCaptions, + currentAttachmentItem: currentAttachmentItem, + shouldHideControls: shouldHideControls) } // MARK: - Navigation Bar @@ -327,15 +329,6 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC return pageViewController.shouldHideControls } - private func updateControlVisibility() { - let hasPresentedView = self.presentedViewController != nil - - if !shouldHideControls, !isFirstResponder, !hasPresentedView { - becomeFirstResponder() - } - bottomToolView.shouldHideControls = shouldHideControls - } - // MARK: - View Helpers func remove(attachmentItem: SignalAttachmentItem) { @@ -411,8 +404,6 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC updateMediaRail() } } - - updateContents() } // MARK: - UIPageViewControllerDataSource @@ -463,6 +454,18 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC return super.viewControllers!.map { $0 as! AttachmentPrepViewController } } + @objc + public override func setViewControllers(_ viewControllers: [UIViewController]?, direction: UIPageViewController.NavigationDirection, animated: Bool, completion: ((Bool) -> Void)? = nil) { + super.setViewControllers(viewControllers, + direction: direction, + animated: animated) { [weak self] (finished) in + if let completion = completion { + completion(finished) + } + self?.updateContents() + } + } + var currentItem: SignalAttachmentItem! { get { return currentPageViewController.attachmentItem @@ -682,7 +685,7 @@ extension AttachmentApprovalViewController: AttachmentPrepViewControllerDelegate } func prepViewControllerUpdateControls() { - updateControlVisibility() + updateInputAccessory() } } From a3efb58d15f2dd6cadf1a901a1dc1b0068a6dd37 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Mar 2019 18:29:19 -0400 Subject: [PATCH 2/2] Respond to CR. --- .../AttachmentApprovalInputAccessoryView.swift | 18 +++++++++++++++--- .../AttachmentApprovalViewController.swift | 6 ++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift index 3f9907d86..eadc0afda 100644 --- a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift +++ b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalInputAccessoryView.swift @@ -120,6 +120,12 @@ class AttachmentApprovalInputAccessoryView: UIView { currentCaptionWrapper.isHidden = isEditingCaptions || !hasCurrentCaption attachmentTextToolbar.isHidden = isEditingCaptions + updateFirstResponder() + + layoutSubviews() + } + + private func updateFirstResponder() { if (shouldHideControls) { if attachmentCaptionToolbar.textView.isFirstResponder { attachmentCaptionToolbar.textView.resignFirstResponder() @@ -138,8 +144,6 @@ class AttachmentApprovalInputAccessoryView: UIView { } // NOTE: We don't automatically make attachmentTextToolbar.textView // first responder; - - layoutSubviews() } public func update(isEditingCaptions: Bool, @@ -149,7 +153,9 @@ class AttachmentApprovalInputAccessoryView: UIView { guard self.isEditingCaptions != isEditingCaptions || self.currentAttachmentItem != currentAttachmentItem || self.shouldHideControls != shouldHideControls else { - return + + updateFirstResponder() + return } self.isEditingCaptions = isEditingCaptions @@ -168,6 +174,12 @@ class AttachmentApprovalInputAccessoryView: UIView { return CGSize.zero } } + + public var hasFirstResponder: Bool { + return (isFirstResponder || + attachmentCaptionToolbar.textView.isFirstResponder || + attachmentTextToolbar.textView.isFirstResponder) + } } // MARK: - diff --git a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift index 569ca847a..dc837b294 100644 --- a/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApproval/AttachmentApprovalViewController.swift @@ -206,6 +206,12 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC } let currentAttachmentItem: SignalAttachmentItem? = currentPageViewController?.attachmentItem + let hasPresentedView = self.presentedViewController != nil + let isToolbarFirstResponder = bottomToolView.hasFirstResponder + if !shouldHideControls, !isFirstResponder, !hasPresentedView, !isToolbarFirstResponder { + becomeFirstResponder() + } + bottomToolView.update(isEditingCaptions: isEditingCaptions, currentAttachmentItem: currentAttachmentItem, shouldHideControls: shouldHideControls)