From 55ed7a04f0e36a40c3c97f80cd3aff44a8ed6e2f Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 19 Apr 2022 13:08:26 -0500 Subject: [PATCH 1/8] fix(datetime): if no default value, don't highlight active day until one is selected --- core/src/components/datetime/datetime.tsx | 7 +++- .../src/components/datetime/test/basic/e2e.ts | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index c1200432a0b..820324eb8ce 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -67,6 +67,7 @@ export class Datetime implements ComponentInterface { private popoverRef?: HTMLIonPopoverElement; private clearFocusVisible?: () => void; private overlayIsPresenting = false; + private highlightActiveParts = false; private parsedMinuteValues?: number[]; private parsedHourValues?: number[]; @@ -1058,6 +1059,7 @@ export class Datetime implements ComponentInterface { }; private processValue = (value?: string | null) => { + this.highlightActiveParts = !!value; const valueToProcess = value || getToday(); const { month, day, year, hour, minute, tzOffset } = parseDate(valueToProcess); @@ -1349,6 +1351,7 @@ 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; @@ -1424,7 +1427,7 @@ export class Datetime implements ComponentInterface { class={{ 'calendar-day-padding': day === null, 'calendar-day': true, - 'calendar-day-active': isActive, + 'calendar-day-active': isActive && highlightActiveParts, 'calendar-day-today': isToday, }} aria-selected={ariaSelected} @@ -1434,6 +1437,8 @@ export class Datetime implements ComponentInterface { return; } + this.highlightActiveParts = true; + this.setWorkingParts({ ...this.workingParts, month, diff --git a/core/src/components/datetime/test/basic/e2e.ts b/core/src/components/datetime/test/basic/e2e.ts index 81296840fdd..afe3d99634c 100644 --- a/core/src/components/datetime/test/basic/e2e.ts +++ b/core/src/components/datetime/test/basic/e2e.ts @@ -98,6 +98,39 @@ describe('datetime: selecting a day', () => { expect(newActiveDay.innerText).toEqual('13'); }); + + const testHighlight = async (page) => { + const today = new Date(); + const todayBtn = await page.find(`ion-datetime >>> .calendar-day[data-day="${today.getDate()}"][data-month="${today.getMonth() + 1}"]`); + + expect(todayBtn).toHaveClass('calendar-day-today'); + expect(todayBtn).not.toHaveClass('calendar-day-active'); + + await todayBtn.click(); + await page.waitForChanges(); + + expect(todayBtn).toHaveClass('calendar-day-active'); + }; + + it('should not highlight a day until one is selected', async () => { + const page = await newE2EPage({ + html: ` + + `, + }); + + await testHighlight(page); + }); + + it('should not highlight a day until one is selected, with default-buttons', async () => { + const page = await newE2EPage({ + html: ` + + `, + }); + + await testHighlight(page); + }); }); test('datetime:rtl: basic', async () => { From 2c652f6f04590c83ee9603e3038129c190f48e5d Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 19 Apr 2022 13:19:30 -0500 Subject: [PATCH 2/8] chore(): run prettier --- core/src/components/datetime/test/basic/e2e.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/test/basic/e2e.ts b/core/src/components/datetime/test/basic/e2e.ts index afe3d99634c..92b83f2b886 100644 --- a/core/src/components/datetime/test/basic/e2e.ts +++ b/core/src/components/datetime/test/basic/e2e.ts @@ -101,7 +101,9 @@ describe('datetime: selecting a day', () => { const testHighlight = async (page) => { const today = new Date(); - const todayBtn = await page.find(`ion-datetime >>> .calendar-day[data-day="${today.getDate()}"][data-month="${today.getMonth() + 1}"]`); + const todayBtn = await page.find( + `ion-datetime >>> .calendar-day[data-day="${today.getDate()}"][data-month="${today.getMonth() + 1}"]` + ); expect(todayBtn).toHaveClass('calendar-day-today'); expect(todayBtn).not.toHaveClass('calendar-day-active'); From c4b6eec89792c41b04406d6b4a2b1fd84e110695 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Apr 2022 14:00:56 -0500 Subject: [PATCH 3/8] test(datetime): add playwright versions of new tests --- .../datetime/test/basic/datetime.e2e.ts | 26 +++++++++++++++++++ .../components/datetime/test/basic/index.html | 5 ++++ 2 files changed, 31 insertions(+) create mode 100644 core/src/components/datetime/test/basic/datetime.e2e.ts diff --git a/core/src/components/datetime/test/basic/datetime.e2e.ts b/core/src/components/datetime/test/basic/datetime.e2e.ts new file mode 100644 index 00000000000..152105e3387 --- /dev/null +++ b/core/src/components/datetime/test/basic/datetime.e2e.ts @@ -0,0 +1,26 @@ +import { expect } from '@playwright/test'; +import type { E2EPage } from '@utils/test/playwright'; +import { test } from '@utils/test/playwright'; + +test.describe('datetime: selecting a day', () => { + const testHighlight = async (page: E2EPage, datetimeID: string) => { + const today = new Date(); + await page.goto('/src/components/datetime/test/basic'); + + const todayBtn = page.locator( + `#${datetimeID} >>> .calendar-day[data-day="${today.getDate()}"][data-month="${today.getMonth() + 1}"]` + ); + + const classes = await todayBtn.evaluate((el: any) => el.classList.value); + expect(classes).toMatch('calendar-day-today'); + expect(classes).not.toMatch('calendar-day-active'); + }; + + test('should not highlight a day until one is selected', async ({ page }) => { + await testHighlight(page, 'inline-datetime-no-value'); + }); + + test('should not highlight a day until one is selected, with default-buttons', async ({ page }) => { + await testHighlight(page, 'custom-datetime'); + }); +}); \ No newline at end of file diff --git a/core/src/components/datetime/test/basic/index.html b/core/src/components/datetime/test/basic/index.html index b918590be2b..153c85688a7 100644 --- a/core/src/components/datetime/test/basic/index.html +++ b/core/src/components/datetime/test/basic/index.html @@ -261,6 +261,11 @@

