From 13309a54287e4a199a3893ec33cb15ef31ea7b36 Mon Sep 17 00:00:00 2001 From: Robert MacEachern Date: Fri, 1 Sep 2023 14:38:16 -0500 Subject: [PATCH] Update KeyboardObserver (#463) * Update KeyboardObserver * Update CHANGELOG.md * Used shared instance of `KeyboardObserver` in `ScrollView` * Lenient screen parsing. Fallback to `window.screen` --- .../Sources/Internal/KeyboardObserver.swift | 147 +++++++++++++++--- .../Sources/ScrollView.swift | 11 +- .../Internal/KeyboardObserverTests.swift | 122 +++++++++++++-- .../Tests/Sources/XCTestCaseExtensions.swift | 28 ++++ CHANGELOG.md | 2 + 5 files changed, 267 insertions(+), 43 deletions(-) diff --git a/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift b/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift index c9536956b..3fa500ddf 100644 --- a/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift +++ b/BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift @@ -6,7 +6,7 @@ protocol KeyboardObserverDelegate: AnyObject { func keyboardFrameWillChange( for observer: KeyboardObserver, animationDuration: Double, - options: UIView.AnimationOptions + animationCurve: UIView.AnimationCurve ) } @@ -30,29 +30,46 @@ protocol KeyboardObserverDelegate: AnyObject { Notes ----- - Implementation borrowed from Listable: - https://github.com/kyleve/Listable/blob/master/Listable/Sources/Internal/KeyboardObserver.swift - iOS Docs for keyboard management: https://developer.apple.com/library/archive/documentation/StringsTextFonts/Conceptual/TextAndWebiPhoneOS/KeyboardManagement/KeyboardManagement.html */ final class KeyboardObserver { + /// The global shared keyboard observer. Why is it a global shared instance? + /// We can only know the keyboard position via the keyboard frame notifications. + /// + /// If a keyboard observing view is created while a keyboard is already on-screen, we'd have no way to determine the + /// keyboard frame, and thus couldn't provide the correct content insets to avoid the visible keyboard. + /// + /// Thus, the `shared` observer is set up on app startup + /// (see `SetupKeyboardObserverOnAppStartup.m`) to avoid this problem. + static let shared: KeyboardObserver = KeyboardObserver(center: .default) + + /// Allow logging to the console if app startup-timed shared instance startup did not + /// occur; this could cause bugs for the reasons outlined above. + fileprivate static var didSetupSharedInstanceDuringAppStartup = false + private let center: NotificationCenter - weak var delegate: KeyboardObserverDelegate? + private(set) var delegates: [Delegate] = [] + + struct Delegate { + private(set) weak var value: KeyboardObserverDelegate? + } // // MARK: Initialization // - init(center: NotificationCenter = .default) { + init(center: NotificationCenter) { self.center = center /// We need to listen to both `will` and `keyboardDidChangeFrame` notifications. Why? + /// /// When dealing with an undocked or floating keyboard, moving the keyboard /// around the screen does NOT call `willChangeFrame`; only `didChangeFrame` is called. + /// /// Before calling the delegate, we compare `old.endingFrame != new.endingFrame`, /// which ensures that the delegate is notified if the frame really changes, and /// prevents duplicate calls. @@ -73,6 +90,35 @@ final class KeyboardObserver { private var latestNotification: NotificationInfo? + // + // MARK: Delegates + // + + func add(delegate: KeyboardObserverDelegate) { + + if delegates.contains(where: { $0.value === delegate }) { + return + } + + delegates.append(Delegate(value: delegate)) + + removeDeallocatedDelegates() + } + + func remove(delegate: KeyboardObserverDelegate) { + delegates.removeAll { + $0.value === delegate + } + + removeDeallocatedDelegates() + } + + private func removeDeallocatedDelegates() { + delegates.removeAll { + $0.value == nil + } + } + // // MARK: Handling Changes // @@ -90,7 +136,7 @@ final class KeyboardObserver { /// or the observer has not yet learned about the keyboard's position, this method returns nil. func currentFrame(in view: UIView) -> KeyboardFrame? { - guard view.window != nil else { + guard let window = view.window else { return nil } @@ -98,7 +144,12 @@ final class KeyboardObserver { return nil } - let frame = view.convert(notification.endingFrame, from: nil) + let screen = notification.screen ?? window.screen + + let frame = screen.coordinateSpace.convert( + notification.endingFrame, + to: view + ) if frame.intersects(view.bounds) { return .overlapping(frame: frame) @@ -123,19 +174,13 @@ final class KeyboardObserver { return } - /** - 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) - - delegate?.keyboardFrameWillChange( - for: self, - animationDuration: new.animationDuration, - options: animationOptions - ) + delegates.forEach { + $0.value?.keyboardFrameWillChange( + for: self, + animationDuration: new.animationDuration, + animationCurve: new.animationCurve + ) + } } // @@ -148,7 +193,7 @@ final class KeyboardObserver { let info = try NotificationInfo(with: notification) receivedUpdatedKeyboardInfo(info) } catch { - assertionFailure("Blueprint could not read system keyboard notification. This error needs to be fixed in Blueprint. Error: \(error)") + assertionFailure("Could not read system keyboard notification: \(error)") } } } @@ -159,7 +204,21 @@ extension KeyboardObserver { var endingFrame: CGRect = .zero var animationDuration: Double = 0.0 - var animationCurve: UInt = 0 + var animationCurve: UIView.AnimationCurve = .easeInOut + + /// The `UIScreen` that the keyboard appears on. + /// + /// This may influence the `KeyboardFrame` calculation when the app is not in full screen, + /// such as in Split View, Slide Over, and Stage Manager. + /// + /// - note: In iOS 16.1 and later, every `keyboardWillChangeFrameNotification` and + /// `keyboardDidChangeFrameNotification` is _supposed_ to include a `UIScreen` + /// in a the notification, however we've had reports that this isn't always the case (at least when + /// using the iOS 16.1 simulator runtime). If a screen is _not_ included in an iOS 16.1+ notification, + /// we do not throw a `ParseError` as it would cause the entire notification to be discarded. + /// + /// [Apple Documentation](https://developer.apple.com/documentation/uikit/uiresponder/1621623-keyboardwillchangeframenotificat) + var screen: UIScreen? init(with notification: Notification) throws { @@ -179,11 +238,15 @@ extension KeyboardObserver { self.animationDuration = animationDuration - guard let animationCurve = (userInfo[UIResponder.keyboardAnimationCurveUserInfoKey] as? NSNumber)?.uintValue else { + guard let curveValue = (userInfo[UIResponder.keyboardAnimationCurveUserInfoKey] as? NSNumber)?.intValue, + let animationCurve = UIView.AnimationCurve(rawValue: curveValue) + else { throw ParseError.missingAnimationCurve } self.animationCurve = animationCurve + + screen = notification.object as? UIScreen } enum ParseError: Error, Equatable { @@ -195,3 +258,39 @@ extension KeyboardObserver { } } } + + +extension KeyboardObserver { + private static let isExtensionContext: Bool = { + // This is our best guess for "is this executable an extension?" + if let _ = Bundle.main.infoDictionary?["NSExtension"] { + return true + } else if Bundle.main.bundlePath.hasSuffix(".appex") { + return true + } else { + return false + } + }() + + /// This should be called by a keyboard-observing view on setup, to warn developers if something has gone wrong with + /// keyboard setup. + static func logKeyboardSetupWarningIfNeeded() { + guard !isExtensionContext else { + return + } + + if KeyboardObserver.didSetupSharedInstanceDuringAppStartup { + return + } + + print( + """ + WARNING: The shared instance of the `KeyboardObserver` was not instantiated during + app startup. While not fatal, this could result in a view being created that does + not properly position itself to account for the keyboard, if the view is created + while the keyboard is already visible. + """ + ) + } +} + diff --git a/BlueprintUICommonControls/Sources/ScrollView.swift b/BlueprintUICommonControls/Sources/ScrollView.swift index 964ecd51a..12ee1a0bd 100644 --- a/BlueprintUICommonControls/Sources/ScrollView.swift +++ b/BlueprintUICommonControls/Sources/ScrollView.swift @@ -304,7 +304,7 @@ extension ScrollView { fileprivate final class ScrollerWrapperView: UIView { let scrollView = UIScrollView() - let keyboardObserver = KeyboardObserver() + let keyboardObserver = KeyboardObserver.shared /// The current `ScrollView` state we represent. private var representedElement: ScrollView @@ -341,7 +341,7 @@ fileprivate final class ScrollerWrapperView: UIView { super.init(frame: frame) - keyboardObserver.delegate = self + keyboardObserver.add(delegate: self) addSubview(scrollView) } @@ -564,11 +564,12 @@ extension ScrollerWrapperView: KeyboardObserverDelegate { func keyboardFrameWillChange( for observer: KeyboardObserver, animationDuration: Double, - options: UIView.AnimationOptions + animationCurve: UIView.AnimationCurve ) { - UIView.animate(withDuration: animationDuration, delay: 0.0, options: options, animations: { + UIViewPropertyAnimator(duration: animationDuration, curve: animationCurve) { self.updateBottomContentInsetWithKeyboardFrame() - }) + } + .startAnimation() } } diff --git a/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift b/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift index 5e395fa35..4621650ff 100644 --- a/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift +++ b/BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift @@ -6,6 +6,76 @@ import XCTest class KeyboardObserverTests: XCTestCase { + func test_add() { + let center = NotificationCenter() + let observer = KeyboardObserver(center: center) + + var delegate1: Delegate? = Delegate() + weak var weakDelegate1 = delegate1 + + let delegate2 = Delegate() + let delegate3 = Delegate() + + // Validate that delegates are only registered once. + + XCTAssertEqual(observer.delegates.count, 0) + + observer.add(delegate: delegate1!) + XCTAssertEqual(observer.delegates.count, 1) + + observer.add(delegate: delegate1!) + XCTAssertEqual(observer.delegates.count, 1) + + // Register a second observer + + observer.add(delegate: delegate2) + XCTAssertEqual(observer.delegates.count, 2) + + // Register a third, but deallocate the first. Should be removed. + + delegate1 = nil + + waitFor { + weakDelegate1 == nil + } + + observer.add(delegate: delegate3) + XCTAssertEqual(observer.delegates.count, 2) + } + + func test_remove() { + let center = NotificationCenter() + let observer = KeyboardObserver(center: center) + + let delegate1: Delegate? = Delegate() + + var delegate2: Delegate? = Delegate() + weak var weakDelegate2 = delegate2 + + let delegate3: Delegate? = Delegate() + + // Register all 3 observers + + observer.add(delegate: delegate1!) + observer.add(delegate: delegate2!) + observer.add(delegate: delegate3!) + + XCTAssertEqual(observer.delegates.count, 3) + + // Nil out the second delegate + + delegate2 = nil + + waitFor { + weakDelegate2 == nil + } + + // Should only have 1 left + + observer.remove(delegate: delegate3!) + XCTAssertEqual(observer.delegates.count, 1) + } + func test_notifications() { let center = NotificationCenter() @@ -14,7 +84,7 @@ class KeyboardObserverTests: XCTestCase { let observer = KeyboardObserver(center: center) let delegate = Delegate() - observer.delegate = delegate + observer.add(delegate: delegate) let userInfo: [AnyHashable: Any] = [ UIResponder.keyboardFrameEndUserInfoKey: NSValue(cgRect: CGRect( @@ -28,7 +98,11 @@ class KeyboardObserverTests: XCTestCase { ] XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 0) - center.post(Notification(name: UIWindow.keyboardWillChangeFrameNotification, object: nil, userInfo: userInfo)) + center.post(Notification( + name: UIWindow.keyboardWillChangeFrameNotification, + object: UIScreen.main, + userInfo: userInfo + )) XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 1) } @@ -37,7 +111,7 @@ class KeyboardObserverTests: XCTestCase { let observer = KeyboardObserver(center: center) let delegate = Delegate() - observer.delegate = delegate + observer.add(delegate: delegate) let userInfo: [AnyHashable: Any] = [ UIResponder.keyboardFrameEndUserInfoKey: NSValue(cgRect: CGRect( @@ -51,7 +125,11 @@ class KeyboardObserverTests: XCTestCase { ] XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 0) - center.post(Notification(name: UIWindow.keyboardDidChangeFrameNotification, object: nil, userInfo: userInfo)) + center.post(Notification( + name: UIWindow.keyboardDidChangeFrameNotification, + object: UIScreen.main, + userInfo: userInfo + )) XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 1) } @@ -60,7 +138,7 @@ class KeyboardObserverTests: XCTestCase { let observer = KeyboardObserver(center: center) let delegate = Delegate() - observer.delegate = delegate + observer.add(delegate: delegate) let userInfo: [AnyHashable: Any] = [ UIResponder.keyboardFrameEndUserInfoKey: NSValue(cgRect: CGRect( @@ -74,11 +152,19 @@ class KeyboardObserverTests: XCTestCase { ] XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 0) - center.post(Notification(name: UIWindow.keyboardDidChangeFrameNotification, object: nil, userInfo: userInfo)) + center.post(Notification( + name: UIWindow.keyboardDidChangeFrameNotification, + object: UIScreen.main, + userInfo: userInfo + )) XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 1) XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 1) - center.post(Notification(name: UIWindow.keyboardDidChangeFrameNotification, object: nil, userInfo: userInfo)) + center.post(Notification( + name: UIWindow.keyboardDidChangeFrameNotification, + object: UIScreen.main, + userInfo: userInfo + )) XCTAssertEqual(delegate.keyboardFrameWillChange_callCount, 1) } } @@ -90,7 +176,7 @@ class KeyboardObserverTests: XCTestCase { func keyboardFrameWillChange( for observer: KeyboardObserver, animationDuration: Double, - options: UIView.AnimationOptions + animationCurve: UIView.AnimationCurve ) { keyboardFrameWillChange_callCount += 1 @@ -117,12 +203,16 @@ class KeyboardObserver_NotificationInfo_Tests: XCTestCase { // Successful Init do { let info = try! KeyboardObserver.NotificationInfo( - with: Notification(name: UIResponder.keyboardDidShowNotification, object: nil, userInfo: defaultUserInfo) + with: Notification( + name: UIResponder.keyboardDidShowNotification, + object: UIScreen.main, + userInfo: defaultUserInfo + ) ) XCTAssertEqual(info.endingFrame, CGRect(x: 10.0, y: 10.0, width: 100.0, height: 200.0)) XCTAssertEqual(info.animationDuration, 2.5) - XCTAssertEqual(info.animationCurve, 123) + XCTAssertEqual(info.animationCurve, UIView.AnimationCurve(rawValue: 123)!) } // Failed Inits @@ -131,7 +221,11 @@ class KeyboardObserver_NotificationInfo_Tests: XCTestCase { do { XCTAssertThrowsError( try _ = KeyboardObserver.NotificationInfo( - with: Notification(name: UIResponder.keyboardDidShowNotification, object: nil, userInfo: nil) + with: Notification( + name: UIResponder.keyboardDidShowNotification, + object: UIScreen.main, + userInfo: nil + ) ) ) { error in XCTAssertEqual(error as? KeyboardObserver.NotificationInfo.ParseError, .missingUserInfo) @@ -147,7 +241,7 @@ class KeyboardObserver_NotificationInfo_Tests: XCTestCase { try _ = KeyboardObserver.NotificationInfo( with: Notification( name: UIResponder.keyboardDidShowNotification, - object: nil, + object: UIScreen.main, userInfo: userInfo ) ) @@ -165,7 +259,7 @@ class KeyboardObserver_NotificationInfo_Tests: XCTestCase { try _ = KeyboardObserver.NotificationInfo( with: Notification( name: UIResponder.keyboardDidShowNotification, - object: nil, + object: UIScreen.main, userInfo: userInfo ) ) @@ -183,7 +277,7 @@ class KeyboardObserver_NotificationInfo_Tests: XCTestCase { try KeyboardObserver.NotificationInfo( with: Notification( name: UIResponder.keyboardDidShowNotification, - object: nil, + object: UIScreen.main, userInfo: userInfo ) ) diff --git a/BlueprintUICommonControls/Tests/Sources/XCTestCaseExtensions.swift b/BlueprintUICommonControls/Tests/Sources/XCTestCaseExtensions.swift index ec2892a5e..432f76ea7 100644 --- a/BlueprintUICommonControls/Tests/Sources/XCTestCaseExtensions.swift +++ b/BlueprintUICommonControls/Tests/Sources/XCTestCaseExtensions.swift @@ -172,3 +172,31 @@ extension XCTestCase { window.windowScene = nil } } + +extension XCTestCase { + + /// Allows waiting for asynchronous work to complete within a test method by + /// spinning the main runloop until the `predicate` returns true. + public func waitFor( + timeout: TimeInterval = 10.0, + predicate: () throws -> Bool, + error: (() -> String)? = nil + ) rethrows { + let runloop = RunLoop.main + let timeout = Date(timeIntervalSinceNow: timeout) + + while Date() < timeout { + if try predicate() { + return + } + + runloop.run(mode: .default, before: Date(timeIntervalSinceNow: 0.001)) + } + + if let error = error { + XCTFail("waitUntil timed out waiting for a check to pass. Error: \(error())") + } else { + XCTFail("waitUntil timed out waiting for a check to pass.") + } + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d463a340..ea40e2ed0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `KeyboardObserver` has been updated to handle iOS 16.1+ changes that use the screen's coordinate space to report keyboard position. This can impact reported values when the app isn't full screen in Split View, Slide Over, and Stage Manager. + ### Added ### Removed