nathantannar4 / InputBarAccessoryView

A simple and easily customizable InputAccessoryView for making powerful input bars with autocomplete and attachments
MIT License
1.17k stars 229 forks source link

Layout issues on iPad when using pagesheets #238

Closed Janneman84 closed 1 year ago

Janneman84 commented 2 years ago

I just updated to MessageKit 4.0. My chats are shown on a pagesheet VC. When the keyboard shows on iPad the input bar goes up way too much. This is on an iOS 16.0 simulator:

Simulator Screen Shot - iPad Air (5th generation) - 2022-09-30 at 12 37 09

It used to work fine on older versions.

Kaspik commented 2 years ago

Hey.

1) Should this be MessageKit issue instead or IBAV? I see you created duplicate - https://github.com/MessageKit/MessageKit/issues/1750 2) There was a change in 6.1.0 that added bottom constraint - https://github.com/nathantannar4/InputBarAccessoryView/pull/230 - did you check that?

nathantannar4 commented 2 years ago

Possibly related to #141, but could also be MessageKit

Janneman84 commented 1 year ago

I managed to fix the issue myself. It turns out the fault lies with InputBarAccessoryView and not MessageKit.

The current code assumes the bar is positioned to the very bottom of the screen. In case of pagesheets on iPad this is not the case. Also when using VC containers this might happen. When the keyboard goes up the bar goes up the same amount even when it wasn't stuck to the bottom of the screen, causing a gap.

I created some code that checks and compensates for this. I'm tweaking it right now.

nathantannar4 commented 1 year ago

@Janneman84 thanks for finding the fault. I haven't been working on this project, though I'm trying to get involved again.

It's seems as though there have been some regressions perhaps in recent released of InputBarAccessoryView? Must be the case if you updated and it broke.

Janneman84 commented 1 year ago

I believe it recently moved from being an inputAccessoryView to being a regular subview. I don't know why this was done, but I'm glad they did because it fixes some weird bugs and glitches.

This move caused new bugs of course, but nothing that can't be fixed.

Kaspik commented 1 year ago

Yes, this was done in https://github.com/MessageKit/MessageKit/pull/1704.

Janneman84 commented 1 year ago

Oh I see so this package supports both ways and you can choose?

Janneman84 commented 1 year ago

I created some code to fix the issue. I'm sorry it's not a proper MR but it's a bit hacky so if someone can give it a try first would be nice.

Try putting this in KeyboardManager.swift:

/// Used to fix a glitch that would otherwise occur when using pagesheets on iPad in iOS 14
private var justDidWillHide = false
/// When e.g. using pagesheets on iPad the inputAccessoryView is not stuck to the bottom of the screen.
/// This value represents the size of the gap between the bottom of the screen and the bottom of the inputAccessoryView.
private var bottomGap: CGFloat {
    if let inputAccessoryView = inputAccessoryView, let window = inputAccessoryView.window, let superView = inputAccessoryView.superview {
        return window.frame.height - superView.convert(superView.frame, to: window).maxY
    }
    return 0
}

Change this:

var yCoordinateDirectlyAboveKeyboard = -frame.height

To this:

var yCoordinateDirectlyAboveKeyboard = -frame.height + bottomGap

And most importantly replace the bind method like this:

@discardableResult
open func bind(inputAccessoryView: UIView, withAdditionalBottomSpace additionalBottomSpace: (() -> CGFloat)? = .none) -> Self {

    guard let superview = inputAccessoryView.superview else {
        fatalError("`inputAccessoryView` must have a superview")
    }
    self.inputAccessoryView = inputAccessoryView
    self.additionalBottomSpace = additionalBottomSpace
    inputAccessoryView.translatesAutoresizingMaskIntoConstraints = false
    constraints = NSLayoutConstraintSet(
        bottom: inputAccessoryView.bottomAnchor.constraint(equalTo: superview.bottomAnchor, constant: additionalInputViewBottomConstraintConstant()),
        left: inputAccessoryView.leftAnchor.constraint(equalTo: superview.leftAnchor),
        right: inputAccessoryView.rightAnchor.constraint(equalTo: superview.rightAnchor)
    ).activate()

    callbacks[.willShow] = { [weak self] (notification) in
        guard
            self?.isKeyboardHidden == false,
            self?.constraints?.bottom?.constant == self?.additionalInputViewBottomConstraintConstant(),
            notification.isForCurrentApp
        else { return }

        let keyboardHeight = notification.endFrame.height
        self?.animateAlongside(notification) {
            self?.constraints?.bottom?.constant = min(0, -keyboardHeight + (self?.bottomGap ?? 0)) - (additionalBottomSpace?() ?? 0)
            self?.inputAccessoryView?.superview?.layoutIfNeeded()
        }

        // Doing it a second time delayed is required for accurate placement
        DispatchQueue.main.async {
            let bottomGap = self?.bottomGap ?? 0
            if bottomGap != 0 {
                self?.animateAlongside(notification) {
                    self?.constraints?.bottom?.constant = min(0, -keyboardHeight + bottomGap) - (additionalBottomSpace?() ?? 0)
                    self?.inputAccessoryView?.superview?.layoutIfNeeded()
                }
            }
        }
    }
    callbacks[.willChangeFrame] = { [weak self] (notification) in
        let keyboardHeight = notification.endFrame.height
        guard
            self?.isKeyboardHidden == false,
            notification.isForCurrentApp
        else {
            return
        }
        self?.animateAlongside(notification) {
            self?.constraints?.bottom?.constant = min(0, -keyboardHeight + (self?.bottomGap ?? 0)) - (additionalBottomSpace?() ?? 0)
            self?.inputAccessoryView?.superview?.layoutIfNeeded()
        }

        // Doing it a second time delayed is required for accurate placement
        DispatchQueue.main.async {
            let bottomGap = self?.bottomGap ?? 0
            if bottomGap != 0 && !(self?.justDidWillHide ?? false) {
                self?.animateAlongside(notification) {
                    self?.constraints?.bottom?.constant = min(0, -keyboardHeight + bottomGap) - (additionalBottomSpace?() ?? 0)
                    self?.inputAccessoryView?.superview?.layoutIfNeeded()
                }
            }
        }
    }
    callbacks[.willHide] = { [weak self] (notification) in
        guard notification.isForCurrentApp else { return }
        self?.justDidWillHide = true
        self?.animateAlongside(notification) { [weak self] in
            self?.constraints?.bottom?.constant = self?.additionalInputViewBottomConstraintConstant() ?? 0
            self?.inputAccessoryView?.superview?.layoutIfNeeded()
        }

        // Doing it a second time delayed is required for accurate placement
        DispatchQueue.main.async {
            self?.justDidWillHide = false
            self?.animateAlongside(notification) { [weak self] in
                self?.constraints?.bottom?.constant = self?.additionalInputViewBottomConstraintConstant() ?? 0
                self?.inputAccessoryView?.superview?.layoutIfNeeded()
            }
        }
    }
    return self
}
fhernandezKt88 commented 1 year ago

