From 7ee38f808d358598a0555eff3d558424b0960996 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Feb 2019 16:42:08 -0500 Subject: [PATCH 1/5] Show "add attachment caption" button for non-media attachments; only show if more than one attachment. --- .../AttachmentApprovalViewController.swift | 32 +++++++++++++++++++ .../Views/ImageEditor/ImageEditorView.swift | 14 ++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index 776aa4295..4221b49f3 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -48,6 +48,10 @@ class AttachmentItemCollection { func remove(item: SignalAttachmentItem) { attachmentItems = attachmentItems.filter { $0 != item } } + + func count() -> Int { + return attachmentItems.count + } } // MARK: - @@ -618,6 +622,10 @@ extension AttachmentApprovalViewController: AttachmentPrepViewControllerDelegate func prepViewControllerUpdateNavigationBar() { self.updateNavigationBar() } + + func prepViewControllerAttachmentCount() -> Int { + return attachmentItemCollection.count() + } } // MARK: GalleryRail @@ -671,6 +679,8 @@ protocol AttachmentPrepViewControllerDelegate: class { func prepViewController(_ prepViewController: AttachmentPrepViewController, didUpdateCaptionForAttachmentItem attachmentItem: SignalAttachmentItem) func prepViewControllerUpdateNavigationBar() + + func prepViewControllerAttachmentCount() -> Int } public class AttachmentPrepViewController: OWSViewController, PlayerProgressBarDelegate, OWSVideoPlayerDelegate { @@ -884,11 +894,25 @@ public class AttachmentPrepViewController: OWSViewController, PlayerProgressBarD public func navigationBarItems() -> [UIView] { guard let imageEditorView = imageEditorView else { + // Show the "add caption" button for non-image attachments if + // there is more than one attachment. + if let prepDelegate = prepDelegate, + prepDelegate.prepViewControllerAttachmentCount() > 1 { + let captionButton = navigationBarButton(imageName: "image_editor_caption", + selector: #selector(didTapCaption(sender:))) + return [captionButton] + } return [] } return imageEditorView.navigationBarItems() } + @objc func didTapCaption(sender: UIButton) { + Logger.verbose("") + + imageEditorPresentCaptionView() + } + // MARK: - Event Handlers @objc @@ -1165,6 +1189,14 @@ extension AttachmentPrepViewController: ImageEditorViewDelegate { public func imageEditorUpdateNavigationBar() { prepDelegate?.prepViewControllerUpdateNavigationBar() } + + public func imageEditorAttachmentCount() -> Int { + guard let prepDelegate = prepDelegate else { + owsFailDebug("Missing prepDelegate.") + return 0 + } + return prepDelegate.prepViewControllerAttachmentCount() + } } // MARK: - diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index 0e9286845..3638d5d96 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -10,6 +10,7 @@ public protocol ImageEditorViewDelegate: class { withNavigation: Bool) func imageEditorPresentCaptionView() func imageEditorUpdateNavigationBar() + func imageEditorAttachmentCount() -> Int } // MARK: - @@ -102,11 +103,20 @@ public class ImageEditorView: UIView { let captionButton = navigationBarButton(imageName: "image_editor_caption", selector: #selector(didTapCaption(sender:))) + var buttons: [UIView] if model.canUndo() { - return [undoButton, newTextButton, brushButton, cropButton, captionButton] + buttons = [undoButton, newTextButton, brushButton, cropButton] } else { - return [newTextButton, brushButton, cropButton, captionButton] + buttons = [newTextButton, brushButton, cropButton] } + + // Show the "add caption" button for non-image attachments if + // there is more than one attachment. + if let delegate = delegate, + delegate.imageEditorAttachmentCount() > 1 { + buttons.append(captionButton) + } + return buttons } // MARK: - Actions From d15f5b581f2b946faeb508330ffc50446aab186d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Feb 2019 17:09:32 -0500 Subject: [PATCH 2/5] Tweak how image editor overlays are presented. --- .../AttachmentApprovalViewController.swift | 32 ++++++++----------- .../Views/ImageEditor/ImageEditorView.swift | 16 +++++----- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index 4221b49f3..7909d9421 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -1156,32 +1156,28 @@ extension AttachmentPrepViewController: UIScrollViewDelegate { // MARK: - extension AttachmentPrepViewController: ImageEditorViewDelegate { - public func imageEditor(presentFullScreenOverlay viewController: UIViewController, - withNavigation: Bool) { + public func imageEditor(presentFullScreenView viewController: UIViewController, + isTransparent: Bool) { - if withNavigation { - let navigationController = OWSNavigationController(rootViewController: viewController) - navigationController.modalPresentationStyle = .overFullScreen + let navigationController = OWSNavigationController(rootViewController: viewController) + navigationController.modalPresentationStyle = (isTransparent + ? .overFullScreen + : .fullScreen) - if let navigationBar = navigationController.navigationBar as? OWSNavigationBar { - navigationBar.overrideTheme(type: .clear) - } else { - owsFailDebug("navigationBar was nil or unexpected class") - } - - self.present(navigationController, animated: false) { - // Do nothing. - } + if let navigationBar = navigationController.navigationBar as? OWSNavigationBar { + navigationBar.overrideTheme(type: .clear) } else { - self.present(viewController, animated: false) { - // Do nothing. - } + owsFailDebug("navigationBar was nil or unexpected class") + } + + self.present(navigationController, animated: false) { + // Do nothing. } } public func imageEditorPresentCaptionView() { let view = AttachmentCaptionViewController(delegate: self, attachmentItem: attachmentItem) - self.imageEditor(presentFullScreenOverlay: view, withNavigation: true) + self.imageEditor(presentFullScreenView: view, isTransparent: true) isShowingCaptionView = true } diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index 3638d5d96..d72d507a4 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -6,8 +6,8 @@ import UIKit @objc public protocol ImageEditorViewDelegate: class { - func imageEditor(presentFullScreenOverlay viewController: UIViewController, - withNavigation: Bool) + func imageEditor(presentFullScreenView viewController: UIViewController, + isTransparent: Bool) func imageEditorPresentCaptionView() func imageEditorUpdateNavigationBar() func imageEditorAttachmentCount() -> Int @@ -134,8 +134,8 @@ public class ImageEditorView: UIView { Logger.verbose("") let brushView = ImageEditorBrushViewController(delegate: self, model: model, currentColor: currentColor) - self.delegate?.imageEditor(presentFullScreenOverlay: brushView, - withNavigation: true) + self.delegate?.imageEditor(presentFullScreenView: brushView, + isTransparent: false) } @objc func didTapCrop(sender: UIButton) { @@ -434,8 +434,8 @@ public class ImageEditorView: UIView { model: model, textItem: textItem, maxTextWidthPoints: maxTextWidthPoints) - self.delegate?.imageEditor(presentFullScreenOverlay: textEditor, - withNavigation: true) + self.delegate?.imageEditor(presentFullScreenView: textEditor, + isTransparent: false) } // MARK: - Crop Tool @@ -458,8 +458,8 @@ public class ImageEditorView: UIView { } let cropTool = ImageEditorCropViewController(delegate: self, model: model, srcImage: srcImage, previewImage: previewImage) - self.delegate?.imageEditor(presentFullScreenOverlay: cropTool, - withNavigation: true) + self.delegate?.imageEditor(presentFullScreenView: cropTool, + isTransparent: false) }} // MARK: - From 65ee1dbd754bfce7388fca66c3f290f72dcc40a5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Feb 2019 17:18:50 -0500 Subject: [PATCH 3/5] Hide the current text item while the text item editor is open. --- .../Views/ImageEditor/ImageEditorCanvasView.swift | 11 ++++++++++- .../ImageEditor/ImageEditorTextViewController.swift | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorCanvasView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorCanvasView.swift index a755ae9b9..79a1ae986 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorCanvasView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorCanvasView.swift @@ -27,9 +27,13 @@ public class ImageEditorCanvasView: UIView { private let model: ImageEditorModel + private let itemIdsToIgnore: [String] + @objc - public required init(model: ImageEditorModel) { + public required init(model: ImageEditorModel, + itemIdsToIgnore: [String] = []) { self.model = model + self.itemIdsToIgnore = itemIdsToIgnore super.init(frame: .zero) @@ -180,6 +184,11 @@ public class ImageEditorCanvasView: UIView { updateImageLayer() for item in model.items() { + guard !itemIdsToIgnore.contains(item.itemId) else { + // Ignore this item. + continue + } + guard let layer = ImageEditorCanvasView.layerForItem(item: item, model: model, transform: transform, diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift index 680f26823..a865aa5e3 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift @@ -122,7 +122,8 @@ public class ImageEditorTextViewController: OWSViewController, VAlignTextViewDel self.model = model self.textItem = textItem self.maxTextWidthPoints = maxTextWidthPoints - self.canvasView = ImageEditorCanvasView(model: model) + self.canvasView = ImageEditorCanvasView(model: model, + itemIdsToIgnore: [textItem.itemId]) self.paletteView = ImageEditorPaletteView(currentColor: textItem.color) super.init(nibName: nil, bundle: nil) From 919e886eb72ae8870fa30bec13df5083d9b29954 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Feb 2019 17:27:26 -0500 Subject: [PATCH 4/5] Ensure brush strokes include the entire gesture. --- .../ImageEditorBrushViewController.swift | 17 +++++++++----- .../ImageEditorPanGestureRecognizer.swift | 22 ++++++++++++++++--- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift index 6c79fde1b..4f9e5ccfd 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift @@ -127,7 +127,7 @@ public class ImageEditorBrushViewController: OWSViewController { private var currentStrokeSamples = [ImageEditorStrokeItem.StrokeSample]() @objc - public func handleBrushGesture(_ gestureRecognizer: UIGestureRecognizer) { + public func handleBrushGesture(_ gestureRecognizer: ImageEditorPanGestureRecognizer) { AssertIsOnMainThread() let removeCurrentStroke = { @@ -137,10 +137,9 @@ public class ImageEditorBrushViewController: OWSViewController { self.currentStroke = nil self.currentStrokeSamples.removeAll() } - let tryToAppendStrokeSample = { + let tryToAppendStrokeSample = { (locationInView: CGPoint) in let view = self.canvasView.gestureReferenceView let viewBounds = view.bounds - let locationInView = gestureRecognizer.location(in: view) let newSample = ImageEditorCanvasView.locationImageUnit(forLocationInView: locationInView, viewBounds: viewBounds, model: self.model, @@ -162,14 +161,22 @@ public class ImageEditorBrushViewController: OWSViewController { case .began: removeCurrentStroke() - tryToAppendStrokeSample() + // Apply the location history of the gesture so that the stroke reflects + // the touch's movement before the gesture recognized. + for location in gestureRecognizer.locations { + tryToAppendStrokeSample(location) + } + + let locationInView = gestureRecognizer.location(in: canvasView.gestureReferenceView) + tryToAppendStrokeSample(locationInView) let stroke = ImageEditorStrokeItem(color: strokeColor, unitSamples: currentStrokeSamples, unitStrokeWidth: unitStrokeWidth) model.append(item: stroke) currentStroke = stroke case .changed, .ended: - tryToAppendStrokeSample() + let locationInView = gestureRecognizer.location(in: canvasView.gestureReferenceView) + tryToAppendStrokeSample(locationInView) guard let lastStroke = self.currentStroke else { owsFailDebug("Missing last stroke.") diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift b/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift index 3f5cc39ee..835df3b93 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift @@ -13,7 +13,12 @@ public class ImageEditorPanGestureRecognizer: UIPanGestureRecognizer { public weak var referenceView: UIView? - public var locationStart: CGPoint? + // Capture the location history of this gesture. + public var locations = [CGPoint]() + + public var locationStart: CGPoint? { + return locations.first + } // MARK: - Touch Handling @@ -25,12 +30,23 @@ public class ImageEditorPanGestureRecognizer: UIPanGestureRecognizer { owsFailDebug("Missing view") return } - locationStart = self.location(in: referenceView) + locations.append(location(in: referenceView)) + } + + @objc + public override func touchesMoved(_ touches: Set, with event: UIEvent) { + super.touchesMoved(touches, with: event) + + guard let referenceView = referenceView else { + owsFailDebug("Missing view") + return + } + locations.append(location(in: referenceView)) } public override func reset() { super.reset() - locationStart = nil + locations.removeAll() } } From 9be84fc9125340fceb4fcb3ed6a63b7d87c96e42 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 1 Mar 2019 09:44:52 -0500 Subject: [PATCH 5/5] Respond to CR. --- .../AttachmentApprovalViewController.swift | 55 ++++++++++++------- .../Views/ImageEditor/ImageEditorView.swift | 16 ------ 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index 7909d9421..6cebb6f58 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -49,7 +49,7 @@ class AttachmentItemCollection { attachmentItems = attachmentItems.filter { $0 != item } } - func count() -> Int { + var count: Int { return attachmentItems.count } } @@ -624,7 +624,7 @@ extension AttachmentApprovalViewController: AttachmentPrepViewControllerDelegate } func prepViewControllerAttachmentCount() -> Int { - return attachmentItemCollection.count() + return attachmentItemCollection.count } } @@ -893,24 +893,52 @@ public class AttachmentPrepViewController: OWSViewController, PlayerProgressBarD // MARK: - Navigation Bar public func navigationBarItems() -> [UIView] { + let captionButton = navigationBarButton(imageName: "image_editor_caption", + selector: #selector(didTapCaption(sender:))) + guard let imageEditorView = imageEditorView else { // Show the "add caption" button for non-image attachments if // there is more than one attachment. if let prepDelegate = prepDelegate, prepDelegate.prepViewControllerAttachmentCount() > 1 { - let captionButton = navigationBarButton(imageName: "image_editor_caption", - selector: #selector(didTapCaption(sender:))) return [captionButton] } return [] } - return imageEditorView.navigationBarItems() + var navigationBarItems = imageEditorView.navigationBarItems() + + // Show the caption UI if there's more than one attachment + // OR if the attachment already has a caption. + var shouldShowCaptionUI = attachmentCount() > 0 + if let captionText = attachmentItem.captionText, captionText.count > 0 { + shouldShowCaptionUI = true + } + if shouldShowCaptionUI { + navigationBarItems.append(captionButton) + } + + return navigationBarItems + } + + private func attachmentCount() -> Int { + guard let prepDelegate = prepDelegate else { + owsFailDebug("Missing prepDelegate.") + return 0 + } + return prepDelegate.prepViewControllerAttachmentCount() } @objc func didTapCaption(sender: UIButton) { Logger.verbose("") - imageEditorPresentCaptionView() + presentCaptionView() + } + + private func presentCaptionView() { + let view = AttachmentCaptionViewController(delegate: self, attachmentItem: attachmentItem) + self.imageEditor(presentFullScreenView: view, isTransparent: true) + + isShowingCaptionView = true } // MARK: - Event Handlers @@ -1175,24 +1203,9 @@ extension AttachmentPrepViewController: ImageEditorViewDelegate { } } - public func imageEditorPresentCaptionView() { - let view = AttachmentCaptionViewController(delegate: self, attachmentItem: attachmentItem) - self.imageEditor(presentFullScreenView: view, isTransparent: true) - - isShowingCaptionView = true - } - public func imageEditorUpdateNavigationBar() { prepDelegate?.prepViewControllerUpdateNavigationBar() } - - public func imageEditorAttachmentCount() -> Int { - guard let prepDelegate = prepDelegate else { - owsFailDebug("Missing prepDelegate.") - return 0 - } - return prepDelegate.prepViewControllerAttachmentCount() - } } // MARK: - diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index d72d507a4..146c0a2de 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -8,9 +8,7 @@ import UIKit public protocol ImageEditorViewDelegate: class { func imageEditor(presentFullScreenView viewController: UIViewController, isTransparent: Bool) - func imageEditorPresentCaptionView() func imageEditorUpdateNavigationBar() - func imageEditorAttachmentCount() -> Int } // MARK: - @@ -100,8 +98,6 @@ public class ImageEditorView: UIView { selector: #selector(didTapCrop(sender:))) let newTextButton = navigationBarButton(imageName: "image_editor_text", selector: #selector(didTapNewText(sender:))) - let captionButton = navigationBarButton(imageName: "image_editor_caption", - selector: #selector(didTapCaption(sender:))) var buttons: [UIView] if model.canUndo() { @@ -110,12 +106,6 @@ public class ImageEditorView: UIView { buttons = [newTextButton, brushButton, cropButton] } - // Show the "add caption" button for non-image attachments if - // there is more than one attachment. - if let delegate = delegate, - delegate.imageEditorAttachmentCount() > 1 { - buttons.append(captionButton) - } return buttons } @@ -162,12 +152,6 @@ public class ImageEditorView: UIView { edit(textItem: textItem) } - @objc func didTapCaption(sender: UIButton) { - Logger.verbose("") - - delegate?.imageEditorPresentCaptionView() - } - @objc func didTapDone(sender: UIButton) { Logger.verbose("") }