Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScrollView contentOffset not adjusted correctly when form is displayed as page sheet and keyboard is displayed #1952

Open
funkenstrahlen opened this issue Dec 12, 2019 · 11 comments

Comments

@funkenstrahlen
Copy link

I discovered an issue with adjusting the form ScrollView contentInset and contentOffset when the user edits text in a TextAreaRow. This becomes a problem as soon as the form is presented as an page sheet on iOS 13. It looks like the scrolling does not consider the additional top space added by the new page sheet presentation style.

2019-12-12 21-31-01 2019-12-12 21_31_58

This only happens when the view is displayed with the page sheet style (space at the top).

Also the behavior is not consistent as you can see in this clip:

2019-12-12 21-37-07 2019-12-12 21_37_46

The same code on iOS 12 without the page sheet presentation style: Also not consistent but does not scroll badly so the first line is invisible:

2019-12-12 21-40-40 2019-12-12 21_41_07

I guess this is the code that needs some update to fix this:

Eureka/Source/Core/Core.swift

Lines 1009 to 1037 in 2924259

@objc open func keyboardWillShow(_ notification: Notification) {
guard let table = tableView, let cell = table.findFirstResponder()?.formCell() else { return }
let keyBoardInfo = notification.userInfo!
let endFrame = keyBoardInfo[UIResponder.keyboardFrameEndUserInfoKey] as! NSValue
let keyBoardFrame = table.window!.convert(endFrame.cgRectValue, to: table.superview)
let newBottomInset = table.frame.origin.y + table.frame.size.height - keyBoardFrame.origin.y + rowKeyboardSpacing
var tableInsets = table.contentInset
var scrollIndicatorInsets = table.scrollIndicatorInsets
oldBottomInset = oldBottomInset ?? tableInsets.bottom
if newBottomInset > oldBottomInset! {
tableInsets.bottom = newBottomInset
scrollIndicatorInsets.bottom = tableInsets.bottom
UIView.beginAnimations(nil, context: nil)
UIView.setAnimationDuration((keyBoardInfo[UIResponder.keyboardAnimationDurationUserInfoKey] as! Double))
UIView.setAnimationCurve(UIView.AnimationCurve(rawValue: (keyBoardInfo[UIResponder.keyboardAnimationCurveUserInfoKey] as! Int))!)
table.contentInset = tableInsets
table.scrollIndicatorInsets = scrollIndicatorInsets
if let selectedRow = table.indexPath(for: cell) {
if ProcessInfo.processInfo.operatingSystemVersion.majorVersion == 11 {
let rect = table.rectForRow(at: selectedRow)
table.scrollRectToVisible(rect, animated: animateScroll)
} else {
table.scrollToRow(at: selectedRow, at: .none, animated: animateScroll)
}
}
UIView.commitAnimations()
}
}

I tried to figure it out myself, but could not find a solution yet.

Environment: Eureka 5.1.0, Xcode 11.3

@funkenstrahlen
Copy link
Author

This might be related to this PR: #1833

However I do not use any Tabbar in my code.

@funkenstrahlen
Copy link
Author

Probably also related to #1938.

@funkenstrahlen
Copy link
Author

I did some more debugging and realized this issue is caused by table.scrollToRow(at: selectedRow, at: .none, animated: animateScroll). It does not respect the additional safeAreaInsets on top added by the page sheet presentation.

Seems like an iOS 13 bug even though I am already running on iOS 13.3.

@funkenstrahlen
Copy link
Author

I found a workaround:

            if let selectedRow = table.indexPath(for: cell) {
                var rect = table.rectForRow(at: selectedRow)
                if #available(iOSApplicationExtension 11.0, *) {
                    rect.origin.y -= table.safeAreaInsets.top
                }
                table.scrollRectToVisible(rect, animated: animateScroll)
            }

@funkenstrahlen
Copy link
Author

I created a PR: #1953

@mats-claassen
Copy link
Member

Hi @funkenstrahlen, thanks for reporting and providing a pull request. The way I understand it is an issue with scrollToRow which should be fixed in a future release, right?

Can I ask why you use iOSApplicationExtension in the #available? Also, as this issue only seems to happen in iOS 13 with the page sheet presentation, does this code change work on iOS 12 for example?

@funkenstrahlen
Copy link
Author

funkenstrahlen commented Dec 16, 2019

The way I understand it is an issue with scrollToRow which should be fixed in a future release, right?

Yes I hope Apple will fix this in a Future Release of iOS 13. I have not found time to report this to Apple yet.

does this code change work on iOS 12 for example?

As far as my testing goes it does work on iOS 12 too.

Can I ask why you use iOSApplicationExtension in the #available?

You are right. Using application extension check here is wrong. I did not give it a closer look as Xcode added this when clicking on auto-fix. If you want I can adjust the PR to fix that.

@yunhao
Copy link
Contributor

yunhao commented Dec 26, 2019

Similiar to #1959.
The issue is caused by FormViewController because it does not handle safe area properly. The example app should be update, it doesn't event use any safe area api.

@philipbel
Copy link

I am seeing this on iOS 14.4, gif attached. Eureka already has #1953 applied.

Eureka FormSheet Keyboard Top Row

philipbel added a commit to philipbel/Eureka that referenced this issue Mar 27, 2021
@philipbel
Copy link

I did some digging.
If I use autolayout to pin the table view to the safe area of the presented (as a form sheet) view controller, then everything works just fine—the scrolling is OK.

I have also tried playing with ProcessInfo.processInfo.operatingSystemVersion.majorVersion in keyboardWillShow(), to no avail. Adjusting newBottomInset by the top of safeAreaInsets didn't help either. So, the bug really seems to be in iOS itself when autoresizing masks are used for table view scrolling.

I have created a PR #2148. Please let me know what you think.

philipbel added a commit to philipbel/Eureka that referenced this issue Mar 29, 2021
@mats-claassen
Copy link
Member

@philipbel I cannot reproduce this with the example app. There might be something else in your code provoking that issue. It would be helpful to have a small code snippet to reproduce that issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants