Skip to content

Commit

Permalink
fix(scroll-assist): improve input scroll accuracy with native resizing (
Browse files Browse the repository at this point in the history
#28169)

Issue number: Part of #22940

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

While working on a fix for #22940, I discovered another bug that
impacted the reliability of my proposed fix for #22940. When we compute
the scroll data (i.e. how much the input needs to be scrolled by), we
subtract the `keyboardHeight` from the `platformHeight` (i.e. the
viewport height):
https://github.com/ionic-team/ionic-framework/blob/1015c06cbef4ceb10d43e722157c04844d984509/core/src/utils/input-shims/hacks/scroll-data.ts#L34

Every time we tap between inputs (even if the keyboard is already open)
we re-run scroll assist because the newly focused input could be
partially obscured by the keyboard. However, in this case we scroll by
too much because we effectively subtract the keyboard height twice. This
is because by the time we compute `platformHeight`, the platform
dimensions have already shrunk to account for the keyboard (when the
webview resizes).

As a result, when we subtract `keyboardHeight` we get a visible area
that is much smaller than the actual visible area.

Examples below with different resize modes. Notice that with the
"Native" resize mode (entire webview resizes when keyboard is open)
tapping into other inputs scrolls the content by much more than it needs
to.

| Body | Native | Ionic | None |
| - | - | - | - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/06d1cd20-0349-4a59-ad85-c1c8a8a03caa"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/1d4e8363-a69b-45c4-931c-d6227e548ec9"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/7e4304c1-7d56-48c8-aed8-16fc7e51641a"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/7869c5e0-b202-46e1-af82-49e41b3b067e"></video>
|

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- We now compute the viewport height on load rather than on focus. This
ensures that we always use the full viewport height when computing the
visible area.

| Body | Native | Ionic | None |
| - | - | - | - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/c5a66287-0cad-42db-bece-da16edad60e3"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/372a45c8-e8bd-43d2-bf50-d87b7250e9b3"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/3d656467-8e2e-48cc-8d72-dc89a67ef8b1"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/19969535-7d06-404c-98e4-ae49957e0ffe"></video>
|



## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.3.4-dev.11694548895.1578981b`
  • Loading branch information
liamdebeasi authored Sep 19, 2023
1 parent 81714d4 commit b5c736f
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
32 changes: 29 additions & 3 deletions core/src/utils/input-shims/hacks/scroll-assist.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { KeyboardResizeOptions } from '@capacitor/keyboard';
import { win } from '@utils/browser';

import { getScrollElement, scrollByPoint } from '../../content';
import { raf } from '../../helpers';
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions core/src/utils/input-shims/hacks/scroll-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
};

Expand Down

0 comments on commit b5c736f

Please sign in to comment.