-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: overlay layout shift when auto-focusing input #10102
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
13eafc2
64a2d76
8bf7085
2ed61d1
522b02e
c9fbf32
c93b460
9bcbdf4
2d6fc45
70a2394
9ac2c05
aef324f
2d2649d
77c36bf
f20c922
d0638df
47bc548
d492b1b
3bcc076
04cb377
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,11 +11,10 @@ | |
| */ | ||
|
|
||
| import {chain} from '../utils/chain'; | ||
|
|
||
| import {getActiveElement, getEventTarget} from '../utils/shadowdom/DOMFunctions'; | ||
| import {getNonce} from '../utils/getNonce'; | ||
| import {getScrollParent} from '../utils/getScrollParent'; | ||
| import {isIOS} from '../utils/platform'; | ||
| import {isIOS, isWebKit} from '../utils/platform'; | ||
| import {isScrollable} from '../utils/isScrollable'; | ||
| import {useLayoutEffect} from '../utils/useLayoutEffect'; | ||
| import {willOpenKeyboard} from '../utils/keyboard'; | ||
|
|
@@ -46,7 +45,7 @@ export function usePreventScroll(options: PreventScrollOptions = {}): void { | |
|
|
||
| preventScrollCount++; | ||
| if (preventScrollCount === 1) { | ||
| if (isIOS()) { | ||
| if (isIOS() && isWebKit()) { | ||
|
Contributor
Author
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. Regressed in #1514. Not sure if lucky accident though? Will need testing.
Member
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 to confirm, the changed flow is now that we need to test iOS Chrome and iOS Firefox as they'd be the two which used to go through here and no longer do?
Contributor
Author
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. Correct. I did that, and havent found any regressions, but I would feel better with an additional test session. Note I did find a pre-existing bug that window scroll is possible when pinch zooming outwards (some browser must first be pinched inwards to then pinch outwards) and down, aka two finger scrolling. Outwards pinch in general is pretty funky - even on Safari Desktop. |
||
| restore = preventScrollMobileSafari(); | ||
| } else { | ||
| restore = preventScrollStandard(); | ||
|
|
@@ -197,18 +196,22 @@ function preventScrollMobileSafari() { | |
|
|
||
| // Override programmatic focus to scroll into view without scrolling the whole page. | ||
| let focus = HTMLElement.prototype.focus; | ||
| HTMLElement.prototype.focus = function (opts) { | ||
| // Track whether the keyboard was already visible before. | ||
| let activeElement = getActiveElement(); | ||
| let wasKeyboardVisible = activeElement != null && willOpenKeyboard(activeElement); | ||
|
|
||
| // Focus the element without scrolling the page. | ||
| focus.call(this, {...opts, preventScroll: true}); | ||
|
|
||
| if (!opts || !opts.preventScroll) { | ||
| scrollIntoViewWhenReady(this, wasKeyboardVisible); | ||
| Reflect.defineProperty(HTMLElement.prototype, 'focus', { | ||
|
nwidynski marked this conversation as resolved.
|
||
| configurable: true, | ||
| writable: true, | ||
| value: function (opts?: FocusOptions) { | ||
| // Track whether the keyboard was already visible before. | ||
| let activeElement = getActiveElement(); | ||
| let wasKeyboardVisible = activeElement != null && willOpenKeyboard(activeElement); | ||
|
|
||
| // Focus the element without scrolling the page. | ||
| focus.call(this, {...opts, preventScroll: true}); | ||
|
|
||
| if (!opts || !opts.preventScroll) { | ||
| scrollIntoViewWhenReady(this, wasKeyboardVisible); | ||
| } | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| let removeEvents = chain( | ||
| addEvent(document, 'touchstart', onTouchStart, {passive: false, capture: true}), | ||
|
|
@@ -220,7 +223,11 @@ function preventScrollMobileSafari() { | |
| restoreOverflow(); | ||
| removeEvents(); | ||
| style.remove(); | ||
| HTMLElement.prototype.focus = focus; | ||
| Reflect.defineProperty(HTMLElement.prototype, 'focus', { | ||
| configurable: true, | ||
| writable: true, | ||
| value: focus | ||
| }); | ||
| }; | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.