freshOS / KeyboardLayoutGuide

⌨️ KeyboardLayoutGuide, back from when it didn't exist.
MIT License
1.19k stars 59 forks source link

Better API for using safe area #28

Closed abekert closed 4 years ago

abekert commented 4 years ago

This is a very handy framework!

Currently keyboardLayoutGuide automatically pins to safeAreaLayoutGuide.bottom. There is a way to disable it getting a pointer to keyboardLayoutGuide and setting its usesSafeArea property to usesSafeArea.

Issue

There are two cases:

1) Currently you can't pin two existing views to the keyboardLayoutGuide, one taking safeArea into account, the second doesn't — keyboardLayoutGuide is a single instance property. 2) It's not very comfy. You should write something like:

let keyboardLayoutGuide = view.keyboardLayoutGuide // we don't know about the implementation, so we're holding a reference here
keyboardLayoutGuide.usesSafeArea = false
pinnedView.bottomAnchor.constraint(equalTo: keyboardLayoutGuide.topAnchor).isActive = true

Solution

I prefer one from the following:

1) having a separate property for keyboardLayoutGuide with usesSafeArea = false

pinnedView.bottomAnchor.constraint(equalTo: keyboardLayoutGuide.topAnchor).isActive = true
pinnedView.bottomAnchor.constraint(equalTo: keyboardLayoutGuideNoSafeArea.topAnchor).isActive = true

2) having a computed property returning KeyboardLayoutGuide with usesSafeArea turned off

pinnedView.bottomAnchor.constraint(equalTo: keyboardLayoutGuide.topAnchor).isActive = true
pinnedView.bottomAnchor.constraint(equalTo: keyboardLayoutGuide.topAnchor).isActive = true
pinnedView.bottomAnchor.constraint(equalTo: keyboardLayoutGuide.noSafeArea.topAnchor).isActive = true

Achievable via extension like:

public extension KeyboardLayoutGuide {
    var noSafeArea: KeyboardLayoutGuide {
        self.usesSafeArea = false
        return self
    }
}
s4cha commented 4 years ago

Hi @abekert, first of all massive thanks for taking the time to report this and document it very clearly. This is very appreciated. This is a limitation indeed, I had not thought of this but this is a perfectly valid use case. As you noted in your PR, to support this we need the distinct layout guides. The PR looks very good. I'm going to review it right now

PS: please excuse the late reply 🙏