From 14419288c0861244feac56c1c5f647ef1dcdcb71 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 12 Dec 2025 06:39:42 -0800 Subject: [PATCH 1/4] fix(datetime): use ResizeObserver to reliably detect visibility changes --- core/src/components/datetime/datetime.tsx | 163 +++++++----------- .../datetime/test/basic/datetime.e2e.ts | 36 +--- 2 files changed, 65 insertions(+), 134 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 3411f28eb57..4bd5d84f1da 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -108,8 +108,8 @@ export class Datetime implements ComponentInterface { private inputId = `ion-dt-${datetimeIds++}`; private calendarBodyRef?: HTMLElement; private popoverRef?: HTMLIonPopoverElement; - private intersectionTrackerRef?: HTMLElement; private clearFocusVisible?: () => void; + private resizeObserver?: ResizeObserver; private parsedMinuteValues?: number[]; private parsedHourValues?: number[]; private parsedMonthValues?: number[]; @@ -1077,6 +1077,10 @@ export class Datetime implements ComponentInterface { this.clearFocusVisible(); this.clearFocusVisible = undefined; } + if (this.resizeObserver) { + this.resizeObserver.disconnect(); + this.resizeObserver = undefined; + } } /** @@ -1101,34 +1105,8 @@ export class Datetime implements ComponentInterface { this.initializeKeyboardListeners(); } - /** - * TODO(FW-6931): Remove this fallback upon solving the root cause - * Fallback to ensure the datetime becomes ready even if - * IntersectionObserver never reports it as intersecting. - * - * This is primarily used in environments where the observer - * might not fire as expected, such as when running under - * synthetic tests that stub IntersectionObserver. - */ - private ensureReadyIfVisible = () => { - if (this.el.classList.contains('datetime-ready')) { - return; - } - - const rect = this.el.getBoundingClientRect(); - if (rect.width === 0 || rect.height === 0) { - return; - } - - this.initializeListeners(); - - writeTask(() => { - this.el.classList.add('datetime-ready'); - }); - }; - componentDidLoad() { - const { el, intersectionTrackerRef } = this; + const { el } = this; /** * If a scrollable element is hidden using `display: none`, @@ -1136,79 +1114,68 @@ export class Datetime implements ComponentInterface { * into view. As a result, we will need to wait for the datetime to become * visible if used inside of a modal or a popover otherwise the scrollable * areas will not have the correct values snapped into place. + * + * FW-6931: We use ResizeObserver to detect when the element transitions + * between having dimensions (visible) and zero dimensions (hidden). This + * is more reliable than IntersectionObserver for detecting visibility + * changes, especially when the element is inside a modal or popover. */ - const visibleCallback = (entries: IntersectionObserverEntry[]) => { - const ev = entries[0]; - if (!ev.isIntersecting) { - return; - } + if (typeof ResizeObserver !== 'undefined') { + this.resizeObserver = new ResizeObserver((entries) => { + const entry = entries[0]; + const { width, height } = entry.contentRect; + const isVisible = width > 0 && height > 0; + const isReady = el.classList.contains('datetime-ready'); - this.initializeListeners(); + if (isVisible && !isReady) { + this.initializeListeners(); - /** - * TODO FW-2793: Datetime needs a frame to ensure that it - * can properly scroll contents into view. As a result - * we hide the scrollable content until after that frame - * so users do not see the content quickly shifting. The downside - * is that the content will pop into view a frame after. Maybe there - * is a better way to handle this? - */ - writeTask(() => { - this.el.classList.add('datetime-ready'); - }); - }; - const visibleIO = new IntersectionObserver(visibleCallback, { threshold: 0.01, root: el }); - - /** - * Use raf to avoid a race condition between the component loading and - * its display animation starting (such as when shown in a modal). This - * could cause the datetime to start at a visibility of 0, erroneously - * triggering the `hiddenIO` observer below. - */ - raf(() => visibleIO?.observe(intersectionTrackerRef!)); - - /** - * TODO(FW-6931): Remove this fallback upon solving the root cause - * Fallback: If IntersectionObserver never reports that the - * datetime is visible but the host clearly has layout, ensure - * we still initialize listeners and mark the component as ready. - * - * We schedule this after everything has had a chance to run. - */ - setTimeout(() => { - this.ensureReadyIfVisible(); - }, 100); + /** + * TODO FW-2793: Datetime needs a frame to ensure that it + * can properly scroll contents into view. As a result + * we hide the scrollable content until after that frame + * so users do not see the content quickly shifting. The downside + * is that the content will pop into view a frame after. Maybe there + * is a better way to handle this? + */ + writeTask(() => { + el.classList.add('datetime-ready'); + }); + } else if (!isVisible && isReady) { + /** + * Clean up listeners when hidden so we can properly + * reinitialize scroll positions on re-presentation. + */ + this.destroyInteractionListeners(); - /** - * We need to clean up listeners when the datetime is hidden - * in a popover/modal so that we can properly scroll containers - * back into view if they are re-presented. When the datetime is hidden - * the scroll areas have scroll widths/heights of 0px, so any snapping - * we did originally has been lost. - */ - const hiddenCallback = (entries: IntersectionObserverEntry[]) => { - const ev = entries[0]; - if (ev.isIntersecting) { - return; - } + /** + * Close month/year picker when hidden, otherwise + * it will be open when re-presented with a 0-height + * scroll area, showing the wrong month. + */ + this.showMonthAndYear = false; - this.destroyInteractionListeners(); + writeTask(() => { + el.classList.remove('datetime-ready'); + }); + } + }); /** - * When datetime is hidden, we need to make sure that - * the month/year picker is closed. Otherwise, - * it will be open when the datetime re-appears - * and the scroll area of the calendar grid will be 0. - * As a result, the wrong month will be shown. + * Use raf to avoid a race condition between the component loading and + * its display animation starting (such as when shown in a modal). + */ + raf(() => this.resizeObserver?.observe(el)); + } else { + /** + * Fallback for test environments where ResizeObserver is not available. + * Just mark as ready without initializing scroll/keyboard listeners + * since those also require browser APIs not available in Jest. */ - this.showMonthAndYear = false; - writeTask(() => { - this.el.classList.remove('datetime-ready'); + el.classList.add('datetime-ready'); }); - }; - const hiddenIO = new IntersectionObserver(hiddenCallback, { threshold: 0, root: el }); - raf(() => hiddenIO?.observe(intersectionTrackerRef!)); + } /** * Datetime uses Ionic components that emit @@ -2693,20 +2660,6 @@ export class Datetime implements ComponentInterface { }), }} > - {/* - WebKit has a quirk where IntersectionObserver callbacks are delayed until after - an accelerated animation finishes if the "root" specified in the config is the - browser viewport (the default behavior if "root" is not specified). This means - that when presenting a datetime in a modal on iOS the calendar body appears - blank until the modal animation finishes. - - We can work around this by observing .intersection-tracker and using the host - (ion-datetime) as the "root". This allows the IO callback to fire the moment - the datetime is visible. The .intersection-tracker element should not have - dimensions or additional styles, and it should not be positioned absolutely - otherwise the IO callback may fire at unexpected times. - */} -
(this.intersectionTrackerRef = el)}>
{this.renderDatetime(mode)} ); diff --git a/core/src/components/datetime/test/basic/datetime.e2e.ts b/core/src/components/datetime/test/basic/datetime.e2e.ts index 6104d0014cf..122af7da7dc 100644 --- a/core/src/components/datetime/test/basic/datetime.e2e.ts +++ b/core/src/components/datetime/test/basic/datetime.e2e.ts @@ -395,40 +395,18 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => }); /** - * Synthetic IntersectionObserver fallback behavior. - * - * This test stubs IntersectionObserver so that the callback - * never reports an intersecting entry. The datetime should - * still become ready via its internal fallback logic. + * Verify that datetime becomes ready via ResizeObserver. + * This tests that the datetime properly initializes when it has + * dimensions, using ResizeObserver to detect visibility. */ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { - test.describe(title('datetime: IO fallback'), () => { - test('should become ready even if IntersectionObserver never reports visible', async ({ page }, testInfo) => { + test.describe(title('datetime: visibility detection'), () => { + test('should become ready when rendered with dimensions', async ({ page }, testInfo) => { testInfo.annotations.push({ type: 'issue', description: 'https://github.com/ionic-team/ionic-framework/issues/30706', }); - await page.addInitScript(() => { - const OriginalIO = window.IntersectionObserver; - (window as any).IntersectionObserver = function (callback: any, options: any) { - const instance = new OriginalIO(() => {}, options); - const originalObserve = instance.observe.bind(instance); - - instance.observe = (target: Element) => { - originalObserve(target); - callback([ - { - isIntersecting: false, - target, - } as IntersectionObserverEntry, - ]); - }; - - return instance; - } as any; - }); - await page.setContent( ` @@ -438,8 +416,8 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { const datetime = page.locator('ion-datetime'); - // Give the fallback a short amount of time to run - await page.waitForTimeout(100); + // Wait for the datetime to become ready via ResizeObserver + await page.locator('.datetime-ready').waitFor(); await expect(datetime).toHaveClass(/datetime-ready/); From 79d5716baf1517378ffaaba9cf274dc71c583137 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 12 Dec 2025 07:29:30 -0800 Subject: [PATCH 2/4] fix(tests): updating tests to wait for datetime --- .../datetime-button/test/a11y/datetime-button.e2e.ts | 2 ++ .../datetime-button/test/overlays/datetime-button.e2e.ts | 6 ++++++ core/src/components/datetime/test/custom/datetime.e2e.ts | 1 + .../datetime/test/first-day-of-week/datetime.e2e.ts | 1 + .../datetime/test/highlighted-dates/datetime.e2e.ts | 1 + 5 files changed, 11 insertions(+) diff --git a/core/src/components/datetime-button/test/a11y/datetime-button.e2e.ts b/core/src/components/datetime-button/test/a11y/datetime-button.e2e.ts index a48a06a2894..0f4d682a6dc 100644 --- a/core/src/components/datetime-button/test/a11y/datetime-button.e2e.ts +++ b/core/src/components/datetime-button/test/a11y/datetime-button.e2e.ts @@ -21,6 +21,7 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, screenshot, c ); const datetimeButton = page.locator('ion-datetime-button'); + await page.locator('.datetime-ready').waitFor(); await expect(datetimeButton).toHaveScreenshot(screenshot(`datetime-button-scale`)); }); @@ -40,6 +41,7 @@ configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, screenshot, c ); const datetimeButton = page.locator('ion-datetime-button'); + await page.locator('.datetime-ready').waitFor(); await expect(datetimeButton).toHaveScreenshot(screenshot(`datetime-button-scale-wrap`)); }); diff --git a/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts b/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts index a626dd90855..5e4fefd243e 100644 --- a/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts +++ b/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts @@ -24,6 +24,9 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { await dateButton.click(); await ionModalDidPresent.next(); + // Wait for datetime to be ready before taking screenshot + await page.locator('ion-datetime.datetime-ready').waitFor(); + await expect(page).toHaveScreenshot(screenshot(`datetime-overlay-modal`)); }); @@ -44,6 +47,9 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { await dateButton.click(); await ionPopoverDidPresent.next(); + // Wait for datetime to be ready before taking screenshot + await page.locator('ion-datetime.datetime-ready').waitFor(); + await expect(page).toHaveScreenshot(screenshot(`datetime-overlay-popover`)); }); }); diff --git a/core/src/components/datetime/test/custom/datetime.e2e.ts b/core/src/components/datetime/test/custom/datetime.e2e.ts index 60339fbf742..b62622f0ada 100644 --- a/core/src/components/datetime/test/custom/datetime.e2e.ts +++ b/core/src/components/datetime/test/custom/datetime.e2e.ts @@ -52,6 +52,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { test.describe(title('datetime: custom focus'), () => { test('should focus the selected day and then the day after', async ({ page }) => { await page.goto(`/src/components/datetime/test/custom`, config); + await page.locator('.datetime-ready').last().waitFor(); const datetime = page.locator('#custom-calendar-days'); diff --git a/core/src/components/datetime/test/first-day-of-week/datetime.e2e.ts b/core/src/components/datetime/test/first-day-of-week/datetime.e2e.ts index 3630d2c5f45..9cb8862415a 100644 --- a/core/src/components/datetime/test/first-day-of-week/datetime.e2e.ts +++ b/core/src/components/datetime/test/first-day-of-week/datetime.e2e.ts @@ -7,6 +7,7 @@ configs().forEach(({ title, screenshot, config }) => { await page.goto('/src/components/datetime/test/first-day-of-week', config); const datetime = page.locator('ion-datetime'); + await page.locator('.datetime-ready').waitFor(); await expect(datetime).toHaveScreenshot(screenshot(`datetime-day-of-week`)); }); }); diff --git a/core/src/components/datetime/test/highlighted-dates/datetime.e2e.ts b/core/src/components/datetime/test/highlighted-dates/datetime.e2e.ts index 12f014b74e1..89ada6a9d75 100644 --- a/core/src/components/datetime/test/highlighted-dates/datetime.e2e.ts +++ b/core/src/components/datetime/test/highlighted-dates/datetime.e2e.ts @@ -10,6 +10,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { `, config ); + await page.locator('.datetime-ready').waitFor(); }); test('should render highlights correctly when using an array', async ({ page }) => { From 12ef9345d6e5a9e7f60afb959b11807daaa2d3f1 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 12 Dec 2025 08:03:54 -0800 Subject: [PATCH 3/4] fix(date-time): adding fallback for visibility back for e2e tests --- core/src/components/datetime/datetime.tsx | 85 ++++++++++++++--------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 4bd5d84f1da..e129b144d91 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1105,6 +1105,26 @@ export class Datetime implements ComponentInterface { this.initializeKeyboardListeners(); } + /** + * FW-6931: Fallback check for when ResizeObserver doesn't fire reliably + * (e.g., WebKit during modal re-presentation). Called after element is + * hidden to catch when it becomes visible again. + */ + private checkVisibilityFallback = () => { + const { el } = this; + if (el.classList.contains('datetime-ready')) { + return; + } + + const rect = el.getBoundingClientRect(); + if (rect.width > 0 && rect.height > 0) { + this.initializeListeners(); + writeTask(() => { + el.classList.add('datetime-ready'); + }); + } + }; + componentDidLoad() { const { el } = this; @@ -1115,11 +1135,31 @@ export class Datetime implements ComponentInterface { * visible if used inside of a modal or a popover otherwise the scrollable * areas will not have the correct values snapped into place. * - * FW-6931: We use ResizeObserver to detect when the element transitions - * between having dimensions (visible) and zero dimensions (hidden). This - * is more reliable than IntersectionObserver for detecting visibility - * changes, especially when the element is inside a modal or popover. + * We use ResizeObserver to detect when the element transitions between + * having dimensions (visible) and zero dimensions (hidden). This is more + * reliable than IntersectionObserver for detecting visibility changes, + * especially when the element is inside a modal or popover. */ + const markReady = () => { + this.initializeListeners(); + writeTask(() => { + el.classList.add('datetime-ready'); + }); + }; + + const markHidden = () => { + this.destroyInteractionListeners(); + this.showMonthAndYear = false; + writeTask(() => { + el.classList.remove('datetime-ready'); + }); + /** + * Schedule fallback check for browsers where ResizeObserver + * doesn't fire reliably on re-presentation (e.g., WebKit). + */ + setTimeout(() => this.checkVisibilityFallback(), 100); + }; + if (typeof ResizeObserver !== 'undefined') { this.resizeObserver = new ResizeObserver((entries) => { const entry = entries[0]; @@ -1128,36 +1168,9 @@ export class Datetime implements ComponentInterface { const isReady = el.classList.contains('datetime-ready'); if (isVisible && !isReady) { - this.initializeListeners(); - - /** - * TODO FW-2793: Datetime needs a frame to ensure that it - * can properly scroll contents into view. As a result - * we hide the scrollable content until after that frame - * so users do not see the content quickly shifting. The downside - * is that the content will pop into view a frame after. Maybe there - * is a better way to handle this? - */ - writeTask(() => { - el.classList.add('datetime-ready'); - }); + markReady(); } else if (!isVisible && isReady) { - /** - * Clean up listeners when hidden so we can properly - * reinitialize scroll positions on re-presentation. - */ - this.destroyInteractionListeners(); - - /** - * Close month/year picker when hidden, otherwise - * it will be open when re-presented with a 0-height - * scroll area, showing the wrong month. - */ - this.showMonthAndYear = false; - - writeTask(() => { - el.classList.remove('datetime-ready'); - }); + markHidden(); } }); @@ -1166,6 +1179,12 @@ export class Datetime implements ComponentInterface { * its display animation starting (such as when shown in a modal). */ raf(() => this.resizeObserver?.observe(el)); + + /** + * Fallback for initial presentation in case ResizeObserver + * doesn't fire reliably (e.g., WebKit). + */ + setTimeout(() => this.checkVisibilityFallback(), 100); } else { /** * Fallback for test environments where ResizeObserver is not available. From a8772e8bd9273eb65e257a2e303f6bc147a6a682 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 12 Dec 2025 08:41:17 -0800 Subject: [PATCH 4/4] fix(datetime): moving to overlay listeners for showing instead --- core/src/components/datetime/datetime.tsx | 116 ++++++++++++---------- 1 file changed, 63 insertions(+), 53 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index e129b144d91..c6e868a6908 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -118,6 +118,7 @@ export class Datetime implements ComponentInterface { private destroyCalendarListener?: () => void; private destroyKeyboardMO?: () => void; + private destroyOverlayListeners?: () => void; // TODO(FW-2832): types (DatetimeParts causes some errors that need untangling) private minParts?: any; @@ -1081,6 +1082,10 @@ export class Datetime implements ComponentInterface { this.resizeObserver.disconnect(); this.resizeObserver = undefined; } + if (this.destroyOverlayListeners) { + this.destroyOverlayListeners(); + this.destroyOverlayListeners = undefined; + } } /** @@ -1106,41 +1111,21 @@ export class Datetime implements ComponentInterface { } /** - * FW-6931: Fallback check for when ResizeObserver doesn't fire reliably - * (e.g., WebKit during modal re-presentation). Called after element is - * hidden to catch when it becomes visible again. + * Sets up visibility detection for the datetime component. + * + * Uses multiple strategies to reliably detect when the datetime becomes + * visible, which is necessary for proper initialization of scrollable areas: + * 1. ResizeObserver - detects dimension changes + * 2. Overlay event listeners - for datetime inside modals/popovers + * 3. Polling fallback - for browsers where observers are unreliable (WebKit) */ - private checkVisibilityFallback = () => { - const { el } = this; - if (el.classList.contains('datetime-ready')) { - return; - } - - const rect = el.getBoundingClientRect(); - if (rect.width > 0 && rect.height > 0) { - this.initializeListeners(); - writeTask(() => { - el.classList.add('datetime-ready'); - }); - } - }; - - componentDidLoad() { + private initializeVisibilityObserver() { const { el } = this; - /** - * If a scrollable element is hidden using `display: none`, - * it will not have a scroll height meaning we cannot scroll elements - * into view. As a result, we will need to wait for the datetime to become - * visible if used inside of a modal or a popover otherwise the scrollable - * areas will not have the correct values snapped into place. - * - * We use ResizeObserver to detect when the element transitions between - * having dimensions (visible) and zero dimensions (hidden). This is more - * reliable than IntersectionObserver for detecting visibility changes, - * especially when the element is inside a modal or popover. - */ const markReady = () => { + if (el.classList.contains('datetime-ready')) { + return; + } this.initializeListeners(); writeTask(() => { el.classList.add('datetime-ready'); @@ -1153,17 +1138,50 @@ export class Datetime implements ComponentInterface { writeTask(() => { el.classList.remove('datetime-ready'); }); - /** - * Schedule fallback check for browsers where ResizeObserver - * doesn't fire reliably on re-presentation (e.g., WebKit). - */ - setTimeout(() => this.checkVisibilityFallback(), 100); + startVisibilityPolling(); + }; + + /** + * FW-6931: Poll for visibility as a fallback for browsers where + * ResizeObserver doesn't fire reliably (e.g., WebKit). + */ + const startVisibilityPolling = () => { + let pollCount = 0; + const poll = () => { + if (el.classList.contains('datetime-ready') || pollCount++ >= 60) { + return; + } + const { width, height } = el.getBoundingClientRect(); + if (width > 0 && height > 0) { + markReady(); + } else { + raf(poll); + } + }; + raf(poll); }; + /** + * FW-6931: Listen for overlay present/dismiss events when datetime + * is inside a modal or popover. + */ + const parentOverlay = el.closest('ion-modal, ion-popover') as HTMLIonModalElement | HTMLIonPopoverElement | null; + if (parentOverlay) { + const handlePresent = () => markReady(); + const handleDismiss = () => markHidden(); + + parentOverlay.addEventListener('didPresent', handlePresent); + parentOverlay.addEventListener('didDismiss', handleDismiss); + + this.destroyOverlayListeners = () => { + parentOverlay.removeEventListener('didPresent', handlePresent); + parentOverlay.removeEventListener('didDismiss', handleDismiss); + }; + } + if (typeof ResizeObserver !== 'undefined') { this.resizeObserver = new ResizeObserver((entries) => { - const entry = entries[0]; - const { width, height } = entry.contentRect; + const { width, height } = entries[0].contentRect; const isVisible = width > 0 && height > 0; const isReady = el.classList.contains('datetime-ready'); @@ -1174,27 +1192,19 @@ export class Datetime implements ComponentInterface { } }); - /** - * Use raf to avoid a race condition between the component loading and - * its display animation starting (such as when shown in a modal). - */ + // Use raf to avoid race condition with modal/popover animations raf(() => this.resizeObserver?.observe(el)); - - /** - * Fallback for initial presentation in case ResizeObserver - * doesn't fire reliably (e.g., WebKit). - */ - setTimeout(() => this.checkVisibilityFallback(), 100); + startVisibilityPolling(); } else { - /** - * Fallback for test environments where ResizeObserver is not available. - * Just mark as ready without initializing scroll/keyboard listeners - * since those also require browser APIs not available in Jest. - */ + // Test environment fallback - mark ready immediately writeTask(() => { el.classList.add('datetime-ready'); }); } + } + + componentDidLoad() { + this.initializeVisibilityObserver(); /** * Datetime uses Ionic components that emit