Skip to content

Commit 666b48e

Browse files
committed
Merge branch 'main' into FW-7698
2 parents b0dd608 + bf0607e commit 666b48e

15 files changed

Lines changed: 759 additions & 53 deletions

File tree

‎.github/workflows/codeql-analysis.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
security-events: write
1616
steps:
1717
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
18-
- uses: github/codeql-action/init@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0
18+
- uses: github/codeql-action/init@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1
1919
with:
2020
languages: javascript
21-
- uses: github/codeql-action/analyze@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0
21+
- uses: github/codeql-action/analyze@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,24 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
267267
await expect(selectedDay).toBeInViewport();
268268
});
269269

270+
test('should keep the calendar visible across repeated open and dismiss cycles', async ({ page }, testInfo) => {
271+
testInfo.annotations.push({
272+
type: 'issue',
273+
description: 'https://github.com/ionic-team/ionic-framework/issues/30933',
274+
});
275+
276+
const calendarBody = datetime.locator('.calendar-body');
277+
278+
for (let cycle = 0; cycle < 10; cycle++) {
279+
await openModal(page);
280+
281+
await expect(calendarBody).toHaveCSS('opacity', '1');
282+
await expect(monthYear).toHaveText('March 2022');
283+
284+
await dismissModal();
285+
}
286+
});
287+
270288
test('should navigate to the previous month when reopened', async ({ page }, testInfo) => {
271289
testInfo.annotations.push({
272290
type: 'issue',

‎core/src/components/datetime/datetime.tsx‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -136,16 +136,6 @@ export class Datetime implements ComponentInterface {
136136
private todayParts!: DatetimeParts;
137137
private defaultParts!: DatetimeParts;
138138
private loadTimeout: ReturnType<typeof setTimeout> | undefined;
139-
/**
140-
* Set true only by `visibleCallback`. Lets `hiddenCallback` ignore the
141-
* synthetic "not intersecting" entry IntersectionObserver fires on
142-
* `observe()` when the host mounts offscreen.
143-
*
144-
* Don't reset this in `disconnectedCallback`. Overlays disconnect and
145-
* reconnect the host without re-creating the observers, so a reset there
146-
* makes `hiddenCallback` miss the dismissal.
147-
*/
148-
private hasBeenIntersecting = false;
149139

150140
private prevPresentation: string | null = null;
151141

@@ -1158,14 +1148,23 @@ export class Datetime implements ComponentInterface {
11581148
return;
11591149
}
11601150

1161-
const rect = this.el.getBoundingClientRect();
1162-
if (rect.width === 0 || rect.height === 0) {
1151+
if (!this.hasLayoutBox()) {
11631152
return;
11641153
}
11651154

11661155
this.markReady();
11671156
};
11681157

1158+
/**
1159+
* Whether the datetime is on screen. A modal or popover hides its contents
1160+
* with `display: none`, which leaves the host without a layout box.
1161+
*/
1162+
private hasLayoutBox = () => {
1163+
const { width, height } = this.el.getBoundingClientRect();
1164+
1165+
return width > 0 && height > 0;
1166+
};
1167+
11691168
private markReady = () => {
11701169
if (this.el.classList.contains('datetime-ready')) {
11711170
return;
@@ -1203,12 +1202,15 @@ export class Datetime implements ComponentInterface {
12031202
* areas will not have the correct values snapped into place.
12041203
*/
12051204
const visibleCallback = (entries: IntersectionObserverEntry[]) => {
1206-
const ev = entries[0];
1205+
/**
1206+
* The browser can batch several observations into one callback, so
1207+
* only the last entry describes the datetime now.
1208+
*/
1209+
const ev = entries[entries.length - 1];
12071210
if (!ev.isIntersecting) {
12081211
return;
12091212
}
12101213

1211-
this.hasBeenIntersecting = true;
12121214
this.markReady();
12131215
};
12141216
const visibleIO = new IntersectionObserver(visibleCallback, { threshold: 0.01, root: el });
@@ -1244,16 +1246,23 @@ export class Datetime implements ComponentInterface {
12441246
* we did originally has been lost.
12451247
*/
12461248
const hiddenCallback = (entries: IntersectionObserverEntry[]) => {
1247-
const ev = entries[0];
1249+
const ev = entries[entries.length - 1];
12481250
if (ev.isIntersecting) {
12491251
return;
12501252
}
12511253

1252-
// Ignore the initial "not intersecting" entry IntersectionObserver fires on observe().
1253-
if (!this.hasBeenIntersecting) {
1254+
/**
1255+
* WebKit reports a datetime that is still on screen as not
1256+
* intersecting, and it doesn't deliver that entry to every observer on
1257+
* the same root and target, so `visibleCallback` may never hear about
1258+
* it and add `datetime-ready` back. That is what left the calendar
1259+
* blank in #30933. Checking the host instead of trusting the entry
1260+
* also covers the synthetic "not intersecting" entry `observe()` fires
1261+
* when the datetime mounts offscreen.
1262+
*/
1263+
if (this.hasLayoutBox()) {
12541264
return;
12551265
}
1256-
this.hasBeenIntersecting = false;
12571266

12581267
this.destroyInteractionListeners();
12591268

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

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,107 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
488488
});
489489
});
490490

491+
/**
492+
* WebKit can report a datetime that is still on screen as not intersecting,
493+
* so the datetime must not tear down its ready state on that alone.
494+
*/
495+
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
496+
test.describe(title('datetime: spurious hidden report'), () => {
497+
test('should stay ready when the observer reports it hidden while it is on screen', async ({ page }, testInfo) => {
498+
testInfo.annotations.push({
499+
type: 'issue',
500+
description: 'https://github.com/ionic-team/ionic-framework/issues/30933',
501+
});
502+
503+
await page.addInitScript(() => {
504+
const OriginalIO = window.IntersectionObserver;
505+
const datetimeObservers: {
506+
callback: IntersectionObserverCallback;
507+
targets: Element[];
508+
sawVisible: boolean;
509+
}[] = [];
510+
let reportedHidden = false;
511+
512+
/**
513+
* The datetime only tears down once its observers have reported it
514+
* visible, so the test waits for that first.
515+
*/
516+
(window as any).datetimeObserversSawVisible = () =>
517+
datetimeObservers.length > 0 && datetimeObservers.every(({ sawVisible }) => sawVisible);
518+
519+
/**
520+
* Reports the datetime as not intersecting and then goes quiet,
521+
* since WebKit never sends a recovery entry to undo it.
522+
*/
523+
(window as any).reportHiddenToDatetimeObservers = () => {
524+
reportedHidden = true;
525+
datetimeObservers.forEach(({ callback, targets }) => {
526+
targets.forEach((target) => {
527+
callback([{ isIntersecting: false, target } as IntersectionObserverEntry], null as any);
528+
});
529+
});
530+
};
531+
532+
(window as any).IntersectionObserver = function (
533+
callback: IntersectionObserverCallback,
534+
options?: IntersectionObserverInit
535+
) {
536+
const root = options?.root as Element | null;
537+
538+
if (root?.tagName !== 'ION-DATETIME') {
539+
return new OriginalIO(callback, options);
540+
}
541+
542+
const record = { callback, targets: [] as Element[], sawVisible: false };
543+
datetimeObservers.push(record);
544+
545+
const instance = new OriginalIO((entries, observer) => {
546+
if (reportedHidden) {
547+
return;
548+
}
549+
550+
if (entries.some((entry) => entry.isIntersecting)) {
551+
record.sawVisible = true;
552+
}
553+
554+
callback(entries, observer);
555+
}, options);
556+
557+
const originalObserve = instance.observe.bind(instance);
558+
instance.observe = (target: Element) => {
559+
record.targets.push(target);
560+
originalObserve(target);
561+
};
562+
563+
return instance;
564+
} as any;
565+
});
566+
567+
await page.setContent(`<ion-datetime value="2022-05-03"></ion-datetime>`, config);
568+
569+
const datetime = page.locator('ion-datetime');
570+
const calendarBody = datetime.locator('.calendar-body');
571+
572+
await expect(datetime).toHaveClass(/datetime-ready/);
573+
await expect(calendarBody).toHaveCSS('opacity', '1');
574+
await page.waitForFunction(() => (window as any).datetimeObserversSawVisible());
575+
576+
/**
577+
* A one-shot layout fallback runs 100ms after the datetime loads and
578+
* would add the class back. Real reports arrive long after that, so
579+
* wait it out.
580+
*/
581+
await page.waitForTimeout(300);
582+
583+
await page.evaluate(() => (window as any).reportHiddenToDatetimeObservers());
584+
await page.waitForChanges();
585+
586+
await expect(datetime).toHaveClass(/datetime-ready/);
587+
await expect(calendarBody).toHaveCSS('opacity', '1');
588+
});
589+
});
590+
});
591+
491592
/**
492593
* We are setting RTL on the component
493594
* instead, so we don't need to test

‎core/src/components/popover/animations/ios.enter.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { Animation } from '../../../interface';
66
import {
77
calculateWindowAdjustment,
88
getArrowDimensions,
9+
getElementCSSZoom,
910
getPopoverDimensions,
1011
getPopoverPosition,
1112
getSafeAreaInsets,
@@ -32,16 +33,31 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
3233
const { event: ev, size, trigger, reference, side, align } = opts;
3334
const doc = baseEl.ownerDocument as any;
3435
const rtl = isRTL(baseEl);
35-
const bodyWidth = doc.defaultView.innerWidth;
36-
const bodyHeight = doc.defaultView.innerHeight;
37-
3836
const root = getElementRoot(baseEl);
3937
const contentEl = root.querySelector('.popover-content') as HTMLElement;
4038
const arrowEl = root.querySelector('.popover-arrow') as HTMLElement | null;
4139

40+
/**
41+
* A CSS `zoom` other than 1 on an ancestor (e.g. the `html` element) causes
42+
* geometry APIs like `getBoundingClientRect()` to report zoomed values while
43+
* inline `top`/`left`/`--width` styles are interpreted in the unzoomed layout
44+
* space. Normalize all rect-derived measurements by this factor so the
45+
* popover is positioned and sized correctly.
46+
*/
47+
const zoom = getElementCSSZoom(contentEl);
48+
49+
/**
50+
* `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so they must be
51+
* scaled down to the same layout space as the normalized measurements above.
52+
* Otherwise the popover would be clamped against a viewport that is larger
53+
* than the space actually available to it.
54+
*/
55+
const bodyWidth = doc.defaultView.innerWidth / zoom;
56+
const bodyHeight = doc.defaultView.innerHeight / zoom;
57+
4258
const referenceSizeEl = trigger || ev?.detail?.ionShadowTarget || ev?.target;
43-
const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl);
44-
const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl);
59+
const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl, zoom);
60+
const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl, zoom);
4561

4662
const defaultPosition = {
4763
top: bodyHeight / 2 - contentHeight / 2,
@@ -61,7 +77,8 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
6177
align,
6278
defaultPosition,
6379
trigger,
64-
ev
80+
ev,
81+
zoom
6582
);
6683

6784
const padding = size === 'cover' ? 0 : POPOVER_IOS_BODY_PADDING;

‎core/src/components/popover/animations/md.enter.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import { getElementRoot } from '@utils/helpers';
33
import { isRTL } from '@utils/rtl';
44

55
import type { Animation } from '../../../interface';
6-
import { calculateWindowAdjustment, getPopoverDimensions, getPopoverPosition, getSafeAreaInsets } from '../utils';
6+
import {
7+
calculateWindowAdjustment,
8+
getElementCSSZoom,
9+
getPopoverDimensions,
10+
getPopoverPosition,
11+
getSafeAreaInsets,
12+
} from '../utils';
713

814
const POPOVER_MD_BODY_PADDING = 12;
915

@@ -16,14 +22,29 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
1622
const doc = baseEl.ownerDocument as any;
1723
const rtl = isRTL(baseEl);
1824

19-
const bodyWidth = doc.defaultView.innerWidth;
20-
const bodyHeight = doc.defaultView.innerHeight;
21-
2225
const root = getElementRoot(baseEl);
2326
const contentEl = root.querySelector('.popover-content') as HTMLElement;
2427

28+
/**
29+
* A CSS `zoom` other than 1 on an ancestor (e.g. the `html` element) causes
30+
* geometry APIs like `getBoundingClientRect()` to report zoomed values while
31+
* inline `top`/`left`/`--width` styles are interpreted in the unzoomed layout
32+
* space. Normalize all rect-derived measurements by this factor so the
33+
* popover is positioned and sized correctly.
34+
*/
35+
const zoom = getElementCSSZoom(contentEl);
36+
37+
/**
38+
* `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so they must be
39+
* scaled down to the same layout space as the normalized measurements above.
40+
* Otherwise the popover would be clamped against a viewport that is larger
41+
* than the space actually available to it.
42+
*/
43+
const bodyWidth = doc.defaultView.innerWidth / zoom;
44+
const bodyHeight = doc.defaultView.innerHeight / zoom;
45+
2546
const referenceSizeEl = trigger || ev?.detail?.ionShadowTarget || ev?.target;
26-
const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl);
47+
const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl, zoom);
2748

2849
const defaultPosition = {
2950
top: bodyHeight / 2 - contentHeight / 2,
@@ -43,7 +64,8 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
4364
align,
4465
defaultPosition,
4566
trigger,
46-
ev
67+
ev,
68+
zoom
4769
);
4870

4971
const padding = size === 'cover' ? 0 : POPOVER_MD_BODY_PADDING;

0 commit comments

Comments
 (0)