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

Use document.documentElement.clientHeight instead of window.innerHeight #22

Merged
merged 2 commits into from
Dec 8, 2019
Merged

Use document.documentElement.clientHeight instead of window.innerHeight #22

merged 2 commits into from
Dec 8, 2019

Conversation

roman-kaspar
Copy link
Contributor

Hello Mikhail,

I like your library and am using it on my projects. However, recently I encountered this WebKit issue (WebKit runs PWAs on iOS): https://bugs.webkit.org/show_bug.cgi?id=170595

TL;DR version of the issue is that one cannot rely on window.innerHeight after rotating the phone back and forth (several times), as it has useless / bogus values. In the thread they recommend using document.documentElement.clientHeight instead.

I updated the getWindowHeight.js file like that (still having window.innerHeight as a fallback) and tested it on desktop, mobile Safari on iOS, and with my testing PWA in WebKit on iOS as well. Works nicely everywhere.

In this PR there are two commits, first one with the one-line change as described above, in the second commit I updated some dependencies and CHANGELOG.mg and bumped the version in case you decide to merge the PR.

PS: I checked the reported issues here as well, and believe it is the same problem, so perhaps this change might address them as well...

In case you'd like me to update this PR in any way, please let me know.

Cheers,
-Roman

@mvasin mvasin merged commit f7b8779 into mvasin:master Dec 8, 2019
@mvasin
Copy link
Owner

mvasin commented Dec 8, 2019

This PR closes #13, [email protected] is already live on npm. Thank you, Roman!

@mvasin mvasin mentioned this pull request Dec 8, 2019
@pbarbiero
Copy link

This, unfortunately breaks chrome when you hide the url bar. Chrome > 56 correctly updates innerHeight, but not clientHeight.

@mvasin
Copy link
Owner

mvasin commented Aug 22, 2020

Thank you for your comment, @pbarbiero! We will try to fix it in #30

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

Successfully merging this pull request may close these issues.

3 participants