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

fix(ios): avoid app scrolling to top on keyboard hide #533

Conversation

Lindsay-Needs-Sleep
Copy link
Contributor

Sister PR for 2.x: PR #532


The history of the keyboard dismissal problems are kind of messy so here we go:

1)

The main bug that started this all is reported here:

To summarize: Apple did something and broke a part of WKWebView for iOS 12 and 13. The problem occurs when the keyboard is dismissed. After dismissal the bottom part of the page has a white unused portion / the content behind the keyboard becomes unusable.

The issue is truly an apple problem. So this PR, and the previously accepted PR #201, should not be required if apple ever actually releases the fix. (The fix has been completed already apparently).
You can see the apple bug report links here.

2)

PR #179 and PR #201 attempt to fix this problem.

3)

Unfortunately, PR #201 introduces another a secondary bug, #399.
Now, when the keyboard is dismissed the screen scrolls all the way back to the top.
(Also reported in cordova-plugin-ionic-keyboard #84.)

4)

To fix the scroll to top issue we have PR #342. Unfortunately, this introduces another (less problematic bug): the view now "jumps" a little when moving between text fields.


Proposed Solution

Modify the changes from PR #201 using inspiriration from PR #342.

In KeyboardWillHide event:

  • Detect when the presence of the keyboard has allowed the user to scroll further than they would have been able to without the keyboard. (aka. If there will be whitespace at the bottom when the keyboard goes down)
  • If in the situation above, do some math to calculate the maximum allowed offset, and set the view to that. This causes the view to move into place properly. (So no empty whitespace or unusable portion of the view.)

Environment

I have been testing with:

  • Xcode 10.2

I have been testing on these devices:

  • iPad mini 2, iOS 12.4.5 (real)
  • iPhone 6s, iOS 13.3.1 (real)
  • iPhone X, iOS 12.2 (emulator)

And these projects:

modified ionic tutorial

  • cordova-plugin-ionic-webview 4.x
  • cordova-plugin-ionic-keyboard / NO cordova-plugin-ionic-keyboard

modified cordova helloWorld

  • NO cordova-plugin-ionic-keyboard

I have also tested this change with, and without, the changes in PR #531

I have also tested the 2.x sister PR #532 (which has identical changes basically) with this project:

cordova

  • cordova-plugin-ionic-webview 2.x
  • no keyboard plugins
  • Lots of other plugins that shouldn't matter

Summary

Fixes:

Compatible with PR #531.

Alternate Solution

Pros:

  • Less code muddling on our side (cleaner history)

Cons:

  • Apparently the plugin may result in a rejection

(tested this plugin on the ionic and cordova project for 4.x, and cordova for 2.x)

Since this plugin has already fixed this issue, I suppose it is probably better to just fix the fix at this point rather than this.

…#176) (ionic-team#399)

- We only need the KeyboardWillHide event to fix this problem
- Do some math to make sure the offset when the keyboard goes down doesn't result in empty white space being displayed.
- This solution can potentially be removed when apple finally releases the fix for the issue discussed here: apache/cordova-ios#417 (comment)
For full details see PR ionic-team#533
@Lindsay-Needs-Sleep Lindsay-Needs-Sleep changed the title (iOS) Fix issues related to keyboard dismissal Fix(iOS): Issues related to keyboard dismissal Mar 23, 2020
@Lindsay-Needs-Sleep Lindsay-Needs-Sleep changed the title Fix(iOS): Issues related to keyboard dismissal Fix(iOS): Issues related to keyboard dismissal (4.x) Mar 23, 2020
@imhoffd
Copy link
Contributor

imhoffd commented Mar 26, 2020

@Lindsay-Needs-Sleep It looks like iOS 13.4 fixes this issue?

@Lindsay-Needs-Sleep
Copy link
Contributor Author

@dwieeb I didn't notice the release, thanks!

I just updated to 13.4 and 12.4.6 on my devices and tried it out.

You are right, it looks like the issue is fixed on 13.4! (Really Apple, the day after I make my fix? :p)

But the issue still appears to exist on 12.4.6. (Also, "Really Apple?")

So this fix is still valid for 12.x (and probably 11.x). I'll add a commit to limit it.

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-ionic-webview that referenced this pull request Mar 27, 2020
@mattcwebster
Copy link

@Lindsay-Needs-Sleep I just downloaded IOS 13.4 and the scroll top bug still exists in my Ionic app consuming cordova-plugin-ionic-webview 4.1.3. It seems like some of the previous PRs addressing WkWebview IOS error are now doing more harm than good since IOS 13.4 fixes the keyboard dismissal bug.

Does anyone have a work around they're using to not see the hideous keyboard dismissal bug but aren't getting the equally as shitty scroll to the top bug.

@Lindsay-Needs-Sleep
Copy link
Contributor Author

Does anyone have a work around they're using to not see the hideous keyboard dismissal bug but aren't getting the equally as shitty scroll to the top bug.

That is exactly what this PR is. :S

Background:

  • ios 13.4 fixed the whitespace/unusable portion at the bottom on keyboard dismissal.
    • But this is still a problem on ios 11 - 13.3.x
  • The ‘scroll to top’ was a bug introduced solely in this plugin while implementing a workaround for the issue in the point above.

