-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Mobile Web Safari: Adds auto scroll back - Issue 5894 #6413
Changes from all commits
e04c6a7
038536a
df1f19a
92af417
4c38171
3534799
ec6cd6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
/* The autoScrollBack address Mobile Safari-specific issues when the user overscrolls the window while the keyboard is visible */ | ||
/* It has no effect to other platforms */ | ||
export default function () { } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
/* The autoScrollBack address Mobile Safari-specific issues when the user overscrolls the window while the keyboard is visible */ | ||
import Str from 'expensify-common/lib/str'; | ||
import smoothscrollPolyfill from 'smoothscroll-polyfill'; | ||
import CONST from '../../CONST'; | ||
import getBrowser from '../getBrowser'; | ||
|
||
const userAgent = navigator.userAgent.toLowerCase(); | ||
|
||
// The innerHeight when the keyboard is not visible | ||
const baseInnerHeight = window.innerHeight; | ||
|
||
// Control flag if an input/text area was focused and we're waiting the "focus scroll" to identify the screen size | ||
let isWaitingForScroll = false; | ||
|
||
// Calculated value of the maximum value to the screen to scroll when keyboard is visible | ||
let maxScrollY = 0; | ||
|
||
let isTouching = false; | ||
|
||
let scrollbackTimeout; | ||
|
||
const isIOS15 = Str.contains(userAgent, 'iphone os 15_'); | ||
|
||
function scrollback() { | ||
if (isTouching || maxScrollY >= window.scrollY) { | ||
return; | ||
} | ||
window.scrollTo({top: maxScrollY, behavior: 'smooth'}); | ||
} | ||
|
||
function scheduleScrollback() { | ||
if (!maxScrollY) { | ||
return; | ||
} | ||
|
||
if (scrollbackTimeout) { | ||
clearTimeout(scrollbackTimeout); | ||
scrollbackTimeout = undefined; | ||
} | ||
|
||
if (!isTouching && window.scrollY > maxScrollY) { | ||
scrollbackTimeout = setTimeout(scrollback, 34); | ||
} | ||
} | ||
|
||
|
||
function touchStarted() { | ||
isTouching = true; | ||
} | ||
|
||
function scrollbackAfterTouch() { | ||
isTouching = false; | ||
scrollback(); | ||
} | ||
|
||
function scrollbackAfterScroll() { | ||
if (isWaitingForScroll && !maxScrollY) { | ||
isWaitingForScroll = false; | ||
const keyboardHeight = baseInnerHeight - window.visualViewport.height; | ||
|
||
// The iOS 15 Safari has a 52 pixel tall address label that must be manually added | ||
maxScrollY = keyboardHeight + (isIOS15 ? 52 : 0); | ||
} | ||
|
||
scheduleScrollback(); | ||
} | ||
|
||
function startWaitingForScroll() { | ||
isWaitingForScroll = true; | ||
} | ||
|
||
function stopWaitingForScroll() { | ||
isWaitingForScroll = false; | ||
} | ||
|
||
|
||
export default function () { | ||
if (!getBrowser() === CONST.BROWSER.SAFARI || !Str.contains(userAgent, 'iphone os 1')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this supposed to be getBrowser() !== CONST.BROWSER.SAFARI
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WOW. It should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we want to exist early if it regular safari, and only apply the polyfill for mobile ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an early There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IT should only work on Safari web. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OMG 🤦♂️ how I did that?! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kidroca if possible, load that file only for Website + Safari There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey I'm slightly confused I thought the issue is "Mobile Web Safari: Adds auto scroll back" My plan is to use this expression if (_.every([/iP(ad|hone)/i, /WebKit/i, /CriOS/i], re => re.test(navigator.userAgent))) {
const applyAutoScrollbackFix = require('../../libs/iOSSafariAutoScrollback');
applyAutoScrollbackFix();
} The check is based on: https://stackoverflow.com/a/29696509/4834103 I can extract const isMobileSafari = () => _.every([/iP(ad|hone)/i, /WebKit/i, /CriOS/i], re => re.test(navigator.userAgent)); But I don't know where to put it |
||
return; | ||
} | ||
smoothscrollPolyfill.polyfill(); | ||
document.addEventListener('touchstart', touchStarted); | ||
document.addEventListener('touchend', scrollbackAfterTouch); | ||
document.addEventListener('scroll', scrollbackAfterScroll); | ||
document.addEventListener('focusin', startWaitingForScroll); | ||
document.addEventListener('focusout', stopWaitingForScroll); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is something that's only intended to run on web, maybe you should consider importing and calling it only inside
src/setup/platformSetup/index.website.js
I'll would even go as far as
This way extra code is imported only when needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kidroca it is merged already... but that's a GREAT suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion @kidroca! If either of you wouldn't mind submitting a quick PR, I'll review it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks guys!
@roryabraham I'll open a PR about this today, when I get the chance