ScrollView observes and adjusts for keyboard position #55

kyleve · 2020-02-17T03:01:01Z

This PR makes ScrollView smart about observing the position of the system keyboard. When the keyboard is displayed, updated, or dismissed, the ScrollView will adjust its contentInset.bottom to account for the updated inset. This is done through KeyboardObserver, which is an object to observe and manage keyboard positioning, which I've copied over from Listable. I've also added keyboardDismissMode to ScrollView in this PR, allowing management of keyboard dismissal in ScrollView instances.

BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift

BlueprintUICommonControls/Sources/ScrollView.swift

Internal/KeyboardObserver.swift

BlueprintUICommonControls.podspec

BlueprintUICommonControls/Sources/ScrollView.swift

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

BlueprintUICommonControls/Sources/ScrollView.swift

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift

BlueprintUICommonControls/Sources/ScrollView.swift

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

kyleve · 2020-02-26T20:28:53Z

Pushed some, but not all requested changes, will get to those later.

This update also includes changes to how we manage contentInset overall, because the provided ScrollView.contentInset and the keyboard's bottom inset needed to play nicer together. This updates ensures we continue using ScrollView.contentInset.bottom alongside the additional inset from the keyboard. Also centralized calculation of the final inset to one place.

BlueprintUICommonControls/Sources/ScrollView.swift

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

BlueprintUICommonControls/Sources/ScrollView.swift

kylebshr · 2020-03-05T18:39:59Z

Could you verify this isn't also an issue with the implementation here?

bizzemfrog · 2020-03-20T04:15:39Z

any movement on this?

kyleve · 2020-03-21T03:02:24Z

Nope... can try to look soon.

kyleve · 2020-03-23T00:28:27Z

Could you verify this isn't also an issue with the implementation here?

Doesn't seem to be an issue from testing

EDIT: Spoke too soon.. wasn't seeing this on iphone, but it is happening on ipad. Will fix.

SampleApp/Sources/AppDelegate.swift

kyleve · 2020-03-25T20:24:42Z

This should be good for another review.

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

* master: Releasing 0.7.0 Fix comment about extraSize ASCII diagrams in stack layout Measure stack axes separately A better workaround for missing dylibs Workaround for libswiftsimd.dylib not being copied Bump CocoaPods version Update to Xcode 11 and Swift 5.1 Remove iOS 10 availability checks Raise the minimum iOS version to 10 Releasing 0.6.0 Don't apply keyboardDismissMode unless it changed Change property from Bool to UIScrollView.KeyboardDismissMode and pass it directly Allow ScrollView to dismiss the keyboard on tap

watt · 2020-03-30T22:40:25Z

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

+        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


What actually happens with an undocked keyboard? We don't want to treat it as a bottom content inset in those cases, do we? e.g. if the keyboard is floating near the top of the screen, we should probably just leave the content offset alone.

I went back and forth on this... there's four forms of keyboards:

Docked, takes up full width

Undocked, takes up full width

Also undocked, split, also semantically is full width

Floating on iPad, does not take up full width

I figured we generally want this for the first three cases, so kept it simple for now. I'll follow up with something for #4, to not inset in this case?

watt · 2020-03-30T22:44:15Z

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

