From 00b94a5d00879faeecda4d6e94f03577e4071b83 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 14:26:00 +0000 Subject: [PATCH 01/10] test(datetime): add failing test --- .../datetime/test/set-value/datetime.e2e.ts | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/core/src/components/datetime/test/set-value/datetime.e2e.ts b/core/src/components/datetime/test/set-value/datetime.e2e.ts index 3c5fc86f222..fb23bded7ae 100644 --- a/core/src/components/datetime/test/set-value/datetime.e2e.ts +++ b/core/src/components/datetime/test/set-value/datetime.e2e.ts @@ -2,11 +2,13 @@ import { expect } from '@playwright/test'; import { test } from '@utils/test/playwright'; test.describe('datetime: set-value', () => { - test.beforeEach(async ({ page }) => { + test.beforeEach(async ({ skip }) => { + skip.rtl(); + }); + test('should update the active date when value is initially set', async ({ page }) => { await page.goto('/src/components/datetime/test/set-value'); await page.waitForSelector('.datetime-ready'); - }); - test('should update the active date', async ({ page }) => { + const datetime = page.locator('ion-datetime'); await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-11-25T12:40:00.000Z')); @@ -15,7 +17,10 @@ test.describe('datetime: set-value', () => { const activeDate = page.locator('ion-datetime .calendar-day-active'); await expect(activeDate).toHaveText('25'); }); - test('should update the active time', async ({ page }) => { + test('should update the active time when value is initially set', async ({ page }) => { + await page.goto('/src/components/datetime/test/set-value'); + await page.waitForSelector('.datetime-ready'); + const datetime = page.locator('ion-datetime'); await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-11-25T12:40:00.000Z')); @@ -24,4 +29,34 @@ test.describe('datetime: set-value', () => { const activeDate = page.locator('ion-datetime .time-body'); await expect(activeDate).toHaveText('12:40 PM'); }); + test.only('should update active item when value is not initially set', async ({ page }) => { + await page.setContent(` + + `); + await page.waitForSelector('.datetime-ready'); + + const datetime = page.locator('ion-datetime'); + const activeDayButton = page.locator('.calendar-day-active'); + const monthYearButton = page.locator('.calendar-month-year'); + const monthColumn = page.locator('.month-column'); + const yearColumn = page.locator('.year-column'); + + await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-10-05')); + + await monthYearButton.click(); + await page.waitForChanges(); + + await monthColumn.locator('.picker-item[data-value="10"]').click(); + await page.waitForChanges(); + + await yearColumn.locator('.picker-item[data-value="2021"]').click(); + await page.waitForChanges(); + + await monthYearButton.click(); + await page.waitForChanges(); + + await expect(activeDayButton).toHaveAttribute('data-day', '5'); + await expect(activeDayButton).toHaveAttribute('data-month', '10'); + await expect(activeDayButton).toHaveAttribute('data-year', '2021'); + }); }); From 33f8ce6fa9049e75ebf588b2f0f8a4635ef32392 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 14:29:00 +0000 Subject: [PATCH 02/10] refactor(datetime): activeParts now defaults to empty arrray --- core/src/components/datetime/datetime.tsx | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 46db79e0d17..f78cf2b115b 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -114,18 +114,11 @@ export class Datetime implements ComponentInterface { * Duplicate reference to `activeParts` that does not trigger a re-render of the component. * Allows caching an instance of the `activeParts` in between render cycles. */ - private activePartsClone!: DatetimeParts | DatetimeParts[]; + private activePartsClone: DatetimeParts | DatetimeParts[] = []; @State() showMonthAndYear = false; - @State() activeParts: DatetimeParts | DatetimeParts[] = { - month: 5, - day: 28, - year: 2021, - hour: 13, - minute: 52, - ampm: 'pm', - }; + @State() activeParts: DatetimeParts | DatetimeParts[] = []; @State() workingParts: DatetimeParts = { month: 5, From 2f64c3593eed469cb2ba969e07948ad27043c326 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 14:30:13 +0000 Subject: [PATCH 03/10] refactor(datetime): only update active parts if value is set --- core/src/components/datetime/datetime.tsx | 32 ++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index f78cf2b115b..a1db97bda90 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1205,18 +1205,26 @@ export class Datetime implements ComponentInterface { ampm, }); - if (Array.isArray(valueToProcess)) { - this.activeParts = [...valueToProcess]; - } else { - this.activeParts = { - month, - day, - year, - hour, - minute, - tzOffset, - ampm, - }; + /** + * 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)) { + this.activeParts = [...valueToProcess]; + } else { + this.activeParts = { + month, + day, + year, + hour, + minute, + tzOffset, + ampm, + }; + } } }; From 3cc915326fb9bb64f49b675aca8305fab5bbfac0 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 14:41:12 +0000 Subject: [PATCH 04/10] refactor(datetime): remove highightActiveParts --- core/src/components/datetime/datetime.tsx | 50 ++++------------------- 1 file changed, 7 insertions(+), 43 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index a1db97bda90..98cb2fc5803 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -84,17 +84,6 @@ export class Datetime implements ComponentInterface { private calendarBodyRef?: HTMLElement; private popoverRef?: HTMLIonPopoverElement; private clearFocusVisible?: () => void; - - /** - * Whether to highlight the active day with a solid circle (as opposed - * to the outline circle around today). If you don't specify an initial - * value for the datetime, it doesn't automatically init to a default to - * avoid unwanted change events firing. If the solid circle were still - * shown then, it would look like a date had already been selected, which - * is misleading UX. - */ - private highlightActiveParts = false; - private parsedMinuteValues?: number[]; private parsedHourValues?: number[]; private parsedMonthValues?: number[]; @@ -491,16 +480,12 @@ export class Datetime implements ComponentInterface { */ @Method() async confirm(closeOverlay = false) { - const { highlightActiveParts, isCalendarPicker, activeParts } = this; + const { isCalendarPicker, activeParts } = this; /** - * We only update the value if the presentation is not a calendar picker, - * or if `highlightActiveParts` is true; indicating that the user - * has selected a date from the calendar picker. - * - * Otherwise "today" would accidentally be set as the value. + * We only update the value if the presentation is not a calendar picker. */ - if (highlightActiveParts || !isCalendarPicker) { + if (activeParts !== undefined || !isCalendarPicker) { const activePartsIsArray = Array.isArray(activeParts); if (activePartsIsArray && activeParts.length === 0) { this.value = undefined; @@ -575,7 +560,7 @@ export class Datetime implements ComponentInterface { }; private setActiveParts = (parts: DatetimeParts, removeDate = false) => { - const { multiple, activePartsClone, highlightActiveParts } = this; + const { multiple, activePartsClone } = this; /** * When setting the active parts, it is possible @@ -603,15 +588,8 @@ export class Datetime implements ComponentInterface { const activePartsArray = Array.isArray(activePartsClone) ? activePartsClone : [activePartsClone]; if (removeDate) { this.activeParts = activePartsArray.filter((p) => !isSameDay(p, validatedParts)); - } else if (highlightActiveParts) { - this.activeParts = [...activePartsArray, validatedParts]; } else { - /** - * If highlightActiveParts is false, that means we just have a - * default value of today in activeParts; we need to replace that - * rather than adding to it since it's just a placeholder. - */ - this.activeParts = [validatedParts]; + this.activeParts = [...activePartsArray, validatedParts]; } } else { this.activeParts = { @@ -619,18 +597,6 @@ export class Datetime implements ComponentInterface { }; } - /** - * Now that the user has interacted somehow to select something, we can - * show the solid highlight. This needs to be done after checking it above, - * but before the confirm call below. - * - * Note that for datetimes with confirm/cancel buttons, the value - * isn't updated until you call confirm(). We need to bring the - * solid circle back on day click for UX reasons, rather than only - * show the circle if `value` is truthy. - */ - this.highlightActiveParts = true; - const hasSlottedButtons = this.el.querySelector('[slot="buttons"]') !== null; if (hasSlottedButtons || this.showDefaultButtons) { return; @@ -1164,7 +1130,6 @@ export class Datetime implements ComponentInterface { private processValue = (value?: string | string[] | null) => { const hasValue = !!value; - this.highlightActiveParts = hasValue; let valueToProcess = parseDate(value || getToday()); const { minParts, maxParts, multiple } = this; @@ -1892,7 +1857,6 @@ export class Datetime implements ComponentInterface { ); } private renderMonth(month: number, year: number) { - const { highlightActiveParts } = this; const yearAllowed = this.parsedYearValues === undefined || this.parsedYearValues.includes(year); const monthAllowed = this.parsedMonthValues === undefined || this.parsedMonthValues.includes(month); const isCalMonthDisabled = !yearAllowed || !monthAllowed; @@ -1970,7 +1934,7 @@ export class Datetime implements ComponentInterface { class={{ 'calendar-day-padding': day === null, 'calendar-day': true, - 'calendar-day-active': isActive && highlightActiveParts, + 'calendar-day-active': isActive, 'calendar-day-today': isToday, }} aria-selected={ariaSelected} @@ -1995,7 +1959,7 @@ export class Datetime implements ComponentInterface { day, year, }, - isActive && highlightActiveParts + isActive ); } else { this.setActiveParts({ From cfa1b2442f044346121eb5a916dfe2f4590e9637 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 14:45:59 +0000 Subject: [PATCH 05/10] chore(): remove only --- core/src/components/datetime/test/set-value/datetime.e2e.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/test/set-value/datetime.e2e.ts b/core/src/components/datetime/test/set-value/datetime.e2e.ts index fb23bded7ae..ab98bc5ab40 100644 --- a/core/src/components/datetime/test/set-value/datetime.e2e.ts +++ b/core/src/components/datetime/test/set-value/datetime.e2e.ts @@ -29,7 +29,7 @@ test.describe('datetime: set-value', () => { const activeDate = page.locator('ion-datetime .time-body'); await expect(activeDate).toHaveText('12:40 PM'); }); - test.only('should update active item when value is not initially set', async ({ page }) => { + test('should update active item when value is not initially set', async ({ page }) => { await page.setContent(` `); @@ -43,18 +43,22 @@ test.describe('datetime: set-value', () => { await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-10-05')); + // Open month/year picker await monthYearButton.click(); await page.waitForChanges(); + // Select October 2021 await monthColumn.locator('.picker-item[data-value="10"]').click(); await page.waitForChanges(); await yearColumn.locator('.picker-item[data-value="2021"]').click(); await page.waitForChanges(); + // Close month/year picker await monthYearButton.click(); await page.waitForChanges(); + // Check that correct day is highlighted await expect(activeDayButton).toHaveAttribute('data-day', '5'); await expect(activeDayButton).toHaveAttribute('data-month', '10'); await expect(activeDayButton).toHaveAttribute('data-year', '2021'); From 531ec8335a723e97f5883ac15b46c309b29ba4ba Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 14:46:13 +0000 Subject: [PATCH 06/10] fix(datetime): header now renders correctly --- core/src/components/datetime/datetime.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 98cb2fc5803..69961b40fd8 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -2075,13 +2075,21 @@ export class Datetime implements ComponentInterface { return; } + const { activeParts, todayParts } = this; + + /** + * Either render the active part or + * fall back to today's date. + */ + const activePart = Array.isArray(activeParts) ? activeParts[0] : activeParts; + return (
Select Date
{mode === 'md' && !this.multiple && ( -
{getMonthAndDay(this.locale, this.activeParts as DatetimeParts)}
+
{getMonthAndDay(this.locale, activePart || todayParts)}
)}
); From ea6aadb1d051d79ffa79e4e962447fca50dda61f Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 15:18:02 +0000 Subject: [PATCH 07/10] refactor(datetime): add default part logic --- core/src/components/datetime/datetime.tsx | 61 +++++++++++++-------- core/src/components/datetime/utils/parse.ts | 1 + 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 69961b40fd8..f8b840fd017 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -543,6 +543,23 @@ export class Datetime implements ComponentInterface { } } + /** + * Returns the DatetimePart interface + * to use when rendering an initial set of + * data. This should be used when rendering an + * interface in an environment where the `value` + * may not be set. This function works + * by returning the first selected date in + * "activePartsClone" and then falling back to + * today's DatetimeParts if no active date is selected. + */ + private getDefaultPart = () => { + const { activePartsClone, todayParts } = this; + + const firstPart = Array.isArray(activePartsClone) ? activePartsClone[0] : activePartsClone; + return firstPart || todayParts; + }; + private closeParentOverlay = () => { const popoverOrModal = this.el.closest('ion-modal, ion-popover') as | HTMLIonModalElement @@ -1703,13 +1720,15 @@ export class Datetime implements ComponentInterface { } private renderHourPickerColumn(hoursData: PickerColumnItem[]) { - const { workingParts, activePartsClone } = this; + const { workingParts } = this; if (hoursData.length === 0) return []; + const activePart = this.getDefaultPart(); + return ( { @@ -1718,9 +1737,9 @@ export class Datetime implements ComponentInterface { hour: ev.detail.value, }); - if (!Array.isArray(activePartsClone)) { + if (!Array.isArray(activePart)) { this.setActiveParts({ - ...activePartsClone, + ...activePart, hour: ev.detail.value, }); } @@ -1731,13 +1750,15 @@ export class Datetime implements ComponentInterface { ); } private renderMinutePickerColumn(minutesData: PickerColumnItem[]) { - const { workingParts, activePartsClone } = this; + const { workingParts } = this; if (minutesData.length === 0) return []; + const activePart = this.getDefaultPart(); + return ( { @@ -1746,9 +1767,9 @@ export class Datetime implements ComponentInterface { minute: ev.detail.value, }); - if (!Array.isArray(activePartsClone)) { + if (!Array.isArray(activePart)) { this.setActiveParts({ - ...activePartsClone, + ...activePart, minute: ev.detail.value, }); } @@ -1759,18 +1780,20 @@ export class Datetime implements ComponentInterface { ); } private renderDayPeriodPickerColumn(dayPeriodData: PickerColumnItem[]) { - const { workingParts, activePartsClone } = this; + const { workingParts } = this; if (dayPeriodData.length === 0) { return []; } + const activePart = this.getDefaultPart(); const isDayPeriodRTL = isLocaleDayPeriodRTL(this.locale); + console.log(activePart); return ( { const hour = calculateHourFromAMPM(workingParts, ev.detail.value); @@ -1781,9 +1804,9 @@ export class Datetime implements ComponentInterface { hour, }); - if (!Array.isArray(activePartsClone)) { + if (!Array.isArray(activePart)) { this.setActiveParts({ - ...activePartsClone, + ...activePart, ampm: ev.detail.value, hour, }); @@ -2007,6 +2030,8 @@ export class Datetime implements ComponentInterface { private renderTimeOverlay() { const use24Hour = is24Hour(this.locale, this.hourCycle); + const activePart = this.getDefaultPart(); + return [
{this.renderTimeLabel()}
, ,
Select Date
{mode === 'md' && !this.multiple && ( -
{getMonthAndDay(this.locale, activePart || todayParts)}
+
{getMonthAndDay(this.locale, this.getDefaultPart())}
)} ); diff --git a/core/src/components/datetime/utils/parse.ts b/core/src/components/datetime/utils/parse.ts index ebfe2a94eaa..95c05b9be68 100644 --- a/core/src/components/datetime/utils/parse.ts +++ b/core/src/components/datetime/utils/parse.ts @@ -115,6 +115,7 @@ export function parseDate(val: string | string[] | undefined | null): DatetimePa hour: parse[4], minute: parse[5], tzOffset, + ampm: parse[4] < 12 ? 'am' : 'pm', }; } From 0907235c57d975197e81855cf8ed55c2ec0a2471 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 16:05:03 +0000 Subject: [PATCH 08/10] chore(): use getDefaultPart --- core/src/components/datetime/datetime.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 9bf90573470..d90f7ac47f6 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -2126,7 +2126,7 @@ export class Datetime implements ComponentInterface { } } else { // for exactly 1 day selected (multiple set or not), show a formatted version of that - headerText = getMonthAndDay(this.locale, isArray ? activeParts[0] : activeParts); + headerText = getMonthAndDay(this.locale, this.getDefaultPart()); } return ( From d961808ab00558958e009ec1ec0320fdc96cfdcd Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 16:11:14 +0000 Subject: [PATCH 09/10] chore(): lint --- core/src/components/datetime/datetime.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index d90f7ac47f6..981f0b5911c 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -565,7 +565,7 @@ export class Datetime implements ComponentInterface { const { activePartsClone, todayParts } = this; const firstPart = Array.isArray(activePartsClone) ? activePartsClone[0] : activePartsClone; - return firstPart || todayParts; + return firstPart ?? todayParts; }; private closeParentOverlay = () => { From 8e5a58521d5fe779b78cce77b441f8a70a1b6375 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 4 Oct 2022 16:22:13 +0000 Subject: [PATCH 10/10] chore(): remove log --- core/src/components/datetime/datetime.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 981f0b5911c..23bc63fee86 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1798,7 +1798,6 @@ export class Datetime implements ComponentInterface { const activePart = this.getDefaultPart(); const isDayPeriodRTL = isLocaleDayPeriodRTL(this.locale); - console.log(activePart); return (