Inline

+
+

Inline - No Default Value

+ +
+

Popover

Present Popover From 9bf9f6d87434b84f17631cda899e771de7914e78 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Apr 2022 14:33:05 -0500 Subject: [PATCH 4/8] test(datetime): fix shadow root selector and add second class check --- core/src/components/datetime/test/basic/datetime.e2e.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/test/basic/datetime.e2e.ts b/core/src/components/datetime/test/basic/datetime.e2e.ts index 152105e3387..44710f1d570 100644 --- a/core/src/components/datetime/test/basic/datetime.e2e.ts +++ b/core/src/components/datetime/test/basic/datetime.e2e.ts @@ -8,12 +8,18 @@ test.describe('datetime: selecting a day', () => { await page.goto('/src/components/datetime/test/basic'); const todayBtn = page.locator( - `#${datetimeID} >>> .calendar-day[data-day="${today.getDate()}"][data-month="${today.getMonth() + 1}"]` + `#${datetimeID} .calendar-day[data-day='${today.getDate()}'][data-month='${today.getMonth() + 1}']` ); const classes = await todayBtn.evaluate((el: any) => el.classList.value); expect(classes).toMatch('calendar-day-today'); expect(classes).not.toMatch('calendar-day-active'); + + await todayBtn.click(); + await page.waitForChanges(); + + const classes2 = await todayBtn.evaluate((el: any) => el.classList.value); + expect(classes2).toMatch('calendar-day-active'); }; test('should not highlight a day until one is selected', async ({ page }) => { From 129e8af2fb9cc07c2bd032f8bb4637ded61bbdbb Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Apr 2022 14:41:34 -0500 Subject: [PATCH 5/8] test(datetime): remove duplicate Puppeteer tests --- .../src/components/datetime/test/basic/e2e.ts | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/core/src/components/datetime/test/basic/e2e.ts b/core/src/components/datetime/test/basic/e2e.ts index 92b83f2b886..81296840fdd 100644 --- a/core/src/components/datetime/test/basic/e2e.ts +++ b/core/src/components/datetime/test/basic/e2e.ts @@ -98,41 +98,6 @@ describe('datetime: selecting a day', () => { expect(newActiveDay.innerText).toEqual('13'); }); - - const testHighlight = async (page) => { - const today = new Date(); - const todayBtn = await page.find( - `ion-datetime >>> .calendar-day[data-day="${today.getDate()}"][data-month="${today.getMonth() + 1}"]` - ); - - expect(todayBtn).toHaveClass('calendar-day-today'); - expect(todayBtn).not.toHaveClass('calendar-day-active'); - - await todayBtn.click(); - await page.waitForChanges(); - - expect(todayBtn).toHaveClass('calendar-day-active'); - }; - - it('should not highlight a day until one is selected', async () => { - const page = await newE2EPage({ - html: ` - - `, - }); - - await testHighlight(page); - }); - - it('should not highlight a day until one is selected, with default-buttons', async () => { - const page = await newE2EPage({ - html: ` - - `, - }); - - await testHighlight(page); - }); }); test('datetime:rtl: basic', async () => { From e018475bd9c0f76ff15ef272b59b5e5b5e2b9513 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 25 Apr 2022 12:27:30 -0500 Subject: [PATCH 6/8] docs(datetime): add comments for highlightActiveParts --- core/src/components/datetime/datetime.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 820324eb8ce..d5143aea794 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -67,6 +67,15 @@ export class Datetime implements ComponentInterface { private popoverRef?: HTMLIonPopoverElement; private clearFocusVisible?: () => void; private overlayIsPresenting = false; + + /** + * 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[]; @@ -1437,6 +1446,12 @@ export class Datetime implements ComponentInterface { return; } + /** + * 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; this.setWorkingParts({ From 79e7f4f5c415f76b55711d4001a748135c0e7e7b Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 25 Apr 2022 12:28:01 -0500 Subject: [PATCH 7/8] test(datetime): use toHaveClass util instead of manual eval --- core/src/components/datetime/test/basic/datetime.e2e.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/components/datetime/test/basic/datetime.e2e.ts b/core/src/components/datetime/test/basic/datetime.e2e.ts index 44710f1d570..d5314cd886c 100644 --- a/core/src/components/datetime/test/basic/datetime.e2e.ts +++ b/core/src/components/datetime/test/basic/datetime.e2e.ts @@ -11,15 +11,13 @@ test.describe('datetime: selecting a day', () => { `#${datetimeID} .calendar-day[data-day='${today.getDate()}'][data-month='${today.getMonth() + 1}']` ); - const classes = await todayBtn.evaluate((el: any) => el.classList.value); - expect(classes).toMatch('calendar-day-today'); - expect(classes).not.toMatch('calendar-day-active'); + expect(todayBtn).toHaveClass(/calendar-day-today/); + expect(todayBtn).not.toHaveClass(/calendar-day-active/); await todayBtn.click(); await page.waitForChanges(); - const classes2 = await todayBtn.evaluate((el: any) => el.classList.value); - expect(classes2).toMatch('calendar-day-active'); + expect(todayBtn).toHaveClass(/calendar-day-active/); }; test('should not highlight a day until one is selected', async ({ page }) => { From 07cca5894481c219206c60dc5392f22e079302c5 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 25 Apr 2022 12:29:38 -0500 Subject: [PATCH 8/8] chore(): run prettier --- core/src/components/datetime/test/basic/datetime.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/components/datetime/test/basic/datetime.e2e.ts b/core/src/components/datetime/test/basic/datetime.e2e.ts index d5314cd886c..8b2e5efe451 100644 --- a/core/src/components/datetime/test/basic/datetime.e2e.ts +++ b/core/src/components/datetime/test/basic/datetime.e2e.ts @@ -27,4 +27,4 @@ test.describe('datetime: selecting a day', () => { test('should not highlight a day until one is selected, with default-buttons', async ({ page }) => { await testHighlight(page, 'custom-datetime'); }); -}); \ No newline at end of file +});