From 578981b2d0b7219db9b30419e70511b47fa904ea Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 12 Sep 2023 16:00:53 -0400 Subject: [PATCH 1/6] fix(scroll-assist): improve input scroll accuracy --- .../utils/input-shims/hacks/scroll-assist.ts | 32 +++++++++++++++++-- .../utils/input-shims/hacks/scroll-data.ts | 9 ++++-- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/core/src/utils/input-shims/hacks/scroll-assist.ts b/core/src/utils/input-shims/hacks/scroll-assist.ts index 97928d608ec..400b201f094 100644 --- a/core/src/utils/input-shims/hacks/scroll-assist.ts +++ b/core/src/utils/input-shims/hacks/scroll-assist.ts @@ -1,4 +1,5 @@ import type { KeyboardResizeOptions } from '@capacitor/keyboard'; +import { win } from '@utils/browser'; import { getScrollElement, scrollByPoint } from '../../content'; import { raf } from '../../helpers'; @@ -34,6 +35,21 @@ export const enableScrollAssist = ( const addScrollPadding = enableScrollPadding && (keyboardResize === undefined || keyboardResize.mode === KeyboardResize.None); + /** + * When adding scroll padding we need to know + * how much of the viewport the keyboard obscures. + * We do this by subtracting the keyboard height + * from the platform height. + * + * If we compute this value when switching between + * inputs then the webview may already be resized. + * At this point, `win.innerHeight` has already accounted + * for the keyboard meaning we would then subtract + * the keyboard height again. This will result in the input + * being scrolled more than it needs to. + */ + const platformHeight = win !== undefined ? win.innerHeight : 0; + /** * When the input is about to receive * focus, we need to move it to prevent @@ -50,7 +66,16 @@ export const enableScrollAssist = ( inputEl.removeAttribute(SKIP_SCROLL_ASSIST); return; } - jsSetFocus(componentEl, inputEl, contentEl, footerEl, keyboardHeight, addScrollPadding, disableClonedInput); + jsSetFocus( + componentEl, + inputEl, + contentEl, + footerEl, + keyboardHeight, + addScrollPadding, + disableClonedInput, + platformHeight + ); }; componentEl.addEventListener('focusin', focusIn, true); @@ -84,12 +109,13 @@ const jsSetFocus = async ( footerEl: HTMLIonFooterElement | null, keyboardHeight: number, enableScrollPadding: boolean, - disableClonedInput = false + disableClonedInput = false, + platformHeight = 0 ) => { if (!contentEl && !footerEl) { return; } - const scrollData = getScrollData(componentEl, (contentEl || footerEl)!, keyboardHeight); + const scrollData = getScrollData(componentEl, (contentEl || footerEl)!, keyboardHeight, platformHeight); if (contentEl && Math.abs(scrollData.scrollAmount) < 4) { // the text input is in a safe position that doesn't diff --git a/core/src/utils/input-shims/hacks/scroll-data.ts b/core/src/utils/input-shims/hacks/scroll-data.ts index 48b4a12007d..cc82553b559 100644 --- a/core/src/utils/input-shims/hacks/scroll-data.ts +++ b/core/src/utils/input-shims/hacks/scroll-data.ts @@ -9,13 +9,18 @@ export interface ScrollData { inputSafeY: number; } -export const getScrollData = (componentEl: HTMLElement, contentEl: HTMLElement, keyboardHeight: number): ScrollData => { +export const getScrollData = ( + componentEl: HTMLElement, + contentEl: HTMLElement, + keyboardHeight: number, + platformHeight: number +): ScrollData => { const itemEl = (componentEl.closest('ion-item,[ion-item]') as HTMLElement) ?? componentEl; return calcScrollData( itemEl.getBoundingClientRect(), contentEl.getBoundingClientRect(), keyboardHeight, - (componentEl as any).ownerDocument.defaultView.innerHeight // TODO(FW-2832): type + platformHeight ); }; From 1a99036fd124de3856b2c4d301f8fdc4772b0767 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 13 Sep 2023 16:58:39 -0400 Subject: [PATCH 2/6] fix(scroll-assist): run when keyboard dimension changes --- .../utils/input-shims/hacks/scroll-assist.ts | 88 ++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/core/src/utils/input-shims/hacks/scroll-assist.ts b/core/src/utils/input-shims/hacks/scroll-assist.ts index 400b201f094..d24ceacd83f 100644 --- a/core/src/utils/input-shims/hacks/scroll-assist.ts +++ b/core/src/utils/input-shims/hacks/scroll-assist.ts @@ -35,6 +35,15 @@ export const enableScrollAssist = ( const addScrollPadding = enableScrollPadding && (keyboardResize === undefined || keyboardResize.mode === KeyboardResize.None); + /** + * This tracks whether or not the keyboard has been + * presented for a single focused text field. Note + * that it does not track if the keyboard is open + * in general such as if the keyboard is open for + * a different focused text field. + */ + let hasKeyboardBeenPresentedForTextField = false; + /** * When adding scroll padding we need to know * how much of the viewport the keyboard obscures. @@ -50,6 +59,74 @@ export const enableScrollAssist = ( */ const platformHeight = win !== undefined ? win.innerHeight : 0; + /** + * Scroll assist is run when a text field + * is focused. However, it may need to + * re-run when the keyboard size changes + * such that the text field is now hidden + * underneath the keyboard. + * This function re-runs scroll assist + * when that happens. + * + * One limitation of this is on a web browser + * where native keyboard APIs do not have cross-browser + * support. `ionKeyboardDidShow` relies on the Visual Viewport API. + * This means that if the keyboard changes but does not change + * geometry, then scroll assist will not re-run even if + * the user has scrolled the text field under the keyboard. + * This should not be a problem when running in Cordova/Capacitor + * because `ionKeyboardDidShow` uses the native events + * which fire every time the keyboard changes. + */ + const keyboardShow = (ev: any) => { + /** + * If the keyboard has not yet been presented + * for this text field then the text field has just + * received focus. In that case, the focusin listener + * will run scroll assist. + */ + if (hasKeyboardBeenPresentedForTextField === false) { + hasKeyboardBeenPresentedForTextField = true; + return; + } + + /** + * Otherwise, the keyboard has already been presented + * for the focused text field. + * This means that the keyboard likely changed + * geometry, and we need to re-run scroll assist. + * This can happen when the user rotates their device + * or when they switch keyboards. + * + * Make sure we pass in the computed keyboard height + * rather than the estimated keyboard height. + * + * Since the keyboard is already open then we do not + * need to wait for the webview to resize, so we pass + * "waitForResize: false". + */ + jsSetFocus( + componentEl, + inputEl, + contentEl, + footerEl, + ev.detail.keyboardHeight, + addScrollPadding, + disableClonedInput, + platformHeight, + false + ); + }; + + /** + * Reset the internal state when the text field loses focus. + */ + const focusOut = () => { + hasKeyboardBeenPresentedForTextField = false; + window.removeEventListener('ionKeyboardDidShow', keyboardShow); + componentEl.removeEventListener('focusout', focusOut, true); + }; + /** * When the input is about to receive * focus, we need to move it to prevent @@ -76,11 +153,17 @@ export const enableScrollAssist = ( disableClonedInput, platformHeight ); + + window.addEventListener('ionKeyboardDidShow', keyboardShow); + componentEl.addEventListener('focusout', focusOut, true); }; + componentEl.addEventListener('focusin', focusIn, true); return () => { componentEl.removeEventListener('focusin', focusIn, true); + window.removeEventListener('ionKeyboardDidShow', keyboardShow); + componentEl.removeEventListener('focusout', focusOut, true); }; }; @@ -110,7 +193,8 @@ const jsSetFocus = async ( keyboardHeight: number, enableScrollPadding: boolean, disableClonedInput = false, - platformHeight = 0 + platformHeight = 0, + waitForResize = true ) => { if (!contentEl && !footerEl) { return; @@ -217,7 +301,7 @@ const jsSetFocus = async ( * bandwidth to become available. */ const totalScrollAmount = scrollEl.scrollHeight - scrollEl.clientHeight; - if (scrollData.scrollAmount > totalScrollAmount - scrollEl.scrollTop) { + if (waitForResize && scrollData.scrollAmount > totalScrollAmount - scrollEl.scrollTop) { /** * On iOS devices, the system will show a "Passwords" bar above the keyboard * after the initial keyboard is shown. This prevents the webview from resizing From f663b41644c787c9ba3f43314111b25c2c7bffc2 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 13 Sep 2023 17:12:47 -0400 Subject: [PATCH 3/6] add types --- core/src/utils/input-shims/hacks/scroll-assist.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/src/utils/input-shims/hacks/scroll-assist.ts b/core/src/utils/input-shims/hacks/scroll-assist.ts index d24ceacd83f..93d84d80902 100644 --- a/core/src/utils/input-shims/hacks/scroll-assist.ts +++ b/core/src/utils/input-shims/hacks/scroll-assist.ts @@ -13,6 +13,18 @@ let currentPadding = 0; const SKIP_SCROLL_ASSIST = 'data-ionic-skip-scroll-assist'; +/** + * addEventListener typically expects an Event + * type in the event listener callbacks. In this case + * we have a CustomEvent, so we update the window event map + * to register our specific event as well as the type of that event. + */ +declare global { + interface WindowEventMap { + ionKeyboardDidShow: CustomEvent<{ keyboardHeight: number }>; + } +} + export const enableScrollAssist = ( componentEl: HTMLElement, inputEl: HTMLInputElement | HTMLTextAreaElement, @@ -78,7 +90,7 @@ export const enableScrollAssist = ( * because `ionKeyboardDidShow` uses the native events * which fire every time the keyboard changes. */ - const keyboardShow = (ev: any) => { + const keyboardShow = (ev: CustomEvent<{ keyboardHeight: number }>) => { /** * If the keyboard has not yet been presented * for this text field then the text field has just From 504c90beaec8e09753e5306c6978f30eac72e646 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 14 Sep 2023 09:33:28 -0400 Subject: [PATCH 4/6] update types --- core/src/utils/browser/index.ts | 22 ++++++++++++++++++- .../utils/input-shims/hacks/scroll-assist.ts | 18 +++------------ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/core/src/utils/browser/index.ts b/core/src/utils/browser/index.ts index 5b3e914a45f..ed70843de12 100644 --- a/core/src/utils/browser/index.ts +++ b/core/src/utils/browser/index.ts @@ -20,6 +20,26 @@ * Note: Code inside of this if-block will * not run in an SSR environment. */ -export const win: Window | undefined = typeof window !== 'undefined' ? window : undefined; + +/** + * Even listeners on the window typically expect + * Event types for the listener parameter. If you want to listen + * on the window for certain CustomEvent types you can add that definition + * here as long as you are using the "win" utility below. + */ +type IonicWindow = Window & { + addEventListener( + type: 'ionKeyboardDidShow', + listener: (ev: CustomEvent<{ keyboardHeight: number }>) => void, + options?: boolean | AddEventListenerOptions + ): void; + removeEventListener( + type: 'ionKeyboardDidShow', + listener: (ev: CustomEvent<{ keyboardHeight: number }>) => void, + options?: boolean | AddEventListenerOptions + ): void; +}; + +export const win: IonicWindow | undefined = typeof window !== 'undefined' ? window : undefined; export const doc: Document | undefined = typeof document !== 'undefined' ? document : undefined; diff --git a/core/src/utils/input-shims/hacks/scroll-assist.ts b/core/src/utils/input-shims/hacks/scroll-assist.ts index 93d84d80902..1b35367ce9a 100644 --- a/core/src/utils/input-shims/hacks/scroll-assist.ts +++ b/core/src/utils/input-shims/hacks/scroll-assist.ts @@ -13,18 +13,6 @@ let currentPadding = 0; const SKIP_SCROLL_ASSIST = 'data-ionic-skip-scroll-assist'; -/** - * addEventListener typically expects an Event - * type in the event listener callbacks. In this case - * we have a CustomEvent, so we update the window event map - * to register our specific event as well as the type of that event. - */ -declare global { - interface WindowEventMap { - ionKeyboardDidShow: CustomEvent<{ keyboardHeight: number }>; - } -} - export const enableScrollAssist = ( componentEl: HTMLElement, inputEl: HTMLInputElement | HTMLTextAreaElement, @@ -135,7 +123,7 @@ export const enableScrollAssist = ( */ const focusOut = () => { hasKeyboardBeenPresentedForTextField = false; - window.removeEventListener('ionKeyboardDidShow', keyboardShow); + win?.removeEventListener('ionKeyboardDidShow', keyboardShow); componentEl.removeEventListener('focusout', focusOut, true); }; @@ -166,7 +154,7 @@ export const enableScrollAssist = ( platformHeight ); - window.addEventListener('ionKeyboardDidShow', keyboardShow); + win?.addEventListener('ionKeyboardDidShow', keyboardShow); componentEl.addEventListener('focusout', focusOut, true); }; @@ -174,7 +162,7 @@ export const enableScrollAssist = ( return () => { componentEl.removeEventListener('focusin', focusIn, true); - window.removeEventListener('ionKeyboardDidShow', keyboardShow); + win?.removeEventListener('ionKeyboardDidShow', keyboardShow); componentEl.removeEventListener('focusout', focusOut, true); }; }; From 12cae824b189e847ab15ec0525637aa90d922ea1 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 14 Sep 2023 09:36:14 -0400 Subject: [PATCH 5/6] typo --- core/src/utils/input-shims/hacks/scroll-assist.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/utils/input-shims/hacks/scroll-assist.ts b/core/src/utils/input-shims/hacks/scroll-assist.ts index 1b35367ce9a..01f810f3295 100644 --- a/core/src/utils/input-shims/hacks/scroll-assist.ts +++ b/core/src/utils/input-shims/hacks/scroll-assist.ts @@ -74,7 +74,7 @@ export const enableScrollAssist = ( * This means that if the keyboard changes but does not change * geometry, then scroll assist will not re-run even if * the user has scrolled the text field under the keyboard. - * This should not be a problem when running in Cordova/Capacitor + * This is not a problem when running in Cordova/Capacitor * because `ionKeyboardDidShow` uses the native events * which fire every time the keyboard changes. */ From 34e4185418a8eb9343dcf6656d76299593c6ba14 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 19 Sep 2023 10:48:56 -0400 Subject: [PATCH 6/6] Update core/src/utils/browser/index.ts Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com> --- core/src/utils/browser/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/utils/browser/index.ts b/core/src/utils/browser/index.ts index ed70843de12..5013b9f2c9f 100644 --- a/core/src/utils/browser/index.ts +++ b/core/src/utils/browser/index.ts @@ -22,7 +22,7 @@ */ /** - * Even listeners on the window typically expect + * Event listeners on the window typically expect * Event types for the listener parameter. If you want to listen * on the window for certain CustomEvent types you can add that definition * here as long as you are using the "win" utility below.