From c938054605dffb6c3002a64a3d8aaf36892c7a93 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 1 Sep 2022 08:52:00 -0500 Subject: [PATCH] fix(datetime): month grid no longer loops on ios (#25857) resolves #25752 --- core/src/components/datetime/datetime.tsx | 23 ++++++++- .../datetime/test/minmax/datetime.e2e.ts | 47 +++++++++++++++++++ .../utils/test/playwright/matchers/index.ts | 2 + .../matchers/toHaveReceivedEventTimes.ts | 33 +++++++++++++ .../src/utils/test/playwright/testExpect.d.ts | 5 ++ 5 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 core/src/utils/test/playwright/matchers/toHaveReceivedEventTimes.ts diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 8db5b2fe20..a8c9d46ab6 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1106,7 +1106,28 @@ export class Datetime implements ComponentInterface { * so we need to re-init behavior with the new elements. */ componentDidRender() { - const { presentation, prevPresentation } = this; + const { presentation, prevPresentation, calendarBodyRef, minParts, preferWheel } = this; + + /** + * TODO(FW-2165) + * Remove this when https://bugs.webkit.org/show_bug.cgi?id=235960 is fixed. + * When using `min`, we add `scroll-snap-align: none` + * to the disabled month so that users cannot scroll to it. + * This triggers a bug in WebKit where the scroll position is reset. + * Since the month change logic is handled by a scroll listener, + * this causes the month to change leading to `scroll-snap-align` + * changing again, thus changing the scroll position again and causing + * an infinite loop. + * This issue only applies to the calendar grid, so we can disable + * it if the calendar grid is not being used. + */ + const hasCalendarGrid = !preferWheel && ['date-time', 'time-date', 'date'].includes(presentation); + if (minParts !== undefined && hasCalendarGrid && calendarBodyRef) { + const workingMonth = calendarBodyRef.querySelector('.calendar-month:nth-of-type(1)'); + if (workingMonth) { + calendarBodyRef.scrollLeft = workingMonth.clientWidth * (isRTL(this.el) ? -1 : 1); + } + } if (prevPresentation === null) { this.prevPresentation = presentation; diff --git a/core/src/components/datetime/test/minmax/datetime.e2e.ts b/core/src/components/datetime/test/minmax/datetime.e2e.ts index f3e434d6ed..1d4a567994 100644 --- a/core/src/components/datetime/test/minmax/datetime.e2e.ts +++ b/core/src/components/datetime/test/minmax/datetime.e2e.ts @@ -141,4 +141,51 @@ test.describe('datetime: minmax', () => { await testDisplayedMonth(page, ``, 'June 2012'); }); }); + + // TODO(FW-2165) + test('should not loop infinitely in webkit', async ({ page, skip }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/25752', + }); + + skip.browser('chromium'); + skip.browser('firefox'); + + await page.setContent(` + + + + + `); + await page.waitForSelector('.datetime-ready'); + + const datetimeMonthDidChange = await page.spyOnEvent('datetimeMonthDidChange'); + const eventButton = page.locator('button#bind'); + await eventButton.click(); + + const buttons = page.locator('ion-datetime .calendar-next-prev ion-button'); + await buttons.nth(1).click(); + await page.waitForChanges(); + + await datetimeMonthDidChange.next(); + + /** + * This is hacky, but its purpose is to make sure + * we are not triggering a WebKit bug. When the fix + * for the bug ships in WebKit, this will be removed. + */ + await page.evaluate(() => { + return new Promise((resolve) => { + setTimeout(resolve, 500); + }); + }); + + await expect(datetimeMonthDidChange).toHaveReceivedEventTimes(1); + }); }); diff --git a/core/src/utils/test/playwright/matchers/index.ts b/core/src/utils/test/playwright/matchers/index.ts index 5e3997ccc7..a22b179b6d 100644 --- a/core/src/utils/test/playwright/matchers/index.ts +++ b/core/src/utils/test/playwright/matchers/index.ts @@ -1,7 +1,9 @@ import { toHaveReceivedEvent } from './toHaveReceivedEvent'; import { toHaveReceivedEventDetail } from './toHaveReceivedEventDetail'; +import { toHaveReceivedEventTimes } from './toHaveReceivedEventTimes'; export const matchers = { toHaveReceivedEvent, toHaveReceivedEventDetail, + toHaveReceivedEventTimes, }; diff --git a/core/src/utils/test/playwright/matchers/toHaveReceivedEventTimes.ts b/core/src/utils/test/playwright/matchers/toHaveReceivedEventTimes.ts new file mode 100644 index 0000000000..1484c6a791 --- /dev/null +++ b/core/src/utils/test/playwright/matchers/toHaveReceivedEventTimes.ts @@ -0,0 +1,33 @@ +import type { EventSpy } from '../page/event-spy'; + +export function toHaveReceivedEventTimes(eventSpy: EventSpy, count: number) { + if (!eventSpy) { + return { + message: () => `toHaveReceivedEventTimes event spy is null`, + pass: false, + }; + } + + if (typeof (eventSpy as any).then === 'function') { + return { + message: () => + `expected spy to have received event, but it was not resolved (did you forget an await operator?).`, + pass: false, + }; + } + + if (!eventSpy.eventName) { + return { + message: () => `toHaveReceivedEventTimes did not receive an event spy`, + pass: false, + }; + } + + const pass = eventSpy.length === count; + + return { + message: () => + `expected event "${eventSpy.eventName}" to have been called ${count} times, but it was called ${eventSpy.events.length} times`, + pass: pass, + }; +} diff --git a/core/src/utils/test/playwright/testExpect.d.ts b/core/src/utils/test/playwright/testExpect.d.ts index 9028360c5c..84c4bfeaac 100644 --- a/core/src/utils/test/playwright/testExpect.d.ts +++ b/core/src/utils/test/playwright/testExpect.d.ts @@ -8,6 +8,11 @@ interface CustomMatchers { * @param eventDetail The expected detail of the event. */ toHaveReceivedEventDetail(eventDetail: any): R; + + /** + * Will check how many times the event has been received. + */ + toHaveReceivedEventTimes(count: number): R; } declare namespace PlaywrightTest {