Skip to content

Commit

Permalink
fix(datetime): month grid no longer loops on ios (#25857)
Browse files Browse the repository at this point in the history
resolves #25752
  • Loading branch information
liamdebeasi authored Sep 1, 2022
1 parent 121370e commit c938054
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 1 deletion.
23 changes: 22 additions & 1 deletion core/src/components/datetime/datetime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
47 changes: 47 additions & 0 deletions core/src/components/datetime/test/minmax/datetime.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,51 @@ test.describe('datetime: minmax', () => {
await testDisplayedMonth(page, `<ion-datetime max="2012-06-01"></ion-datetime>`, '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(`
<button id="bind">Bind datetimeMonthDidChange event</button>
<ion-datetime min="2022-04-15" value="2022-04-20" presentation="date" locale="en-US"></ion-datetime>
<script type="module">
import { InitMonthDidChangeEvent } from '/src/components/datetime/test/utils/month-did-change-event.js';
document.querySelector('#bind').addEventListener('click', function() {
InitMonthDidChangeEvent();
});
</script>
`);
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);
});
});
2 changes: 2 additions & 0 deletions core/src/utils/test/playwright/matchers/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { toHaveReceivedEvent } from './toHaveReceivedEvent';
import { toHaveReceivedEventDetail } from './toHaveReceivedEventDetail';
import { toHaveReceivedEventTimes } from './toHaveReceivedEventTimes';

export const matchers = {
toHaveReceivedEvent,
toHaveReceivedEventDetail,
toHaveReceivedEventTimes,
};
Original file line number Diff line number Diff line change
@@ -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,
};
}
5 changes: 5 additions & 0 deletions core/src/utils/test/playwright/testExpect.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ interface CustomMatchers<R = unknown> {
* @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 {
Expand Down

0 comments on commit c938054

Please sign in to comment.