This PR modifies the fix that introduced the ‘scroll to top’ bug:

  • The new fix only applies to iOS less than 13.4
  • It uses a different solution for the original whitespace/unusable keyboard dismissal bug which is ‘scroll to top’ bug free.

But until this PR (or similar) is approved, if you are looking to not change any native code, I guess the best you can do is drop this plugin and go to cordova-plugin-wkwebview-engine.

@andaralex
Copy link

As recommended, shall I use this plugin cordova-plugin-wkkeyboardfix?
Is anyone used and publish their app in Apple Store? because they have given note as - plugin is a hack around a bug in iOS. It might result in your app being rejected from the App Store! Use this at your own risk.

@Lindsay-Needs-Sleep
Copy link
Contributor Author

@andaralex
Note: For the cordova-plugin-wkkeyboardfix to work, you can not use cordova-plugin-ionic-webview

@jcesarmobile jcesarmobile self-assigned this Apr 10, 2020
@jcesarmobile jcesarmobile changed the title Fix(iOS): Issues related to keyboard dismissal (4.x) fix(ios): avoid app scrolling to top on keyboard hide Apr 10, 2020
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@mattcwebster
Copy link

@Lindsay-Needs-Sleep I see this PR has been merged. Is it available for use yet, or does it need to go into a release?

@Lindsay-Needs-Sleep
Copy link
Contributor Author

@mattcwebster It depends on how you have added cordova-plugin-ionic-webview to your project.

Probably, you should remove the plugin and re-add it.

cordova plugin remove cordova-plugin-ionic-webview
cordova plugin add https://github.com/ionic-team/cordova-plugin-ionic-webview#master

This will include the changes made in this pull request.

Ionitron added a commit that referenced this pull request Apr 14, 2020
# [4.2.0](v4.1.3...v4.2.0) (2020-04-14)

### Bug Fixes

* **ionassethandler.m:** fix startPath is getting null ([#463](#463)) ([0bf16f1](0bf16f1))
* **ios:** avoid app scrolling to top on keyboard hide ([#533](#533)) ([7974eb4](7974eb4))
* **ios:** Replace deprecated APIs ([#539](#539)) ([27b9021](27b9021))

### Features

* **android:** proxy service worker requests through local server ([#452](#452)) ([c672175](c672175))
* **ios:** implement custom userAgent handling ([#537](#537)) ([8587114](8587114))
@Ionitron
Copy link
Collaborator

🎉 This PR is included in version 4.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mattcwebster
Copy link

@mattcwebster It depends on how you have added cordova-plugin-ionic-webview to your project.

Probably, you should remove the plugin and re-add it.

cordova plugin remove cordova-plugin-ionic-webview
cordova plugin add https://github.com/ionic-team/cordova-plugin-ionic-webview#master

This will include the changes made in this pull request.

I see this was included in release 4.2.0, however when upgrading to this version, I'm still getting the scroll issue. Not sure if there's something I'm missing?

@mattcwebster
Copy link

When using version 4.2.0 I'm getting both the scroll to top issue and the white-space on keyboard dismissal issue. The only way I can prevent the white-space keyboard dismissal is by adding <preference name="KeyboardResize" value="false" /> to my config.xml. The problem with adding that line is that after I dismiss the keyboard, my footer is now un-clickable.

@Lindsay-Needs-Sleep
Copy link
Contributor Author

Lindsay-Needs-Sleep commented Apr 18, 2020 via email

@mattcwebster
Copy link

@mattcwebster Can you tell me which version of iOS you are still seeing the issue on? (eg. iOS 11) And can you share the output of “cordova plugin” please

I'm using version 13.4

cordova-plugin-device 2.0.3 "Device"
cordova-plugin-inappbrowser 3.2.0 "InAppBrowser"
cordova-plugin-ionic-keyboard 2.2.0 "cordova-plugin-ionic-keyboard"
cordova-plugin-ionic-webview 4.2.0 "cordova-plugin-ionic-webview"
cordova-plugin-splashscreen 5.0.3 "Splashscreen"
cordova-plugin-statusbar 2.4.3 "StatusBar"
cordova-plugin-whitelist 1.3.4 "Whitelist"

@mattcwebster
Copy link

Actually, I may have solved my own problem but I don't know how. I dumped my node_modules folder and deleted my cordova-plugin-ionic-webview folder from within plugins. Did a fresh npm install, and prod build and it seems to be working! Coolio and sorry for the confusion.

Thanks again for your hard work and responsiveness on this one @Lindsay-Needs-Sleep

@Lindsay-Needs-Sleep
Copy link
Contributor Author

NP! Glad to hear it! :D

NatiSP pushed a commit to NatiSP/cordova-plugin-ionic-webview that referenced this pull request Jul 13, 2020
(cherry picked from commit 7974eb4)
(cherry picked from commit af642d8)
@Lindsay-Needs-Sleep Lindsay-Needs-Sleep deleted the fix_4x_keyboard-dismissal-view-shift-399-176 branch November 5, 2020 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants