Skip to content

Commit 9d781db

Browse files
fix(datetime): ensure datetime is shown when intersection observer fails to report visibility (#30793)
Issue number: resolves #30706 --------- <!-- 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? Due to some recent unknown changes, the intersection observer for date time no longer reliably fires, especially in mobile views. ## What is the new behavior? In this PR, we're adding a visibility check after everything has had a chance to render to make sure we're setting up properly even if the intersection observer has failed to trigger for some reason. ## Does this introduce a breaking change? - [ ] Yes - [X] No ## Other information Since the intersection observer is being set up after a `raf`, it's possible something got introduced to make the initial setup slower for some reason, causing timing issues. I think we should do a more thorough investigation into the cause of this problem when we have more time. This PR also adds tests to verify the new fallback works properly. Current dev build: ``` 8.7.10-dev.11763478209.1d9c4cd8 ``` --------- Co-authored-by: Brandy Smith <[email protected]>
1 parent 9ae41ef commit 9d781db

File tree

3 files changed

+124
-2
lines changed

3 files changed

+124
-2
lines changed

core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { expect } from '@playwright/test';
21
import type { Locator } from '@playwright/test';
3-
import { configs, test } from '@utils/test/playwright';
2+
import { expect } from '@playwright/test';
43
import type { EventSpy } from '@utils/test/playwright';
4+
import { configs, test } from '@utils/test/playwright';
55

66
/**
77
* This behavior does not vary across directions.
@@ -176,5 +176,34 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
176176
await ionModalDidPresent.next();
177177
await expect(datetime).toBeVisible();
178178
});
179+
test('should set datetime ready state and keep calendar interactive when reopening modal', async ({
180+
page,
181+
}, testInfo) => {
182+
testInfo.annotations.push({
183+
type: 'issue',
184+
description: 'https://github.com/ionic-team/ionic-framework/issues/30706',
185+
});
186+
187+
const openAndInteract = async () => {
188+
await page.click('#date-button');
189+
await ionModalDidPresent.next();
190+
191+
await page.locator('ion-datetime.datetime-ready').waitFor();
192+
193+
const calendarBody = datetime.locator('.calendar-body');
194+
await expect(calendarBody).toBeVisible();
195+
};
196+
197+
await openAndInteract();
198+
199+
const firstEnabledDay = datetime.locator('.calendar-day:not([disabled])').first();
200+
await firstEnabledDay.click();
201+
await page.waitForChanges();
202+
203+
await modal.evaluate((el: HTMLIonModalElement) => el.dismiss());
204+
await ionModalDidDismiss.next();
205+
206+
await openAndInteract();
207+
});
179208
});
180209
});

core/src/components/datetime/datetime.tsx

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,32 @@ export class Datetime implements ComponentInterface {
11011101
this.initializeKeyboardListeners();
11021102
}
11031103

1104+
/**
1105+
* TODO(FW-6931): Remove this fallback upon solving the root cause
1106+
* Fallback to ensure the datetime becomes ready even if
1107+
* IntersectionObserver never reports it as intersecting.
1108+
*
1109+
* This is primarily used in environments where the observer
1110+
* might not fire as expected, such as when running under
1111+
* synthetic tests that stub IntersectionObserver.
1112+
*/
1113+
private ensureReadyIfVisible = () => {
1114+
if (this.el.classList.contains('datetime-ready')) {
1115+
return;
1116+
}
1117+
1118+
const rect = this.el.getBoundingClientRect();
1119+
if (rect.width === 0 || rect.height === 0) {
1120+
return;
1121+
}
1122+
1123+
this.initializeListeners();
1124+
1125+
writeTask(() => {
1126+
this.el.classList.add('datetime-ready');
1127+
});
1128+
};
1129+
11041130
componentDidLoad() {
11051131
const { el, intersectionTrackerRef } = this;
11061132

@@ -1141,6 +1167,18 @@ export class Datetime implements ComponentInterface {
11411167
*/
11421168
raf(() => visibleIO?.observe(intersectionTrackerRef!));
11431169

1170+
/**
1171+
* TODO(FW-6931): Remove this fallback upon solving the root cause
1172+
* Fallback: If IntersectionObserver never reports that the
1173+
* datetime is visible but the host clearly has layout, ensure
1174+
* we still initialize listeners and mark the component as ready.
1175+
*
1176+
* We schedule this after everything has had a chance to run.
1177+
*/
1178+
setTimeout(() => {
1179+
this.ensureReadyIfVisible();
1180+
}, 100);
1181+
11441182
/**
11451183
* We need to clean up listeners when the datetime is hidden
11461184
* in a popover/modal so that we can properly scroll containers

core/src/components/datetime/test/basic/datetime.e2e.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,61 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
394394
});
395395
});
396396

397+
/**
398+
* Synthetic IntersectionObserver fallback behavior.
399+
*
400+
* This test stubs IntersectionObserver so that the callback
401+
* never reports an intersecting entry. The datetime should
402+
* still become ready via its internal fallback logic.
403+
*/
404+
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
405+
test.describe(title('datetime: IO fallback'), () => {
406+
test('should become ready even if IntersectionObserver never reports visible', async ({ page }, testInfo) => {
407+
testInfo.annotations.push({
408+
type: 'issue',
409+
description: 'https://github.com/ionic-team/ionic-framework/issues/30706',
410+
});
411+
412+
await page.addInitScript(() => {
413+
const OriginalIO = window.IntersectionObserver;
414+
(window as any).IntersectionObserver = function (callback: any, options: any) {
415+
const instance = new OriginalIO(() => {}, options);
416+
const originalObserve = instance.observe.bind(instance);
417+
418+
instance.observe = (target: Element) => {
419+
originalObserve(target);
420+
callback([
421+
{
422+
isIntersecting: false,
423+
target,
424+
} as IntersectionObserverEntry,
425+
]);
426+
};
427+
428+
return instance;
429+
} as any;
430+
});
431+
432+
await page.setContent(
433+
`
434+
<ion-datetime value="2022-05-03"></ion-datetime>
435+
`,
436+
config
437+
);
438+
439+
const datetime = page.locator('ion-datetime');
440+
441+
// Give the fallback a short amount of time to run
442+
await page.waitForTimeout(100);
443+
444+
await expect(datetime).toHaveClass(/datetime-ready/);
445+
446+
const calendarBody = datetime.locator('.calendar-body');
447+
await expect(calendarBody).toBeVisible();
448+
});
449+
});
450+
});
451+
397452
/**
398453
* We are setting RTL on the component
399454
* instead, so we don't need to test

0 commit comments

Comments
 (0)