From 9a679828b1eb15f75fff51c3f4d493314282c8ee Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 17 Jan 2024 13:45:08 -0600 Subject: [PATCH 1/6] fix(datetime): do not animate to new value when multiple values in different months are set --- core/src/components/datetime/datetime.tsx | 51 +++++++++++++------ .../datetime/test/multiple/datetime.e2e.ts | 11 ++-- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 77a8727f64f..57f275e25f0 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1284,21 +1284,42 @@ export class Datetime implements ComponentInterface { (month !== undefined && month !== workingParts.month) || (year !== undefined && year !== workingParts.year); const bodyIsVisible = el.classList.contains('datetime-ready'); const { isGridStyle, showMonthAndYear } = this; - if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { - this.animateToDate(targetValue); - } else { - /** - * We only need to do this if we didn't just animate to a new month, - * since that calls prevMonth/nextMonth which calls setWorkingParts for us. - */ - this.setWorkingParts({ - month, - day, - year, - hour, - minute, - ampm, - }); + + let areAllSelectedDatesInSameMonth = true; + if (Array.isArray(valueToProcess)) { + const firstMonth = valueToProcess[0].month; + for (const date of valueToProcess) { + if (date.month !== firstMonth) { + areAllSelectedDatesInSameMonth = false; + break; + } + } + } + + /** + * If there is more than one date selected + * and the dates aren't all in the same month, + * then we should neither animate to the date + * nor update the working parts because we do + * not know which date the user wants to view. + */ + if (areAllSelectedDatesInSameMonth) { + if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { + this.animateToDate(targetValue); + } else { + /** + * We only need to do this if we didn't just animate to a new month, + * since that calls prevMonth/nextMonth which calls setWorkingParts for us. + */ + this.setWorkingParts({ + month, + day, + year, + hour, + minute, + ampm, + }); + } } }; diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index 7a353a4ed33..8f40761d055 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -5,7 +5,7 @@ import { configs, test } from '@utils/test/playwright'; const SINGLE_DATE = '2022-06-01'; const MULTIPLE_DATES = ['2022-06-01', '2022-06-02', '2022-06-03']; -const MULTIPLE_DATES_SEPARATE_MONTHS = ['2022-04-01', '2022-05-01', '2022-06-01']; +const MULTIPLE_DATES_SEPARATE_MONTHS = ['2022-03-01', '2022-04-01', '2022-05-01']; interface DatetimeMultipleConfig { multiple?: boolean; @@ -158,10 +158,13 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); }); - test('multiple default values across months should display at least one value', async () => { - const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); + test('should not scroll to new month when value is updated with dates in different months', async ({ page }) => { + const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES); + await datetime.evaluate((el: HTMLIonDatetimeElement) => el.value = MULTIPLE_DATES_SEPARATE_MONTHS); + await page.waitForChanges(); + const monthYear = datetime.locator('.calendar-month-year'); - await expect(monthYear).toHaveText(/April 2022/); + await expect(monthYear).toHaveText(/June 2022/); }); test('with buttons, should only update value when confirm is called', async ({ page }) => { From 9db63d1f0bb4906d740b8fccfe53e611cbfbd868 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 17 Jan 2024 13:46:08 -0600 Subject: [PATCH 2/6] lint --- core/src/components/datetime/test/multiple/datetime.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index 8f40761d055..fdf655a8e14 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -160,7 +160,7 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { test('should not scroll to new month when value is updated with dates in different months', async ({ page }) => { const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES); - await datetime.evaluate((el: HTMLIonDatetimeElement) => el.value = MULTIPLE_DATES_SEPARATE_MONTHS); + await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = MULTIPLE_DATES_SEPARATE_MONTHS)); await page.waitForChanges(); const monthYear = datetime.locator('.calendar-month-year'); From 09bc77eb3dcca2b28f35b714a1cef64340917365 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 17 Jan 2024 13:59:33 -0600 Subject: [PATCH 3/6] fix test --- core/src/components/datetime/test/multiple/datetime.e2e.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index fdf655a8e14..f577cec5304 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -160,7 +160,10 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { test('should not scroll to new month when value is updated with dates in different months', async ({ page }) => { const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES); - await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = MULTIPLE_DATES_SEPARATE_MONTHS)); + await datetime.evaluate((el: HTMLIonDatetimeElement, dates: string[]) => { + el.value = dates; + }, MULTIPLE_DATES_SEPARATE_MONTHS); + await page.waitForChanges(); const monthYear = datetime.locator('.calendar-month-year'); From 4369f2adda981eb15f2c9a86f9d6e818812a10fd Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Thu, 18 Jan 2024 10:59:42 -0600 Subject: [PATCH 4/6] add test for scrolling when values are in the same month --- .../datetime/test/multiple/datetime.e2e.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index f577cec5304..4b5974fc9d8 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -158,6 +158,18 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); }); + test('should scroll to new month when value is updated with multiple dates in the same month', async ({ page }) => { + const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); + await datetime.evaluate((el: HTMLIonDatetimeElement, dates: string[]) => { + el.value = dates; + }, MULTIPLE_DATES); + + await page.waitForChanges(); + + const monthYear = datetime.locator('.calendar-month-year'); + await expect(monthYear).toHaveText(/June 2022/); + }); + test('should not scroll to new month when value is updated with dates in different months', async ({ page }) => { const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES); await datetime.evaluate((el: HTMLIonDatetimeElement, dates: string[]) => { From f760bae381f75bdd2c9db22462d3290a10fc1ef5 Mon Sep 17 00:00:00 2001 From: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com> Date: Thu, 18 Jan 2024 12:17:01 -0600 Subject: [PATCH 5/6] Update core/src/components/datetime/test/multiple/datetime.e2e.ts Co-authored-by: Liam DeBeasi --- core/src/components/datetime/test/multiple/datetime.e2e.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index 4b5974fc9d8..26be043867a 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -159,6 +159,10 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); test('should scroll to new month when value is updated with multiple dates in the same month', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/28602', + }); const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); await datetime.evaluate((el: HTMLIonDatetimeElement, dates: string[]) => { el.value = dates; From 3356265ab1bc40d57c410c4c2e05671067e23be9 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Thu, 18 Jan 2024 12:31:47 -0600 Subject: [PATCH 6/6] lint --- .../src/components/datetime/test/multiple/datetime.e2e.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index 26be043867a..c3a195b89b6 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -159,10 +159,10 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); test('should scroll to new month when value is updated with multiple dates in the same month', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/28602', - }); + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/28602', + }); const datetime = await datetimeFixture.goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); await datetime.evaluate((el: HTMLIonDatetimeElement, dates: string[]) => { el.value = dates;