I'm having big problems for this issue. How I can fix this if no configuration or something to disable this behavior at all ?. Please this component is heavily used by the community so try to fix this in a nice way. This is for the moment the unique problem I have with MessageKit/MessageInputBar !

@nathantannar4 Help please?

Janneman84 commented 1 year ago

Have you tried the code changes I suggested?

fhernandezKt88 commented 1 year ago

Have you tried the code changes I suggested?

I don't, because I can't touch KeyboardManager (I install this with SPM) so I can't. You don't have any other idea or solution here ? Thanks for the response.

Janneman84 commented 1 year ago

You can check out the repo then add it to your Xcode project as a local package. It will conveniently override the SPM one. Then you'll be able to make changes.

I also just added a pull request.

fhernandezKt88 commented 1 year ago

You can check out the repo then add it to your Xcode project as a local package. It will conveniently override the SPM one. Then you'll be able to make changes.

I also just added a pull request.

i think is best to fix the problem in the main package for all others. Thanks a loot for the pr. @nathantannar4 please can u merge this pr?

Kaspik commented 1 year ago

We can't just go and "merge it", but I did review it and requested some changes for simplification. Once it looks good, we will merge it.

fhernandezKt88 commented 1 year ago

We can't just go and "merge it", but I did review it and requested some changes for simplification. Once it looks good, we will merge it.

Thanks you @Kaspik !

fhernandezKt88 commented 1 year ago

@nathantannar4 @Kaspik some advance on this ?

nathantannar4 commented 1 year ago

For some context I've been away from this project. Id like to get back and do some cleanup. I started using this project in an app and noticed some issues. I think along the way some bad changes were merged leading to this issue, since it was a regression from earlier versions.

fhernandezKt88 commented 1 year ago

@Kaspik this is merged, thanks but, is the package updated ?

Kaspik commented 1 year ago

No, not yet. You can point SPM to the commit or branch.

fhernandezKt88 commented 1 year ago

No, not yet. You can point SPM to the commit or branch.

Yes, thanks.

fhernandezKt88 commented 1 year ago

Working fine, tested in iPhone 13 Pro (real device) with iOS 16.2. Nice work guys !

fhernandezKt88 commented 1 year ago

@Kaspik @nathantannar4 can this be updated with a new tag (6.3.0 for example) to be able to install the package without point to some branch or commit ?. Are something blocking this ?

Kaspik commented 1 year ago

I'll ship one later today / this week, sorry.

ctlamy commented 1 year ago

No, not yet. You can point SPM to the commit or branch.

@Janneman84 I tried installing commit 6d2f24396fd86924d5fe2fed7cfda4f9e932c363, for me InputBar initializes in incorrect position high up on screen but after you try and scroll it snaps down in correct position on top of keyboard.

Testing with MessageKit on iPhone14 Pro Max and also does on Simulator.

Am I using wrong commit?

https://github.com/nathantannar4/InputBarAccessoryView/assets/23286437/d3c39f2b-5c87-4cf2-84ea-259b908b57cb

Janneman84 commented 1 year ago

Is this on iOS 17 perhaps? Haven't tested that yet.

ctlamy commented 1 year ago

Is this on iOS 17 perhaps? Haven't tested that yet.

Ios 16.5.1

Janneman84 commented 1 year ago

Does checking out the master branche work? It works fine in my own app and the MessageKit example app.

Does it work right on pagesheets on iPad? If not you might just be on a wrong version somehow.

ctlamy commented 1 year ago

Same behavior with master branch.

But if I combine master branch with the hack from https://github.com/MessageKit/MessageKit/issues/1734#issuecomment-1221426408 (".ignoresSafeArea(.keyboard, edges: .bottom) on the SwiftUI side") then its effective. Oh well I guess.

@fhernandezKt88 you got it resolved on its own?

Janneman84 commented 1 year ago

@ctlamy You never mentioned you were using SwiftUI...

I tried the SwiftUI example and I get the issue too, but it was also broken without my fixes.

You're right that combining my fix with the .ignoresSafeArea thingy makes it work correctly again.

I made changes to the example app to test this, maybe I'll make a PR.

fhernandezKt88 commented 1 year ago

@ctlamy I'm using UIkit and this is solved with the PR: https://github.com/nathantannar4/InputBarAccessoryView/pull/244