+
+extension KeyboardObserver
+{
+    struct NotificationInfo : Equatable {


Needs to be internal for testing access.

BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift

watt · 2020-03-30T22:57:41Z

BlueprintUICommonControls/Sources/ScrollView.swift

+
+private extension UIView {
+
+    var bp_safeAreaInsets : UIEdgeInsets {


Smells like Obj-C. How about a name like polyfillSafeAreaInsets?

I'd rather keep this as is... It's clear to developers from patterns what this means.

watt · 2020-03-30T23:10:59Z

BlueprintUICommonControls/Sources/ScrollView.swift

+    // MARK: Keyboard
+    //
+
+    private func updateBottomContentInsetWithKeyboardFrame() {


This is almost the same func as applyContentInset, can they merge?

Doh, will do

Need to dig into this more... there's a bit of nuanced behaviour between the logic in applyContentInset handling changes only from the model updates, vs updateBottomContentInsetWithKeyboardFrame always updating on live views.

Going to leave as is for now and come back to this one.

watt · 2020-03-30T23:18:24Z

BlueprintUICommonControls/Sources/ScrollView.swift

+
+     Note: When `keyboardAdjustmentMode` is used, it will also adjust
+     the on-screen `UIScrollView`s `contentInset.bottom` to make space for the keyboard.
+     */


👍 for comments!

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift

BlueprintUICommonControls/Sources/ScrollView.swift

BlueprintUICommonControls/Tests/ScrollViewTests.swift

kyleve commented Feb 17, 2020

View reviewed changes

BlueprintUICommonControls/Tests/Sources/Internal/KeyboardObserverTests.swift Outdated Show resolved Hide resolved

kyleve commented Feb 19, 2020

View reviewed changes

BlueprintUICommonControls/Sources/ScrollView.swift Show resolved Hide resolved

kyleve commented Feb 19, 2020

View reviewed changes

Internal/KeyboardObserver.swift Outdated Show resolved Hide resolved

bizzemfrog reviewed Feb 22, 2020

View reviewed changes

Internal/KeyboardObserver.swift Outdated Show resolved Hide resolved

kyleve commented Feb 23, 2020

View reviewed changes

Internal/KeyboardObserver.swift Outdated Show resolved Hide resolved

kyleve commented Feb 23, 2020

View reviewed changes

BlueprintUICommonControls.podspec Outdated Show resolved Hide resolved

kyleve changed the title ~~[WIP] ScrollView observes the keyboard~~ ScrollView observes and adjusts for keyboard position Feb 23, 2020

kyleve requested review from timdonnelly, watt, bencochran and kylebshr February 23, 2020 04:16

vocaro reviewed Feb 23, 2020

View reviewed changes

BlueprintUICommonControls/Sources/ScrollView.swift Show resolved Hide resolved

watt reviewed Feb 26, 2020

View reviewed changes

timdonnelly reviewed Feb 26, 2020

View reviewed changes

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift Outdated Show resolved Hide resolved

kyleve force-pushed the kve/keyboard-observation branch from ff932db to d42c5c1 Compare February 26, 2020 20:34

kyleve commented Feb 26, 2020

View reviewed changes

BlueprintUICommonControls/Sources/ScrollView.swift Outdated Show resolved Hide resolved

kylebshr reviewed Mar 5, 2020

View reviewed changes

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift Outdated Show resolved Hide resolved

BlueprintUICommonControls/Sources/ScrollView.swift Outdated Show resolved Hide resolved

BlueprintUICommonControls/Sources/ScrollView.swift Outdated Show resolved Hide resolved

kyleve commented Mar 23, 2020

View reviewed changes

SampleApp/Sources/AppDelegate.swift Outdated Show resolved Hide resolved

kylebshr approved these changes Mar 30, 2020

View reviewed changes

BlueprintUICommonControls/Sources/Internal/KeyboardObserver.swift Outdated Show resolved Hide resolved

Add API for keyboard observation on ScrollView

141eabf

kyleve force-pushed the kve/keyboard-observation branch from 175a19b to 141eabf Compare March 30, 2020 22:23

kyleve added 2 commits March 30, 2020 15:33

Revert BlueprintUICommonControls.podspec testspec

8dcd5d8

watt reviewed Mar 30, 2020

View reviewed changes

Code review

cea7aa6

kyleve commented Mar 31, 2020

View reviewed changes

BlueprintUICommonControls/Tests/ScrollViewTests.swift Outdated Show resolved Hide resolved

kyleve commented Mar 31, 2020

View reviewed changes

BlueprintUICommonControls/Tests/ScrollViewTests.swift Outdated Show resolved Hide resolved

Code review fixes

95633fd

kyleve force-pushed the kve/keyboard-observation branch from 7a5a39b to 95633fd Compare March 31, 2020 01:27

kyleve merged commit 120dff5 into master Apr 1, 2020

watt deleted the kve/keyboard-observation branch November 24, 2021 02:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ScrollView observes and adjusts for keyboard position #55

ScrollView observes and adjusts for keyboard position #55

kyleve commented Feb 17, 2020 •

edited

Loading

kyleve commented Feb 26, 2020

kylebshr commented Mar 5, 2020

bizzemfrog commented Mar 20, 2020

kyleve commented Mar 21, 2020

kyleve commented Mar 23, 2020 •

edited

Loading

kyleve commented Mar 25, 2020

watt Mar 30, 2020

kyleve Mar 31, 2020

kyleve Mar 31, 2020

watt Mar 30, 2020

kyleve Mar 31, 2020

watt Mar 30, 2020

kyleve Mar 31, 2020

watt Mar 30, 2020

kyleve Mar 31, 2020

kyleve Mar 31, 2020

kyleve Mar 31, 2020

watt Mar 30, 2020


		private extension UIView {

		var bp_safeAreaInsets : UIEdgeInsets {

ScrollView observes and adjusts for keyboard position #55

ScrollView observes and adjusts for keyboard position #55

Conversation

kyleve commented Feb 17, 2020 • edited Loading

kyleve commented Feb 26, 2020

kylebshr commented Mar 5, 2020

bizzemfrog commented Mar 20, 2020

kyleve commented Mar 21, 2020

kyleve commented Mar 23, 2020 • edited Loading

kyleve commented Mar 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyleve commented Feb 17, 2020 •

edited

Loading

kyleve commented Mar 23, 2020 •

edited

Loading