Skip to content

Commit

Permalink
perf(picker): avoid flicker on ios (#29101)
Browse files Browse the repository at this point in the history
Issue number: N/A (see below)

---------

<!-- 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. -->

Mike found an issue while testing the Ionic 8 beta where there's a
flicker when presenting a picker in a modal on iOS. [We've seen this
issue
before](https://github.com/ionic-team/ionic-framework/blob/de13633a182d963876434db773aa346833f956fd/core/src/utils/forms/notch-controller.ts#L135-L144),
and the reason why it's happening is there's a quirk in WebKit where the
IntersectionObserver callback is fired _after_ an accelerated animation
completes given a particular IO configuration.

The end result is there's a delay before each picker column is scrolled
to the correct place.

In particular, the modal enter animation on iOS is an accelerated
animation, and we use an IO to control when the picker columns [should
scroll their active options into
view](https://github.com/ionic-team/ionic-framework/blob/60056643a99aea9817d2cd205fd011ab5967a995/core/src/components/picker-column/picker-column.tsx#L107).

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

- The root of the intersection observer is now the parent picker element
which avoids the WebKit quirk.

| `feature-8.0` | branch |
| - | - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/7478512b-a691-405e-aafa-e9e377fc47ac"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/5092e050-733a-4965-8f16-317d251af46a"></video>
|

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## 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: `8.0.0-dev.11709226349.179192de`

Note: Given that this bug is due to a WebKit quirk and is
timing-related, I did not add a test.
  • Loading branch information
liamdebeasi committed Mar 6, 2024
1 parent bf1701e commit 94c3ffc
Showing 1 changed file with 19 additions and 2 deletions.
21 changes: 19 additions & 2 deletions core/src/components/picker-column/picker-column.tsx
Expand Up @@ -87,13 +87,22 @@ export class PickerColumn implements ComponentInterface {
* height of 0px.
*/
componentWillLoad() {
/**
* We cache parentEl in a local variable
* so we don't need to keep accessing
* the class variable (which comes with
* a small performance hit)
*/
const parentEl = (this.parentEl = this.el.closest('ion-picker') as HTMLIonPickerElement | null);

const visibleCallback = (entries: IntersectionObserverEntry[]) => {
const ev = entries[0];

if (ev.isIntersecting) {
const { activeItem, el } = this;

this.isColumnVisible = true;

/**
* Because this initial call to scrollActiveItemIntoView has to fire before
* the scroll listener is set up, we need to manage the active class manually.
Expand All @@ -119,9 +128,17 @@ export class PickerColumn implements ComponentInterface {
}
}
};
new IntersectionObserver(visibleCallback, { threshold: 0.001 }).observe(this.el);
/**
* Set the root to be the parent picker element
* This causes the IO callback
* to be fired in WebKit as soon as the element
* is visible. If we used the default root value
* then WebKit would only fire the IO callback
* after any animations (such as a modal transition)
* finished, and there would potentially be a flicker.
*/
new IntersectionObserver(visibleCallback, { threshold: 0.001, root: this.parentEl }).observe(this.el);

const parentEl = (this.parentEl = this.el.closest('ion-picker') as HTMLIonPickerElement | null);
if (parentEl !== null) {
// TODO(FW-2832): type
parentEl.addEventListener('ionInputModeChange', (ev: any) => this.inputModeChange(ev));
Expand Down

0 comments on commit 94c3ffc

Please sign in to comment.