From 0548fe858854f0187e0dfe00efaec142cd5bb6cf Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Mon, 17 Oct 2022 13:19:02 -0500 Subject: [PATCH] fix(datetime): values are adjusted to be in bounds (#26125) resolves #25894, resolves #25708 --- core/src/components/datetime/datetime.tsx | 4 +- .../datetime/test/manipulation.spec.ts | 70 +++++++ .../datetime/test/minmax/datetime.e2e.ts | 49 +++++ .../components/datetime/utils/manipulation.ts | 68 ++++++- .../picker-column-internal.tsx | 64 ------- .../picker-column-internal.e2e.ts | 175 ------------------ 6 files changed, 188 insertions(+), 242 deletions(-) delete mode 100644 core/src/components/picker-column-internal/test/update-items/picker-column-internal.e2e.ts diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index f2a8a10a07d..34ee4023a13 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -585,7 +585,7 @@ export class Datetime implements ComponentInterface { }; private setActiveParts = (parts: DatetimeParts, removeDate = false) => { - const { multiple, activePartsClone } = this; + const { multiple, minParts, maxParts, activePartsClone } = this; /** * When setting the active parts, it is possible @@ -597,7 +597,7 @@ export class Datetime implements ComponentInterface { * Additionally, we need to update the working parts * too in the event that the validated parts are different. */ - const validatedParts = validateParts(parts); + const validatedParts = validateParts(parts, minParts, maxParts); this.setWorkingParts(validatedParts); if (multiple) { diff --git a/core/src/components/datetime/test/manipulation.spec.ts b/core/src/components/datetime/test/manipulation.spec.ts index 864ba4b775a..f2c81882953 100644 --- a/core/src/components/datetime/test/manipulation.spec.ts +++ b/core/src/components/datetime/test/manipulation.spec.ts @@ -14,6 +14,7 @@ import { calculateHourFromAMPM, subtractDays, addDays, + validateParts, } from '../utils/manipulation'; describe('addDays()', () => { @@ -487,3 +488,72 @@ describe('getPreviousYear()', () => { }); }); }); + +describe('validateParts()', () => { + it('should move day in bounds', () => { + expect(validateParts({ month: 2, day: 31, year: 2022, hour: 8, minute: 0 })).toEqual({ + month: 2, + day: 28, + year: 2022, + hour: 8, + minute: 0, + }); + }); + it('should move the hour back in bounds according to the min', () => { + expect( + validateParts( + { month: 1, day: 1, year: 2022, hour: 8, minute: 0 }, + { month: 1, day: 1, year: 2022, hour: 9, minute: 0 } + ) + ).toEqual({ month: 1, day: 1, year: 2022, hour: 9, minute: 0 }); + }); + it('should move the minute back in bounds according to the min', () => { + expect( + validateParts( + { month: 1, day: 1, year: 2022, hour: 9, minute: 20 }, + { month: 1, day: 1, year: 2022, hour: 9, minute: 30 } + ) + ).toEqual({ month: 1, day: 1, year: 2022, hour: 9, minute: 30 }); + }); + it('should move the hour and minute back in bounds according to the min', () => { + expect( + validateParts( + { month: 1, day: 1, year: 2022, hour: 8, minute: 30 }, + { month: 1, day: 1, year: 2022, hour: 9, minute: 0 } + ) + ).toEqual({ month: 1, day: 1, year: 2022, hour: 9, minute: 0 }); + }); + it('should move the hour back in bounds according to the max', () => { + expect( + validateParts({ month: 1, day: 1, year: 2022, hour: 10, minute: 0 }, undefined, { + month: 1, + day: 1, + year: 2022, + hour: 9, + minute: 0, + }) + ).toEqual({ month: 1, day: 1, year: 2022, hour: 9, minute: 0 }); + }); + it('should move the minute back in bounds according to the max', () => { + expect( + validateParts({ month: 1, day: 1, year: 2022, hour: 9, minute: 40 }, undefined, { + month: 1, + day: 1, + year: 2022, + hour: 9, + minute: 30, + }) + ).toEqual({ month: 1, day: 1, year: 2022, hour: 9, minute: 30 }); + }); + it('should move the hour and minute back in bounds according to the max', () => { + expect( + validateParts({ month: 1, day: 1, year: 2022, hour: 10, minute: 20 }, undefined, { + month: 1, + day: 1, + year: 2022, + hour: 9, + minute: 30, + }) + ).toEqual({ month: 1, day: 1, year: 2022, hour: 9, minute: 30 }); + }); +}); diff --git a/core/src/components/datetime/test/minmax/datetime.e2e.ts b/core/src/components/datetime/test/minmax/datetime.e2e.ts index 7cc7207a19a..27e39281bb0 100644 --- a/core/src/components/datetime/test/minmax/datetime.e2e.ts +++ b/core/src/components/datetime/test/minmax/datetime.e2e.ts @@ -234,4 +234,53 @@ test.describe('datetime: minmax', () => { ); await expect(hourPickerItems).toHaveText(['12', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11']); }); + + test.describe('minmax value adjustment when out of bounds', () => { + test.beforeEach(({ skip }) => { + skip.rtl(); + skip.mode('ios', 'This implementation is the same across modes.'); + }); + test('should reset to min time if out of bounds', async ({ page }) => { + await page.setContent(` + + `); + + await page.waitForSelector('.datetime-ready'); + + const datetime = page.locator('ion-datetime'); + const ionChange = await page.spyOnEvent('ionChange'); + const dayButton = page.locator('ion-datetime .calendar-day[data-day="10"][data-month="10"][data-year="2022"]'); + await dayButton.click(); + + await ionChange.next(); + + const value = await datetime.evaluate((el: HTMLIonDatetimeElement) => el.value); + await expect(typeof value).toBe('string'); + await expect(value!.includes('2022-10-10T08:00')).toBe(true); + }); + test('should reset to max time if out of bounds', async ({ page }) => { + await page.setContent(` + + `); + + await page.waitForSelector('.datetime-ready'); + + const datetime = page.locator('ion-datetime'); + const ionChange = await page.spyOnEvent('ionChange'); + const dayButton = page.locator('ion-datetime .calendar-day[data-day="10"][data-month="10"][data-year="2022"]'); + await dayButton.click(); + + await ionChange.next(); + + const value = await datetime.evaluate((el: HTMLIonDatetimeElement) => el.value); + await expect(typeof value).toBe('string'); + await expect(value!.includes('2022-10-10T08:00')).toBe(true); + }); + }); }); diff --git a/core/src/components/datetime/utils/manipulation.ts b/core/src/components/datetime/utils/manipulation.ts index 98f25750af7..34ac4af7d45 100644 --- a/core/src/components/datetime/utils/manipulation.ts +++ b/core/src/components/datetime/utils/manipulation.ts @@ -1,5 +1,6 @@ import type { DatetimeParts } from '../datetime-interface'; +import { isSameDay } from './comparison'; import { getNumDaysInMonth } from './helpers'; const twoDigit = (val: number | undefined): string => { @@ -345,7 +346,11 @@ export const calculateHourFromAMPM = (currentParts: DatetimeParts, newAMPM: 'am' * values are valid. For days that do not exist, * the closest valid day is used. */ -export const validateParts = (parts: DatetimeParts): DatetimeParts => { +export const validateParts = ( + parts: DatetimeParts, + minParts?: DatetimeParts, + maxParts?: DatetimeParts +): DatetimeParts => { const { month, day, year } = parts; const partsCopy = { ...parts }; @@ -361,5 +366,66 @@ export const validateParts = (parts: DatetimeParts): DatetimeParts => { partsCopy.day = numDays; } + /** + * If value is same day as min day, + * make sure the time value is in bounds. + */ + if (minParts !== undefined && isSameDay(partsCopy, minParts)) { + /** + * If the hour is out of bounds, + * update both the hour and minute. + * This is done so that the new time + * is closest to what the user selected. + */ + if (partsCopy.hour !== undefined && minParts.hour !== undefined) { + if (partsCopy.hour < minParts.hour) { + partsCopy.hour = minParts.hour; + partsCopy.minute = minParts.minute; + + /** + * If only the minute is out of bounds, + * set it to the min minute. + */ + } else if ( + partsCopy.hour === minParts.hour && + partsCopy.minute !== undefined && + minParts.minute !== undefined && + partsCopy.minute < minParts.minute + ) { + partsCopy.minute = minParts.minute; + } + } + } + + /** + * If value is same day as max day, + * make sure the time value is in bounds. + */ + if (maxParts !== undefined && isSameDay(parts, maxParts)) { + /** + * If the hour is out of bounds, + * update both the hour and minute. + * This is done so that the new time + * is closest to what the user selected. + */ + if (partsCopy.hour !== undefined && maxParts.hour !== undefined) { + if (partsCopy.hour > maxParts.hour) { + partsCopy.hour = maxParts.hour; + partsCopy.minute = maxParts.minute; + /** + * If only the minute is out of bounds, + * set it to the max minute. + */ + } else if ( + partsCopy.hour === maxParts.hour && + partsCopy.minute !== undefined && + maxParts.minute !== undefined && + partsCopy.minute > maxParts.minute + ) { + partsCopy.minute = maxParts.minute; + } + } + } + return partsCopy; }; diff --git a/core/src/components/picker-column-internal/picker-column-internal.tsx b/core/src/components/picker-column-internal/picker-column-internal.tsx index 64daf337c81..53996b4eeb0 100644 --- a/core/src/components/picker-column-internal/picker-column-internal.tsx +++ b/core/src/components/picker-column-internal/picker-column-internal.tsx @@ -36,70 +36,6 @@ export class PickerColumnInternal implements ComponentInterface { * A list of options to be displayed in the picker */ @Prop() items: PickerColumnItem[] = []; - @Watch('items') - itemsChange(currentItems: PickerColumnItem[], previousItems: PickerColumnItem[]) { - const { value } = this; - - /** - * When the items change, it is possible for the item - * that was selected to no longer exist. In that case, we need - * to automatically select the nearest item. If we do not, - * then the scroll position will be reset to zero and it will - * look like the first item was automatically selected. - * - * If we cannot find a closest item then we do nothing, and - * the browser will reset the scroll position to 0. - */ - const findCurrentItem = currentItems.find((item) => item.value === value); - if (!findCurrentItem) { - /** - * The default behavior is to assume - * that the new set of data is similar to the old - * set of data, just with some items filtered out. - * We walk backwards through the data to find the - * closest enabled picker item and select it. - * - * Developers can also swap the items out for an entirely - * new set of data. In that case, the value we select - * here likely will not make much sense. For this use case, - * developers should update the `value` prop themselves - * when swapping out the data. - */ - const findPreviousItemIndex = previousItems.findIndex((item) => item.value === value); - if (findPreviousItemIndex === -1) { - return; - } - - /** - * Step through the current items backwards - * until we find a neighbor we can select. - * We start at the last known location of the - * current selected item in order to - * account for data that has been added. This - * search prioritizes stability in that it - * tries to keep the scroll position as close - * to where it was before the update. - * Before Items: ['a', 'b', 'c'], Selected Value: 'b' - * After Items: ['a', 'dog', 'c'] - * Even though 'dog' is a different item than 'b', - * it is the closest item we can select while - * preserving the scroll position. - */ - let nearestItem; - for (let i = findPreviousItemIndex; i >= 0; i--) { - const item = currentItems[i]; - if (item !== undefined && item.disabled !== true) { - nearestItem = item; - break; - } - } - - if (nearestItem) { - this.setValue(nearestItem.value); - return; - } - } - } /** * The selected option in the picker. diff --git a/core/src/components/picker-column-internal/test/update-items/picker-column-internal.e2e.ts b/core/src/components/picker-column-internal/test/update-items/picker-column-internal.e2e.ts deleted file mode 100644 index 4ca337ab38f..00000000000 --- a/core/src/components/picker-column-internal/test/update-items/picker-column-internal.e2e.ts +++ /dev/null @@ -1,175 +0,0 @@ -import { expect } from '@playwright/test'; -import { test } from '@utils/test/playwright'; - -test.describe('picker-column-internal: updating items', () => { - test('should select nearest neighbor when updating items', async ({ page }) => { - await page.setContent(` - - - - - - `); - - const pickerColumn = page.locator('ion-picker-column-internal'); - await expect(pickerColumn).toHaveJSProperty('value', 5); - - await pickerColumn.evaluate((el: HTMLIonPickerColumnInternalElement) => { - el.items = [ - { text: '1', value: 1 }, - { text: '2', value: 2 }, - ]; - }); - - await page.waitForChanges(); - await expect(pickerColumn).toHaveJSProperty('value', 2); - }); - test('should select same position item even if item value is different', async ({ page }) => { - await page.setContent(` - - - - - - `); - - const pickerColumn = page.locator('ion-picker-column-internal'); - await expect(pickerColumn).toHaveJSProperty('value', 5); - - await pickerColumn.evaluate((el: HTMLIonPickerColumnInternalElement) => { - el.items = [ - { text: '1', value: 1 }, - { text: '2', value: 2 }, - { text: '3', value: 3 }, - { text: '4', value: 4 }, - { text: '1000', value: 1000 }, - ]; - }); - - await page.waitForChanges(); - await expect(pickerColumn).toHaveJSProperty('value', 1000); - }); - test('should not select a disabled item', async ({ page }) => { - await page.setContent(` - - - - - - `); - - const pickerColumn = page.locator('ion-picker-column-internal'); - await expect(pickerColumn).toHaveJSProperty('value', 5); - - await pickerColumn.evaluate((el: HTMLIonPickerColumnInternalElement) => { - el.items = [ - { text: '1', value: 1 }, - { text: '2', value: 2 }, - { text: '3', value: 3, disabled: true }, - ]; - }); - - await page.waitForChanges(); - await expect(pickerColumn).toHaveJSProperty('value', 2); - }); - test('should reset to the first item if no good item was found', async ({ page }) => { - await page.setContent(` - - - - - - `); - - const pickerColumn = page.locator('ion-picker-column-internal'); - await expect(pickerColumn).toHaveJSProperty('value', 5); - - await pickerColumn.evaluate((el: HTMLIonPickerColumnInternalElement) => { - el.items = [ - { text: '1', value: 1 }, - { text: '2', value: 2, disabled: true }, - { text: '3', value: 3, disabled: true }, - ]; - }); - - await page.waitForChanges(); - await expect(pickerColumn).toHaveJSProperty('value', 1); - }); - test('should still select correct value if data was added', async ({ page }) => { - await page.setContent(` - - - - - - `); - - const pickerColumn = page.locator('ion-picker-column-internal'); - await expect(pickerColumn).toHaveJSProperty('value', 5); - - await pickerColumn.evaluate((el: HTMLIonPickerColumnInternalElement) => { - el.items = [ - { text: '1', value: 1 }, - { text: '2', value: 2 }, - { text: '3', value: 3 }, - { text: '4', value: 4 }, - { text: '6', value: 6 }, - { text: '7', value: 7 }, - { text: '5', value: 5 }, - ]; - }); - - await page.waitForChanges(); - await expect(pickerColumn).toHaveJSProperty('value', 5); - }); -});