From d42c5c1885c87872cceff27a5b6185301abc570f Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Wed, 26 Feb 2020 12:29:13 -0800 Subject: [PATCH] Code review. Also, centralize calculation of final contentInset amounts, to ensure provided insets play nicely with the keyboard. --- .../Sources/Internal/KeyboardObserver.swift | 86 +++++----- .../Sources/ScrollView.swift | 160 ++++++++++++++---- .../Internal/KeyboardObserverTests.swift | 30 ++-- 3 files changed, 185 insertions(+), 91 deletions(-) diff --git a/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift b/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift index 0de5e848f..194b266a0 100644 --- a/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift +++ b/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift @@ -8,9 +8,13 @@ import UIKit -protocol KeyboardObserverDelegate : AnyObject -{ - func keyboardFrameWillChange(observer : KeyboardObserver) +protocol KeyboardObserverDelegate : AnyObject { + + func keyboardFrameWillChange( + for observer : KeyboardObserver, + animationDuration : Double, + options : UIView.AnimationOptions + ) } /** @@ -20,8 +24,8 @@ protocol KeyboardObserverDelegate : AnyObject iOS Docs for keyboard management: https://developer.apple.com/library/archive/documentation/StringsTextFonts/Conceptual/TextAndWebiPhoneOS/KeyboardManagement/KeyboardManagement.html */ -final class KeyboardObserver -{ +final class KeyboardObserver { + private let center : NotificationCenter weak var delegate : KeyboardObserverDelegate? @@ -30,8 +34,8 @@ final class KeyboardObserver // MARK: Initialization // - init(center : NotificationCenter = .default) - { + init(center : NotificationCenter = .default) { + self.center = center self.center.addObserver(self, selector: #selector(keyboardFrameChanged(_:)), name: UIWindow.keyboardWillChangeFrameNotification, object: nil) @@ -44,14 +48,14 @@ final class KeyboardObserver // MARK: Handling Changes // - enum KeyboardFrame : Equatable - { + enum KeyboardFrame : Equatable { + case notVisible case visible(frame: CGRect) } - func currentFrame(in view : UIView) -> KeyboardFrame? - { + func currentFrame(in view : UIView) -> KeyboardFrame? { + guard view.window != nil else { return nil } @@ -69,55 +73,45 @@ final class KeyboardObserver } } - private func animate(with info : NotificationInfo, _ animations : @escaping () -> ()) - { - if info.animationDuration > 0.0 { - /** - Create an animation curve with the correct curve for showing or hiding the keyboard. - - This is unfortunately a private UIView curve. However, we can map it to the animation options' curve - like so: https://stackoverflow.com/questions/26939105/keyboard-animation-curve-as-int - */ - let animationOptions = UIView.AnimationOptions(rawValue: info.animationCurve << 16) - - UIView.animate( - withDuration: info.animationDuration, - delay: 0.0, - options: animationOptions, - animations: animations - ) - } else { - animations() - } - } - // // MARK: Receiving Updates // - private func receivedUpdatedKeyboardInfo(_ new : NotificationInfo) - { + private func receivedUpdatedKeyboardInfo(_ new : NotificationInfo) { + let old = self.latestNotification self.latestNotification = new + /// Only communicate a frame change to the delegate if the frame actually changed. + if let old = old { guard old.endingFrame != new.endingFrame else { return } } - self.animate(with: new) { - self.delegate?.keyboardFrameWillChange(observer: self) - } + /** + Create an animation curve with the correct curve for showing or hiding the keyboard. + + This is unfortunately a private UIView curve. However, we can map it to the animation options' curve + like so: https://stackoverflow.com/questions/26939105/keyboard-animation-curve-as-int + */ + let animationOptions = UIView.AnimationOptions(rawValue: new.animationCurve << 16) + + self.delegate?.keyboardFrameWillChange( + for: self, + animationDuration: new.animationDuration, + options: animationOptions + ) } // // MARK: Notification Listeners // - @objc func keyboardFrameChanged(_ notification : Notification) - { + @objc func keyboardFrameChanged(_ notification : Notification) { + do { let info = try NotificationInfo(with: notification) self.receivedUpdatedKeyboardInfo(info) @@ -129,15 +123,15 @@ final class KeyboardObserver extension KeyboardObserver { - struct NotificationInfo : Equatable - { + struct NotificationInfo : Equatable { + var endingFrame : CGRect = .zero var animationDuration : Double = 0.0 var animationCurve : UInt = 0 - init(with notification : Notification) throws - { + init(with notification : Notification) throws { + guard let userInfo = notification.userInfo else { throw ParseError.missingUserInfo } @@ -161,8 +155,8 @@ extension KeyboardObserver self.animationCurve = animationCurve } - enum ParseError : Error, Equatable - { + enum ParseError : Error, Equatable { + case missingUserInfo case missingEndingFrame case missingAnimationDuration diff --git a/BlueprintUICommonControls/Sources/ScrollView.swift b/BlueprintUICommonControls/Sources/ScrollView.swift index eb823254e..c2c12071c 100644 --- a/BlueprintUICommonControls/Sources/ScrollView.swift +++ b/BlueprintUICommonControls/Sources/ScrollView.swift @@ -12,12 +12,21 @@ public struct ScrollView: Element { public var contentSize: ContentSize = .fittingHeight public var alwaysBounceVertical = false public var alwaysBounceHorizontal = false + + /** + How much the content of the `ScrollView` should be inset. + + Note: When `keyboardAdjustmentMode` is used, it will also adjust + the on-screen `UIScrollView`s `contentInset.bottom` to make space for the keyboard. + */ public var contentInset: UIEdgeInsets = .zero + public var centersUnderflow: Bool = false public var showsHorizontalScrollIndicator: Bool = true public var showsVerticalScrollIndicator: Bool = true public var pullToRefreshBehavior: PullToRefreshBehavior = .disabled public var keyboardDismissMode: UIScrollView.KeyboardDismissMode = .none + public var keyboardAdjustmentMode : KeyboardAdjustmentMode = .adjustsWhenVisible public init(wrapping element: Element) { self.wrappedElement = element @@ -29,10 +38,15 @@ public struct ScrollView: Element { public func backingViewDescription(bounds: CGRect, subtreeExtent: CGRect?) -> ViewDescription? { return ScrollerWrapperView.describe { config in + config.builder = { + ScrollerWrapperView(frame: bounds, representedElement: self) + } + config.contentView = { $0.scrollView } - config.apply({ (view) in - view.apply(scrollView: self, contentFrame: subtreeExtent ?? .zero) - }) + + config.apply { + $0.apply(scrollView: self, contentFrame: subtreeExtent ?? .zero) + } } } @@ -95,6 +109,11 @@ extension ScrollView { } extension ScrollView { + + public enum KeyboardAdjustmentMode : Equatable { + case none + case adjustsWhenVisible + } public enum ContentSize : Equatable { @@ -140,10 +159,13 @@ extension ScrollView { } -fileprivate final class ScrollerWrapperView: UIView, KeyboardObserverDelegate { +fileprivate final class ScrollerWrapperView: UIView { let scrollView = UIScrollView() let keyboardObserver = KeyboardObserver() + + /// The current `ScrollView` state we represent. + private var representedElement : ScrollView private var refreshControl: UIRefreshControl? = nil { @@ -162,7 +184,10 @@ fileprivate final class ScrollerWrapperView: UIView, KeyboardObserverDelegate { private var refreshAction: () -> Void = { } - override init(frame: CGRect) { + init(frame: CGRect, representedElement : ScrollView) { + + self.representedElement = representedElement + super.init(frame: frame) self.keyboardObserver.delegate = self @@ -186,6 +211,8 @@ fileprivate final class ScrollerWrapperView: UIView, KeyboardObserverDelegate { func apply(scrollView: ScrollView, contentFrame: CGRect) { + self.representedElement = scrollView + switch scrollView.pullToRefreshBehavior { case .disabled, .refreshing: refreshAction = { } @@ -244,16 +271,24 @@ fileprivate final class ScrollerWrapperView: UIView, KeyboardObserverDelegate { if self.scrollView.keyboardDismissMode != scrollView.keyboardDismissMode { self.scrollView.keyboardDismissMode = scrollView.keyboardDismissMode } - - var contentInset = scrollView.contentInset - - if case .refreshing = scrollView.pullToRefreshBehavior, let refreshControl = refreshControl { - // The refresh control lives above the content and adjusts the - // content inset for itself when visible. Do the same adjustment to - // our expected content inset. - contentInset.top += refreshControl.bounds.height - } - + + self.applyContentInset(with: scrollView) + } + + private func applyContentInset(with scrollView : ScrollView) + { + // Keep a copy of the content inset as provided by the developer. + // We will use this when adjusting the `contentInset.bottom` for the keyboard. + + let contentInset = ScrollView.finalContentInset( + with: scrollView.contentInset, + keyboardBottomInset: self.bottomContentInsetAdjustmentForKeyboard, + refreshControlState: scrollView.pullToRefreshBehavior, + refreshControlBounds: refreshControl?.bounds + ) + + // Apply the updated contentInset if it changed. + if self.scrollView.contentInset != contentInset { let wasScrolledToTop = self.scrollView.contentOffset.y == -self.scrollView.contentInset.top @@ -279,7 +314,7 @@ fileprivate final class ScrollerWrapperView: UIView, KeyboardObserverDelegate { super.didMoveToWindow() if self.window != nil { - self.setContentInsetWithKeyboardFrame() + self.updateBottomContentInsetWithKeyboardFrame() } } @@ -287,35 +322,102 @@ fileprivate final class ScrollerWrapperView: UIView, KeyboardObserverDelegate { super.didMoveToSuperview() if self.superview != nil { - self.setContentInsetWithKeyboardFrame() + self.updateBottomContentInsetWithKeyboardFrame() + } + } +} + + +extension ScrollView +{ + // Calculates the correct content inset to apply for the given inputs. + + static func finalContentInset( + with providedInsets : UIEdgeInsets, + keyboardBottomInset : CGFloat, + refreshControlState : PullToRefreshBehavior, + refreshControlBounds : CGRect? + ) -> UIEdgeInsets + { + var finalContentInset = providedInsets + + // Include the keyboard's adjustment at the bottom of the scroll view. + + finalContentInset.bottom += keyboardBottomInset + + // The refresh control lives above the content and adjusts the + // content inset for itself when visible and refreshing. + // Do the same adjustment to our expected content inset. + + if case .refreshing = refreshControlState { + finalContentInset.top += refreshControlBounds?.size.height ?? 0.0 } + + return finalContentInset } +} + + +extension ScrollerWrapperView : KeyboardObserverDelegate { // // MARK: Keyboard // - private func setContentInsetWithKeyboardFrame() { - guard let frame = self.keyboardObserver.currentFrame(in: self) else { - return - } + private func updateBottomContentInsetWithKeyboardFrame() { + + let contentInset = ScrollView.finalContentInset( + with: self.representedElement.contentInset, + keyboardBottomInset: self.bottomContentInsetAdjustmentForKeyboard, + refreshControlState: self.representedElement.pullToRefreshBehavior, + refreshControlBounds: self.refreshControl?.bounds + ) - var inset : CGFloat + /// Setting contentInset, even to the same value, can cause issues during scrolling (such as stopping scrolling). + /// Make sure we're only assigning the value if it changed. - switch frame { - case .notVisible: inset = 0.0 - case .visible(let frame): inset = (self.bounds.size.height - frame.origin.y) + if self.scrollView.contentInset.bottom != contentInset.bottom { + self.scrollView.contentInset.bottom = contentInset.bottom } + } + + fileprivate var bottomContentInsetAdjustmentForKeyboard : CGFloat { - self.scrollView.contentInset.bottom = inset + switch self.representedElement.keyboardAdjustmentMode { + case .none: + return 0.0 + + case .adjustsWhenVisible: + guard let keyboardFrame = self.keyboardObserver.currentFrame(in: self) else { + return 0.0 + } + + var bottomInset : CGFloat + + switch keyboardFrame { + case .notVisible: bottomInset = 0.0 + case .visible(let frame): bottomInset = (self.bounds.size.height - frame.origin.y) + } + + return bottomInset + } } // // MARK: KeyboardObserverDelegate // - func keyboardFrameWillChange(observer : KeyboardObserver) - { - self.setContentInsetWithKeyboardFrame() + func keyboardFrameWillChange( + for observer : KeyboardObserver, + animationDuration : Double, + options : UIView.AnimationOptions + ) { + if animationDuration > 0.0 { + UIView.animate(withDuration: animationDuration, delay: 0.0, options: options, animations: { + self.updateBottomContentInsetWithKeyboardFrame() + }) + } else { + self.updateBottomContentInsetWithKeyboardFrame() + } } } diff --git a/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift b/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift index 8146bed88..269fece96 100644 --- a/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift +++ b/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift @@ -4,10 +4,9 @@ import UIKit @testable import BlueprintUICommonControls -class KeyboardObserverTests: XCTestCase -{ - func test_notifications() - { +class KeyboardObserverTests: XCTestCase { + + func test_notifications() { let center = NotificationCenter() self.testcase("Will Change Frame") { @@ -66,8 +65,8 @@ class KeyboardObserverTests: XCTestCase } } - final class Delegate : KeyboardObserverDelegate - { + final class Delegate : KeyboardObserverDelegate { + var keyboardFrameWillChange_callCount : Int = 0 func keyboardFrameWillChange(observer: KeyboardObserver) @@ -80,10 +79,10 @@ class KeyboardObserverTests: XCTestCase } -class KeyboardObserver_NotificationInfo_Tests : XCTestCase -{ - func test_init() - { +class KeyboardObserver_NotificationInfo_Tests : XCTestCase { + + func test_init() { + let defaultUserInfo : [AnyHashable:Any] = [ UIResponder.keyboardFrameEndUserInfoKey : NSValue(cgRect: CGRect(x: 10.0, y: 10.0, width: 100.0, height: 200.0)), UIResponder.keyboardAnimationDurationUserInfoKey : NSNumber(value: 2.5), @@ -159,15 +158,14 @@ class KeyboardObserver_NotificationInfo_Tests : XCTestCase } -extension XCTestCase -{ - func testcase(_ name : String = "", _ block : () -> ()) - { +extension XCTestCase { + + func testcase(_ name : String = "", _ block : () -> ()) { block() } - func assertThrowsError(test : () throws -> (), verify : (Error) -> ()) - { + func assertThrowsError(test : () throws -> (), verify : (Error) -> ()) { + var thrown = false do {