From 0a278c2ecb1a85231eb809bb304f8087578ae304 Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Tue, 11 Jun 2024 17:43:51 -0400 Subject: [PATCH 1/4] fix(datetime): set initial value with multiple dates selected --- core/src/components/datetime/datetime.tsx | 53 ++++++++--------------- 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 5be7040e7e0..5f6898ba711 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1312,41 +1312,24 @@ export class Datetime implements ComponentInterface { const bodyIsVisible = el.classList.contains('datetime-ready'); const { isGridStyle, showMonthAndYear } = this; - 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, - }); - } + if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { + /** + * Animate to the target date, either the first value in the array or the single value. + */ + 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, + }); } }; From 0a89e664d1941101322c8daf3bde5bff1b03390e Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Wed, 12 Jun 2024 00:16:49 -0400 Subject: [PATCH 2/4] test(datetime): set the working parts to the first selected value --- .../components/datetime/test/multiple/datetime.e2e.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index 0e2c0efb381..8439008ebd6 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -311,4 +311,13 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await expect(header).toHaveText('Mon, Oct 10'); }); }); + + test.describe('with selected days in different months', () => { + test('set the working parts to the first selected value', async ({ page }) => { + const datetime = await new DatetimeMultipleFixture(page).goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); + const monthYear = datetime.locator('.calendar-month-year'); + + await expect(monthYear).toHaveText(/March 2022/); + }); + }); }); From fc5115c9a9aedc490c1ba79833d4d03df4b5cdcc Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Wed, 12 Jun 2024 00:55:14 -0400 Subject: [PATCH 3/4] fix: set active month view to the latest value --- core/src/components/datetime/datetime.tsx | 32 +++++++------------ .../datetime/test/multiple/datetime.e2e.ts | 22 +++++++++++-- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 5f6898ba711..f1e088f34d3 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1262,21 +1262,20 @@ export class Datetime implements ComponentInterface { } /** - * If there are multiple values, pick an arbitrary one to clamp to. This way, - * if the values are across months, we always show at least one of them. Note - * that the values don't necessarily have to be in order. + * If there are multiple values, clamp to the last one. + * This is because the last value is the one that the user + * has most recently interacted with. */ - const singleValue = Array.isArray(valueToProcess) ? valueToProcess[0] : valueToProcess; + const singleValue = Array.isArray(valueToProcess) ? valueToProcess[valueToProcess.length - 1] : valueToProcess; const targetValue = clampDate(singleValue, minParts, maxParts); const { month, day, year, hour, minute } = targetValue; const ampm = parseAmPm(hour!); /** - * Since `activeParts` indicates a value that - * been explicitly selected either by the - * user or the app, only update `activeParts` - * if the `value` property is set. + * Since `activeParts` indicates a value that been explicitly selected + * either by the user or the app, only update `activeParts` if the + * `value` property is set. */ if (hasValue) { if (Array.isArray(valueToProcess)) { @@ -1300,13 +1299,6 @@ export class Datetime implements ComponentInterface { this.activeParts = []; } - /** - * Only animate if: - * 1. We're using grid style (wheel style pickers should just jump to new value) - * 2. The month and/or year actually changed, and both are defined (otherwise there's nothing to animate to) - * 3. The calendar body is visible (prevents animation when in collapsed datetime-button, for example) - * 4. The month/year picker is not open (since you wouldn't see the animation anyway) - */ const didChangeMonth = (month !== undefined && month !== workingParts.month) || (year !== undefined && year !== workingParts.year); const bodyIsVisible = el.classList.contains('datetime-ready'); @@ -1314,14 +1306,14 @@ export class Datetime implements ComponentInterface { if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { /** - * Animate to the target date, either the first value in the array or the single value. + * Only animate if: + * 1. We're using grid style (wheel style pickers should just jump to new value) + * 2. The month and/or year actually changed, and both are defined (otherwise there's nothing to animate to) + * 3. The calendar body is visible (prevents animation when in collapsed datetime-button, for example) + * 4. The month/year picker is not open (since you wouldn't see the animation anyway) */ 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, diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index 8439008ebd6..cf0a4b3258d 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -313,11 +313,27 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); test.describe('with selected days in different months', () => { - test('set the working parts to the first selected value', async ({ page }) => { + test(`set the active month view to the latest value's month`, async ({ page }) => { const datetime = await new DatetimeMultipleFixture(page).goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); - const monthYear = datetime.locator('.calendar-month-year'); + const calendarMonthYear = datetime.locator('.calendar-month-year'); + + await expect(calendarMonthYear).toHaveText(/May 2022/); + }); + + test('does not change the active month view when selecting a day in a different month', async ({ page }) => { + const datetime = await new DatetimeMultipleFixture(page).goto(config, MULTIPLE_DATES_SEPARATE_MONTHS); + const nextButton = page.locator('.calendar-next-prev ion-button:nth-child(2)'); + const calendarMonthYear = datetime.locator('.calendar-month-year'); + + await nextButton.click(); + + await expect(calendarMonthYear).toHaveText(/June 2022/); + + const june8Button = datetime.locator('[data-month="6"][data-day="8"]'); + + await june8Button.click(); - await expect(monthYear).toHaveText(/March 2022/); + await expect(calendarMonthYear).toHaveText(/June 2022/); }); }); }); From 3ba19fed68559c7fbd1f978971c77f0c97f6a69b Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Wed, 12 Jun 2024 01:32:17 -0400 Subject: [PATCH 4/4] test: remove invalid test case --- .../datetime/test/multiple/datetime.e2e.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/core/src/components/datetime/test/multiple/datetime.e2e.ts b/core/src/components/datetime/test/multiple/datetime.e2e.ts index cf0a4b3258d..54121aba1e3 100644 --- a/core/src/components/datetime/test/multiple/datetime.e2e.ts +++ b/core/src/components/datetime/test/multiple/datetime.e2e.ts @@ -174,18 +174,6 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { 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[]) => { - el.value = dates; - }, MULTIPLE_DATES_SEPARATE_MONTHS); - - await page.waitForChanges(); - - const monthYear = datetime.locator('.calendar-month-year'); - await expect(monthYear).toHaveText(/June 2022/); - }); - test('with buttons, should only update value when confirm is called', async ({ page }) => { const datetime = await datetimeFixture.goto(config, SINGLE_DATE, { showDefaultButtons: true }); const june2Button = datetime.locator('[data-month="6"][data-day="2"]');