From ce4c2ebc69a05b689741674b84c40381c71a4c7c Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 14 Jul 2023 12:58:12 -0500 Subject: [PATCH 01/45] always jump to selected month when value changes --- core/src/components/datetime/datetime.tsx | 47 +-------------------- core/src/components/datetime/utils/parse.ts | 3 ++ 2 files changed, 5 insertions(+), 45 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index a09eb0fdadd..0a8ae43075c 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -358,53 +358,10 @@ export class Datetime implements ComponentInterface { */ @Watch('value') protected valueChanged() { - const { value, minParts, maxParts, workingParts } = this; + const { value } = this; if (this.hasValue()) { - this.warnIfIncorrectValueUsage(); - - /** - * Clones the value of the `activeParts` to the private clone, to update - * the date display on the current render cycle without causing another render. - * - * This allows us to update the current value's date/time display without - * refocusing or shifting the user's display (leaves the user in place). - */ - const valueDateParts = parseDate(value); - if (valueDateParts) { - warnIfValueOutOfBounds(valueDateParts, minParts, maxParts); - - if (Array.isArray(valueDateParts)) { - this.activePartsClone = [...valueDateParts]; - } else { - const { month, day, year, hour, minute } = valueDateParts; - const ampm = hour != null ? (hour >= 12 ? 'pm' : 'am') : undefined; - - this.activePartsClone = { - ...this.activeParts, - month, - day, - year, - hour, - minute, - ampm, - }; - - /** - * The working parts am/pm value must be updated when the value changes, to - * ensure the time picker hour column values are generated correctly. - * - * Note that we don't need to do this if valueDateParts is an array, since - * multiple="true" does not apply to time pickers. - */ - this.setWorkingParts({ - ...workingParts, - ampm, - }); - } - } else { - printIonWarning(`Unable to parse date string: ${value}. Please provide a valid ISO 8601 datetime string.`); - } + this.processValue(value); } this.emitStyle(); diff --git a/core/src/components/datetime/utils/parse.ts b/core/src/components/datetime/utils/parse.ts index fda79085ac5..ad8f87f9a3f 100644 --- a/core/src/components/datetime/utils/parse.ts +++ b/core/src/components/datetime/utils/parse.ts @@ -1,3 +1,5 @@ +import { printIonWarning } from '@utils/logging'; + import type { DatetimeParts } from '../datetime-interface'; import { isAfter, isBefore } from './comparison'; @@ -85,6 +87,7 @@ export function parseDate(val: string | string[] | undefined | null): DatetimePa if (parse === null) { // wasn't able to parse the ISO datetime + printIonWarning(`Unable to parse date string: ${val}. Please provide a valid ISO 8601 datetime string.`); return undefined; } From 39d687107fa2a563b8596224e8accde79b7b05aa Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 14 Jul 2023 14:07:23 -0500 Subject: [PATCH 02/45] remove remaining activePartsClone refs --- core/src/components/datetime/datetime.tsx | 36 +++++------------------ 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 0a8ae43075c..5a62a9138f1 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -117,12 +117,6 @@ export class Datetime implements ComponentInterface { private prevPresentation: string | null = null; - /** - * 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[] = []; - @State() showMonthAndYear = false; @State() activeParts: DatetimeParts | DatetimeParts[] = []; @@ -303,11 +297,6 @@ export class Datetime implements ComponentInterface { this.parsedMinuteValues = convertToArrayOfNumbers(this.minuteValues); } - @Watch('activeParts') - protected activePartsChanged() { - this.activePartsClone = this.activeParts; - } - /** * The locale to use for `ion-datetime`. This * impacts month and day name formatting. @@ -554,9 +543,9 @@ export class Datetime implements ComponentInterface { * 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 - * defaultParts if no active date is selected. + * by returning the first selected date and then + * falling back to defaultParts if no active date + * is selected. */ private getActivePartsWithFallback = () => { const { defaultParts } = this; @@ -564,8 +553,8 @@ export class Datetime implements ComponentInterface { }; private getActivePart = () => { - const { activePartsClone } = this; - return Array.isArray(activePartsClone) ? activePartsClone[0] : activePartsClone; + const { activeParts } = this; + return Array.isArray(activeParts) ? activeParts[0] : activeParts; }; private closeParentOverlay = () => { @@ -585,7 +574,7 @@ export class Datetime implements ComponentInterface { }; private setActiveParts = (parts: DatetimeParts, removeDate = false) => { - const { multiple, minParts, maxParts, activePartsClone } = this; + const { multiple, minParts, maxParts, activeParts } = this; /** * When setting the active parts, it is possible @@ -601,16 +590,7 @@ export class Datetime implements ComponentInterface { this.setWorkingParts(validatedParts); if (multiple) { - /** - * We read from activePartsClone here because valueChanged() only updates that, - * so it's the more reliable source of truth. If we read from activeParts, then - * if you click July 1, manually set the value to July 2, and then click July 3, - * the new value would be [July 1, July 3], ignoring the value set. - * - * We can then pass the new value to activeParts (rather than activePartsClone) - * since the clone will be updated automatically by activePartsChanged(). - */ - const activePartsArray = Array.isArray(activePartsClone) ? activePartsClone : [activePartsClone]; + const activePartsArray = Array.isArray(activeParts) ? activeParts : [activeParts]; if (removeDate) { this.activeParts = activePartsArray.filter((p) => !isSameDay(p, validatedParts)); } else { @@ -2003,7 +1983,7 @@ export class Datetime implements ComponentInterface { const { isActive, isToday, ariaLabel, ariaSelected, disabled, text } = getCalendarDayState( this.locale, referenceParts, - this.activePartsClone, + this.activeParts, this.todayParts, this.minParts, this.maxParts, From 5c13d59222b7b4f6db31415a459ba6763e1ae386 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 17 Jul 2023 14:57:04 -0500 Subject: [PATCH 03/45] add ability to animate smoothly to month of new value (needs cleanup but seems to work) --- .../components/datetime/datetime-interface.ts | 5 ++ core/src/components/datetime/datetime.tsx | 72 +++++++++++++++---- core/src/components/datetime/utils/data.ts | 26 ++++++- 3 files changed, 87 insertions(+), 16 deletions(-) diff --git a/core/src/components/datetime/datetime-interface.ts b/core/src/components/datetime/datetime-interface.ts index 5f2772d6c42..9f83e37c453 100644 --- a/core/src/components/datetime/datetime-interface.ts +++ b/core/src/components/datetime/datetime-interface.ts @@ -17,6 +17,11 @@ export interface DatetimeParts { ampm?: 'am' | 'pm'; } +export interface DatetimeMonth { + month: number, + year: number; +} + export type DatetimePresentation = 'date-time' | 'time-date' | 'date' | 'time' | 'month' | 'year' | 'month-year'; export type TitleSelectedDatesFormatter = (selectedDates: string[]) => string; diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 5a62a9138f1..f6c3218d178 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -19,6 +19,7 @@ import type { DatetimeHighlight, DatetimeHighlightStyle, DatetimeHighlightCallback, + DatetimeMonth, } from './datetime-interface'; import { isSameDay, warnIfValueOutOfBounds, isBefore, isAfter } from './utils/comparison'; import { @@ -135,6 +136,8 @@ export class Datetime implements ComponentInterface { @State() isPresented = false; @State() isTimePopoverOpen = false; + @State() forceRenderMonth: DatetimeMonth | null = null; + /** * The color to use from your application's color palette. * Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`. @@ -350,7 +353,7 @@ export class Datetime implements ComponentInterface { const { value } = this; if (this.hasValue()) { - this.processValue(value); + this.processValue(value, true); } this.emitStyle(); @@ -855,6 +858,14 @@ export class Datetime implements ComponentInterface { * If no month was changed, then we can return from * the scroll callback early. */ + const { forceRenderMonth } = this; + if (forceRenderMonth !== null) { + // TODO: some of this is copied from getPrevious/NextMonth, pull into util + const numDaysInMonth = getNumDaysInMonth(forceRenderMonth.month, forceRenderMonth.year); + const forcedDay = numDaysInMonth < parts.day! ? numDaysInMonth : parts.day; + return { month: forceRenderMonth.month, year: forceRenderMonth.year, day: forcedDay }; + } + if (month === startMonth) { return getPreviousMonth(parts); } else if (month === endMonth) { @@ -1133,11 +1144,11 @@ export class Datetime implements ComponentInterface { }); } - private processValue = (value?: string | string[] | null) => { + private processValue = (value?: string | string[] | null, animate = false) => { const hasValue = value !== null && value !== undefined; const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; - const { minParts, maxParts } = this; + const { minParts, maxParts, presentation, preferWheel, workingParts } = this; this.warnIfIncorrectValueUsage(); @@ -1159,19 +1170,11 @@ export class Datetime implements ComponentInterface { * that the values don't necessarily have to be in order. */ const singleValue = Array.isArray(valueToProcess) ? valueToProcess[0] : valueToProcess; + const targetValue = clampDate(singleValue, minParts, maxParts); - const { month, day, year, hour, minute } = clampDate(singleValue, minParts, maxParts); + const { month, day, year, hour, minute } = targetValue; const ampm = parseAmPm(hour!); - this.setWorkingParts({ - month, - day, - year, - hour, - minute, - ampm, - }); - /** * Since `activeParts` indicates a value that * been explicitly selected either by the @@ -1199,6 +1202,47 @@ export class Datetime implements ComponentInterface { */ this.activeParts = []; } + + // TODO: pull this and copies from render() into helper gets? + const hasDatePresentation = presentation === 'date' || presentation === 'date-time' || presentation === 'time-date'; + const hasGrid = hasDatePresentation && !preferWheel; + + /** + * We only need to animate if we're using a grid presentation + * and actually changing months. + */ + if (animate && hasGrid && (month !== workingParts.month || year !== workingParts.year)) { + /** + * Tell other render functions that we need to force the + * target month to appear in place of the actual next/prev month. + */ + this.forceRenderMonth = { month, year }; + + /** + * Animate smoothly to the forced month. This will also update + * workingParts and correct the surrounding months for us. + */ + const targetMonthIsEarlier = isBefore(targetValue, workingParts); + targetMonthIsEarlier ? this.prevMonth() : this.nextMonth(); + + // TODO: remove timeout and instead await prevMonth/nextMonth + setTimeout(() => { + this.forceRenderMonth = null; + }, 1000); + } else { + /** + * We only need to do this if we didn't just animate to a new month, + * since prevMonth/nextMonth call setWorkingParts for us. + */ + this.setWorkingParts({ + month, + day, + year, + hour, + minute, + ampm, + }); + } }; componentWillLoad() { @@ -2092,7 +2136,7 @@ export class Datetime implements ComponentInterface { private renderCalendarBody() { return (
(this.calendarBodyRef = el)} tabindex="0"> - {generateMonths(this.workingParts).map(({ month, year }) => { + {generateMonths(this.workingParts, this.forceRenderMonth).map(({ month, year }) => { return this.renderMonth(month, year); })}
diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index a4609a9be94..8cbe3b290f0 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -1,6 +1,6 @@ import type { Mode } from '../../../interface'; import type { PickerColumnItem } from '../../picker-column-internal/picker-column-internal-interfaces'; -import type { DatetimeParts } from '../datetime-interface'; +import type { DatetimeMonth, DatetimeParts } from '../datetime-interface'; import { isAfter, isBefore, isSameDay } from './comparison'; import { @@ -254,7 +254,29 @@ export const generateTime = ( * Given DatetimeParts, generate the previous, * current, and and next months. */ -export const generateMonths = (refParts: DatetimeParts): DatetimeParts[] => { +export const generateMonths = (refParts: DatetimeParts, forcedMonth: DatetimeMonth | null = null): DatetimeParts[] => { + console.log('generateMonths called; forcedMonth:', forcedMonth); + if (forcedMonth !== null && (refParts.month !== forcedMonth.month || refParts.year !== forcedMonth.year)) { + console.log('forcing month'); + // TODO: some of this is copied from getPrevious/NextMonth, pull into util + const numDaysInMonth = getNumDaysInMonth(forcedMonth.month, forcedMonth.year); + const forcedDay = numDaysInMonth < refParts.day! ? numDaysInMonth : refParts.day; + const currentDate = { month: refParts.month, year: refParts.year, day: refParts.day }; + const forcedDate = { month: forcedMonth.month, year: forcedMonth.year, day: forcedDay }; + + const forcedMonthIsEarlier = forcedMonth.year < refParts.year || (forcedMonth.year === refParts.year && forcedMonth.month < refParts.month); + + return forcedMonthIsEarlier ? [ + forcedDate, + currentDate, + getNextMonth(refParts) + ] : [ + getPreviousMonth(refParts), + currentDate, + forcedDate + ]; + } + return [ getPreviousMonth(refParts), { month: refParts.month, year: refParts.year, day: refParts.day }, From e6a9ed51c4a7d3ca25d99be77ea4943aa1fc2bcb Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 11:18:05 -0500 Subject: [PATCH 04/45] remove DatetimeMonth interface and include day when forcing month --- core/src/components/datetime/datetime-interface.ts | 5 ----- core/src/components/datetime/datetime.tsx | 9 ++++++--- core/src/components/datetime/utils/data.ts | 11 ++++------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/core/src/components/datetime/datetime-interface.ts b/core/src/components/datetime/datetime-interface.ts index 9f83e37c453..5f2772d6c42 100644 --- a/core/src/components/datetime/datetime-interface.ts +++ b/core/src/components/datetime/datetime-interface.ts @@ -17,11 +17,6 @@ export interface DatetimeParts { ampm?: 'am' | 'pm'; } -export interface DatetimeMonth { - month: number, - year: number; -} - export type DatetimePresentation = 'date-time' | 'time-date' | 'date' | 'time' | 'month' | 'year' | 'month-year'; export type TitleSelectedDatesFormatter = (selectedDates: string[]) => string; diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index f6c3218d178..984e5437cab 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -19,7 +19,6 @@ import type { DatetimeHighlight, DatetimeHighlightStyle, DatetimeHighlightCallback, - DatetimeMonth, } from './datetime-interface'; import { isSameDay, warnIfValueOutOfBounds, isBefore, isAfter } from './utils/comparison'; import { @@ -136,7 +135,7 @@ export class Datetime implements ComponentInterface { @State() isPresented = false; @State() isTimePopoverOpen = false; - @State() forceRenderMonth: DatetimeMonth | null = null; + @State() forceRenderMonth: DatetimeParts | null = null; /** * The color to use from your application's color palette. @@ -1215,8 +1214,12 @@ export class Datetime implements ComponentInterface { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. + * Because this is a State variable, a rerender will be triggered + * automatically, updating the rendered months. */ - this.forceRenderMonth = { month, year }; + // TODO: if this works, consider renaming variable to "date" instead of "month" + this.forceRenderMonth = { month, year, day }; + /** * Animate smoothly to the forced month. This will also update diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index 8cbe3b290f0..0fc1e6251f3 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -1,6 +1,6 @@ import type { Mode } from '../../../interface'; import type { PickerColumnItem } from '../../picker-column-internal/picker-column-internal-interfaces'; -import type { DatetimeMonth, DatetimeParts } from '../datetime-interface'; +import type { DatetimeParts } from '../datetime-interface'; import { isAfter, isBefore, isSameDay } from './comparison'; import { @@ -254,17 +254,14 @@ export const generateTime = ( * Given DatetimeParts, generate the previous, * current, and and next months. */ -export const generateMonths = (refParts: DatetimeParts, forcedMonth: DatetimeMonth | null = null): DatetimeParts[] => { +export const generateMonths = (refParts: DatetimeParts, forcedMonth: DatetimeParts | null = null): DatetimeParts[] => { console.log('generateMonths called; forcedMonth:', forcedMonth); if (forcedMonth !== null && (refParts.month !== forcedMonth.month || refParts.year !== forcedMonth.year)) { console.log('forcing month'); - // TODO: some of this is copied from getPrevious/NextMonth, pull into util - const numDaysInMonth = getNumDaysInMonth(forcedMonth.month, forcedMonth.year); - const forcedDay = numDaysInMonth < refParts.day! ? numDaysInMonth : refParts.day; const currentDate = { month: refParts.month, year: refParts.year, day: refParts.day }; - const forcedDate = { month: forcedMonth.month, year: forcedMonth.year, day: forcedDay }; + const forcedDate = { month: forcedMonth.month, year: forcedMonth.year, day: forcedMonth.day }; - const forcedMonthIsEarlier = forcedMonth.year < refParts.year || (forcedMonth.year === refParts.year && forcedMonth.month < refParts.month); + const forcedMonthIsEarlier = isBefore(forcedDate, currentDate); return forcedMonthIsEarlier ? [ forcedDate, From 5ae63ec4c0137189f745c83708cc38b5fbf09219 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 11:24:43 -0500 Subject: [PATCH 05/45] rename state variable to use date instead of month --- core/src/components/datetime/datetime.tsx | 25 ++++++++++++++-------- core/src/components/datetime/utils/data.ts | 20 ++++++++--------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 984e5437cab..44393a36ba0 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -135,7 +135,15 @@ export class Datetime implements ComponentInterface { @State() isPresented = false; @State() isTimePopoverOpen = false; - @State() forceRenderMonth: DatetimeParts | null = null; + /** + * When non-null, will force the datetime to render the month + * containing the specified date. This enables animating the + * transition to a new value, and should be reset to null once + * the transition is finished and the forced month is now in view. + * + * Applies to grid-style datetimes only. + */ + @State() forceRenderDate: DatetimeParts | null = null; /** * The color to use from your application's color palette. @@ -857,12 +865,12 @@ export class Datetime implements ComponentInterface { * If no month was changed, then we can return from * the scroll callback early. */ - const { forceRenderMonth } = this; - if (forceRenderMonth !== null) { + const { forceRenderDate } = this; + if (forceRenderDate !== null) { // TODO: some of this is copied from getPrevious/NextMonth, pull into util - const numDaysInMonth = getNumDaysInMonth(forceRenderMonth.month, forceRenderMonth.year); + const numDaysInMonth = getNumDaysInMonth(forceRenderDate.month, forceRenderDate.year); const forcedDay = numDaysInMonth < parts.day! ? numDaysInMonth : parts.day; - return { month: forceRenderMonth.month, year: forceRenderMonth.year, day: forcedDay }; + return { month: forceRenderDate.month, year: forceRenderDate.year, day: forcedDay }; } if (month === startMonth) { @@ -1217,8 +1225,7 @@ export class Datetime implements ComponentInterface { * Because this is a State variable, a rerender will be triggered * automatically, updating the rendered months. */ - // TODO: if this works, consider renaming variable to "date" instead of "month" - this.forceRenderMonth = { month, year, day }; + this.forceRenderDate = { month, year, day }; /** @@ -1230,7 +1237,7 @@ export class Datetime implements ComponentInterface { // TODO: remove timeout and instead await prevMonth/nextMonth setTimeout(() => { - this.forceRenderMonth = null; + this.forceRenderDate = null; }, 1000); } else { /** @@ -2139,7 +2146,7 @@ export class Datetime implements ComponentInterface { private renderCalendarBody() { return (
(this.calendarBodyRef = el)} tabindex="0"> - {generateMonths(this.workingParts, this.forceRenderMonth).map(({ month, year }) => { + {generateMonths(this.workingParts, this.forceRenderDate).map(({ month, year }) => { return this.renderMonth(month, year); })}
diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index 0fc1e6251f3..f500bad1d65 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -254,23 +254,23 @@ export const generateTime = ( * Given DatetimeParts, generate the previous, * current, and and next months. */ -export const generateMonths = (refParts: DatetimeParts, forcedMonth: DatetimeParts | null = null): DatetimeParts[] => { - console.log('generateMonths called; forcedMonth:', forcedMonth); - if (forcedMonth !== null && (refParts.month !== forcedMonth.month || refParts.year !== forcedMonth.year)) { +export const generateMonths = (refParts: DatetimeParts, forcedDate: DatetimeParts | null = null): DatetimeParts[] => { + console.log('generateMonths called; forcedDate:', forcedDate); + if (forcedDate !== null && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { console.log('forcing month'); - const currentDate = { month: refParts.month, year: refParts.year, day: refParts.day }; - const forcedDate = { month: forcedMonth.month, year: forcedMonth.year, day: forcedMonth.day }; + const current = { month: refParts.month, year: refParts.year, day: refParts.day }; + const forced = { month: forcedDate.month, year: forcedDate.year, day: forcedDate.day }; - const forcedMonthIsEarlier = isBefore(forcedDate, currentDate); + const forcedMonthIsEarlier = isBefore(forced, current); return forcedMonthIsEarlier ? [ - forcedDate, - currentDate, + forced, + current, getNextMonth(refParts) ] : [ getPreviousMonth(refParts), - currentDate, - forcedDate + current, + forced ]; } From 27d76c7482ecdd8e311dcf182d8a72b160dad75d Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 11:28:55 -0500 Subject: [PATCH 06/45] remove missed instance of manually getting forced day --- core/src/components/datetime/datetime.tsx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 44393a36ba0..78f8d56f23f 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -859,20 +859,18 @@ export class Datetime implements ComponentInterface { const monthBox = month.getBoundingClientRect(); if (Math.abs(monthBox.x - box.x) > 2) return; + // TODO: also check if we actually scrolled to the forced month, for safety + const { forceRenderDate } = this; + if (forceRenderDate !== null) { + return { month: forceRenderDate.month, year: forceRenderDate.year, day: forceRenderDate.day }; + } + /** * From here, we can determine if the start * month or the end month was scrolled into view. * If no month was changed, then we can return from * the scroll callback early. */ - const { forceRenderDate } = this; - if (forceRenderDate !== null) { - // TODO: some of this is copied from getPrevious/NextMonth, pull into util - const numDaysInMonth = getNumDaysInMonth(forceRenderDate.month, forceRenderDate.year); - const forcedDay = numDaysInMonth < parts.day! ? numDaysInMonth : parts.day; - return { month: forceRenderDate.month, year: forceRenderDate.year, day: forcedDay }; - } - if (month === startMonth) { return getPreviousMonth(parts); } else if (month === endMonth) { @@ -1218,6 +1216,7 @@ export class Datetime implements ComponentInterface { * We only need to animate if we're using a grid presentation * and actually changing months. */ + // TODO: the latter check doesn't seem to be working if (animate && hasGrid && (month !== workingParts.month || year !== workingParts.year)) { /** * Tell other render functions that we need to force the From 237f0154e5e92086773e1d2a6b41b88053c7507f Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 11:32:36 -0500 Subject: [PATCH 07/45] nevermind this totally works, was jumping to another year lol --- 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 78f8d56f23f..5becf336bca 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1216,7 +1216,6 @@ export class Datetime implements ComponentInterface { * We only need to animate if we're using a grid presentation * and actually changing months. */ - // TODO: the latter check doesn't seem to be working if (animate && hasGrid && (month !== workingParts.month || year !== workingParts.year)) { /** * Tell other render functions that we need to force the From e79a55856c8f5c908e032ee5c4652ffb62b50cc8 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 12:45:24 -0500 Subject: [PATCH 08/45] actually wait for next/prevMonth call to finish before unsetting forceRenderDate --- core/src/components/datetime/datetime.tsx | 31 ++++++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 5becf336bca..7e08e269540 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -117,6 +117,9 @@ export class Datetime implements ComponentInterface { private prevPresentation: string | null = null; + private forceDateScrollingPromise?: Promise; + private resolveForceDateScrolling?: () => void; + @State() showMonthAndYear = false; @State() activeParts: DatetimeParts | DatetimeParts[] = []; @@ -933,6 +936,11 @@ export class Datetime implements ComponentInterface { calendarBodyRef.scrollLeft = workingMonth.clientWidth * (isRTL(this.el) ? -1 : 1); calendarBodyRef.style.removeProperty('overflow'); + + if (this.resolveForceDateScrolling) { + console.log('resolving promise from scroll callback'); + this.resolveForceDateScrolling(); + } }); }; @@ -1149,7 +1157,7 @@ export class Datetime implements ComponentInterface { }); } - private processValue = (value?: string | string[] | null, animate = false) => { + private processValue = async (value?: string | string[] | null, animate = false) => { const hasValue = value !== null && value !== undefined; const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; @@ -1225,18 +1233,27 @@ export class Datetime implements ComponentInterface { */ this.forceRenderDate = { month, year, day }; + /** + * Flag that we've started scrolling to the forced date. + * The resolve function will be called by the datetime's + * scroll listener when it's done updating everything. + * This is a replacement for making prev/nextMonth async, + * since the logic we're waiting on is in a listener. + */ + this.forceDateScrollingPromise = new Promise((resolve) => { + this.resolveForceDateScrolling = resolve; + }); /** * Animate smoothly to the forced month. This will also update * workingParts and correct the surrounding months for us. - */ + */ const targetMonthIsEarlier = isBefore(targetValue, workingParts); targetMonthIsEarlier ? this.prevMonth() : this.nextMonth(); - - // TODO: remove timeout and instead await prevMonth/nextMonth - setTimeout(() => { - this.forceRenderDate = null; - }, 1000); + await this.forceDateScrollingPromise; + this.forceDateScrollingPromise = undefined; + this.resolveForceDateScrolling = undefined; + this.forceRenderDate = null; } else { /** * We only need to do this if we didn't just animate to a new month, From 59e0bde335d7f0fb78c2d0d26f70c8b96c1f5578 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 12:45:56 -0500 Subject: [PATCH 09/45] remove console logs --- core/src/components/datetime/datetime.tsx | 1 - core/src/components/datetime/utils/data.ts | 2 -- 2 files changed, 3 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 7e08e269540..3636db9aa9e 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -938,7 +938,6 @@ export class Datetime implements ComponentInterface { calendarBodyRef.style.removeProperty('overflow'); if (this.resolveForceDateScrolling) { - console.log('resolving promise from scroll callback'); this.resolveForceDateScrolling(); } }); diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index f500bad1d65..1638765f54e 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -255,9 +255,7 @@ export const generateTime = ( * current, and and next months. */ export const generateMonths = (refParts: DatetimeParts, forcedDate: DatetimeParts | null = null): DatetimeParts[] => { - console.log('generateMonths called; forcedDate:', forcedDate); if (forcedDate !== null && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { - console.log('forcing month'); const current = { month: refParts.month, year: refParts.year, day: refParts.day }; const forced = { month: forcedDate.month, year: forcedDate.year, day: forcedDate.day }; From db126befa68931aa2232cbc610e6cf7c1bb50347 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 13:00:40 -0500 Subject: [PATCH 10/45] check if we actually scrolled to the forced month before returning it --- core/src/components/datetime/datetime.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 3636db9aa9e..32efc2246b5 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -862,9 +862,20 @@ export class Datetime implements ComponentInterface { const monthBox = month.getBoundingClientRect(); if (Math.abs(monthBox.x - box.x) > 2) return; - // TODO: also check if we actually scrolled to the forced month, for safety + /** + * If we're force-rendering a month, and we've scrolled to + * that month, return that. + * + * Checking that we've actually scrolled to the forced month + * is mostly for safety; in theory, if there's a forced month, + * that means a new value was manually set, so we should have + * automatically animated directly to it. + */ const { forceRenderDate } = this; - if (forceRenderDate !== null) { + const firstDayEl = month.querySelector('.calendar-day'); + const dataMonth = firstDayEl?.getAttribute('data-month'); + const dataYear = firstDayEl?.getAttribute('data-year'); + if (forceRenderDate !== null && dataMonth && dataYear && parseInt(dataMonth) === forceRenderDate.month && parseInt(dataYear) === forceRenderDate.year) { return { month: forceRenderDate.month, year: forceRenderDate.year, day: forceRenderDate.day }; } From 28775379fdb94220e2d33b5e92d093a8293c7368 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 13:07:36 -0500 Subject: [PATCH 11/45] pull grid style check into private getter --- core/src/components/datetime/datetime.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 32efc2246b5..4a7536d4b1a 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -229,6 +229,12 @@ export class Datetime implements ComponentInterface { */ @Prop() presentation: DatetimePresentation = 'date-time'; + private get isGridStyle() { + const { presentation, preferWheel } = this; + const hasDatePresentation = presentation === 'date' || presentation === 'date-time' || presentation === 'time-date'; + return hasDatePresentation && !preferWheel; + } + /** * The text to display on the picker's cancel button. */ @@ -1226,15 +1232,11 @@ export class Datetime implements ComponentInterface { this.activeParts = []; } - // TODO: pull this and copies from render() into helper gets? - const hasDatePresentation = presentation === 'date' || presentation === 'date-time' || presentation === 'time-date'; - const hasGrid = hasDatePresentation && !preferWheel; - /** * We only need to animate if we're using a grid presentation * and actually changing months. */ - if (animate && hasGrid && (month !== workingParts.month || year !== workingParts.year)) { + if (animate && this.isGridStyle && (month !== workingParts.month || year !== workingParts.year)) { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. @@ -2400,7 +2402,6 @@ export class Datetime implements ComponentInterface { const monthYearPickerOpen = showMonthAndYear && !isMonthAndYearPresentation; const hasDatePresentation = presentation === 'date' || presentation === 'date-time' || presentation === 'time-date'; const hasWheelVariant = hasDatePresentation && preferWheel; - const hasGrid = hasDatePresentation && !preferWheel; renderHiddenInput(true, el, name, formatValue(value), disabled); @@ -2420,7 +2421,7 @@ export class Datetime implements ComponentInterface { [`datetime-presentation-${presentation}`]: true, [`datetime-size-${size}`]: true, [`datetime-prefer-wheel`]: hasWheelVariant, - [`datetime-grid`]: hasGrid, + [`datetime-grid`]: this.isGridStyle, }), }} > From 21e36489c709df959373af566680da45e820c9b0 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 13:17:30 -0500 Subject: [PATCH 12/45] remove unused variables --- 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 4a7536d4b1a..abe270c1462 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1177,7 +1177,7 @@ export class Datetime implements ComponentInterface { const hasValue = value !== null && value !== undefined; const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; - const { minParts, maxParts, presentation, preferWheel, workingParts } = this; + const { minParts, maxParts, workingParts } = this; this.warnIfIncorrectValueUsage(); From f90bcc436c84b8756664f2b4404c9f72770d3e3d Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 13:35:51 -0500 Subject: [PATCH 13/45] don't animate if in closed datetime-button --- core/src/components/datetime/datetime.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index abe270c1462..352622e0ed0 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1177,7 +1177,7 @@ export class Datetime implements ComponentInterface { const hasValue = value !== null && value !== undefined; const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; - const { minParts, maxParts, workingParts } = this; + const { minParts, maxParts, workingParts, el } = this; this.warnIfIncorrectValueUsage(); @@ -1233,10 +1233,14 @@ export class Datetime implements ComponentInterface { } /** - * We only need to animate if we're using a grid presentation - * and actually changing months. + * 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 (otherwise there's nothing to animate to) + * 3. The datetime is visible (prevents animation when in collapsed datetime-button, for example) */ - if (animate && this.isGridStyle && (month !== workingParts.month || year !== workingParts.year)) { + const didChangeMonth = month !== workingParts.month || year !== workingParts.year; + const elIsVisible = el.offsetParent !== null; + if (animate && this.isGridStyle && didChangeMonth && elIsVisible) { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. From 79b8e924a6c6b8956ea7df888b08684f28627521 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 14:07:50 -0500 Subject: [PATCH 14/45] switch lock promise to a local variable --- core/src/components/datetime/datetime.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 352622e0ed0..66b3f0ace42 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -117,7 +117,6 @@ export class Datetime implements ComponentInterface { private prevPresentation: string | null = null; - private forceDateScrollingPromise?: Promise; private resolveForceDateScrolling?: () => void; @State() showMonthAndYear = false; @@ -1256,7 +1255,7 @@ export class Datetime implements ComponentInterface { * This is a replacement for making prev/nextMonth async, * since the logic we're waiting on is in a listener. */ - this.forceDateScrollingPromise = new Promise((resolve) => { + const forceDateScrollingPromise: Promise = new Promise((resolve) => { this.resolveForceDateScrolling = resolve; }); @@ -1266,8 +1265,7 @@ export class Datetime implements ComponentInterface { */ const targetMonthIsEarlier = isBefore(targetValue, workingParts); targetMonthIsEarlier ? this.prevMonth() : this.nextMonth(); - await this.forceDateScrollingPromise; - this.forceDateScrollingPromise = undefined; + await forceDateScrollingPromise; this.resolveForceDateScrolling = undefined; this.forceRenderDate = null; } else { From 69267e4e7338d617972213599c2424b3a6330f7e Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 14:13:46 -0500 Subject: [PATCH 15/45] more comments --- core/src/components/datetime/utils/data.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index 1638765f54e..fbbb1bc0a1b 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -255,6 +255,10 @@ export const generateTime = ( * current, and and next months. */ export const generateMonths = (refParts: DatetimeParts, forcedDate: DatetimeParts | null = null): DatetimeParts[] => { + /** + * If we're forcing a month to appear, and it's different from the current month, + * ensure it appears by replacing the next or previous month as appropriate. + */ if (forcedDate !== null && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { const current = { month: refParts.month, year: refParts.year, day: refParts.day }; const forced = { month: forcedDate.month, year: forcedDate.year, day: forcedDate.day }; From ab9bf2772a68d7503c10c5be108c39741b67549c Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 18 Jul 2023 14:15:45 -0500 Subject: [PATCH 16/45] lint --- core/src/components/datetime/datetime.tsx | 16 +++++++++++----- core/src/components/datetime/utils/data.ts | 14 ++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 66b3f0ace42..3cc1a34f4c4 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -142,7 +142,7 @@ export class Datetime implements ComponentInterface { * containing the specified date. This enables animating the * transition to a new value, and should be reset to null once * the transition is finished and the forced month is now in view. - * + * * Applies to grid-style datetimes only. */ @State() forceRenderDate: DatetimeParts | null = null; @@ -870,7 +870,7 @@ export class Datetime implements ComponentInterface { /** * If we're force-rendering a month, and we've scrolled to * that month, return that. - * + * * Checking that we've actually scrolled to the forced month * is mostly for safety; in theory, if there's a forced month, * that means a new value was manually set, so we should have @@ -880,7 +880,13 @@ export class Datetime implements ComponentInterface { const firstDayEl = month.querySelector('.calendar-day'); const dataMonth = firstDayEl?.getAttribute('data-month'); const dataYear = firstDayEl?.getAttribute('data-year'); - if (forceRenderDate !== null && dataMonth && dataYear && parseInt(dataMonth) === forceRenderDate.month && parseInt(dataYear) === forceRenderDate.year) { + if ( + forceRenderDate !== null && + dataMonth && + dataYear && + parseInt(dataMonth) === forceRenderDate.month && + parseInt(dataYear) === forceRenderDate.year + ) { return { month: forceRenderDate.month, year: forceRenderDate.year, day: forceRenderDate.day }; } @@ -1247,7 +1253,7 @@ export class Datetime implements ComponentInterface { * automatically, updating the rendered months. */ this.forceRenderDate = { month, year, day }; - + /** * Flag that we've started scrolling to the forced date. * The resolve function will be called by the datetime's @@ -1258,7 +1264,7 @@ export class Datetime implements ComponentInterface { const forceDateScrollingPromise: Promise = new Promise((resolve) => { this.resolveForceDateScrolling = resolve; }); - + /** * Animate smoothly to the forced month. This will also update * workingParts and correct the surrounding months for us. diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index fbbb1bc0a1b..9a0e83292ba 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -265,17 +265,11 @@ export const generateMonths = (refParts: DatetimeParts, forcedDate: DatetimePart const forcedMonthIsEarlier = isBefore(forced, current); - return forcedMonthIsEarlier ? [ - forced, - current, - getNextMonth(refParts) - ] : [ - getPreviousMonth(refParts), - current, - forced - ]; + return forcedMonthIsEarlier + ? [forced, current, getNextMonth(refParts)] + : [getPreviousMonth(refParts), current, forced]; } - + return [ getPreviousMonth(refParts), { month: refParts.month, year: refParts.year, day: refParts.day }, From cbfc8e9365db003e07bef5ebff2d90411e2b968e Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 19 Jul 2023 13:58:26 -0500 Subject: [PATCH 17/45] add tests --- .../test/prefer-wheel/datetime.e2e.ts | 24 +++++++++++++++++++ .../datetime/test/set-value/datetime.e2e.ts | 14 +++++++++++ 2 files changed, 38 insertions(+) diff --git a/core/src/components/datetime/test/prefer-wheel/datetime.e2e.ts b/core/src/components/datetime/test/prefer-wheel/datetime.e2e.ts index 9d9f2cf24ba..f00c313889b 100644 --- a/core/src/components/datetime/test/prefer-wheel/datetime.e2e.ts +++ b/core/src/components/datetime/test/prefer-wheel/datetime.e2e.ts @@ -244,6 +244,30 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await ionChange.next(); }); + + test('should jump to selected date when programmatically updating value', async ({ page }) => { + await page.setContent( + ` + + `, + config + ); + + await page.waitForSelector('.datetime-ready'); + const datetime = page.locator('ion-datetime'); + + await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-05-25T12:40:00.000Z')); + await page.waitForChanges(); + + const selectedMonth = datetime.locator('.month-column .picker-item-active'); + const selectedDay = datetime.locator('.day-column .picker-item-active'); + const selectedYear = datetime.locator('.year-column .picker-item-active'); + + await expect(selectedMonth).toHaveText(/May/); + await expect(selectedDay).toHaveText(/25/); + await expect(selectedYear).toHaveText(/2021/); + }); + test.describe('datetime: date wheel localization', () => { test('should correctly localize the date data', async ({ page }) => { await page.setContent( 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 821affaa633..5bbfc00d374 100644 --- a/core/src/components/datetime/test/set-value/datetime.e2e.ts +++ b/core/src/components/datetime/test/set-value/datetime.e2e.ts @@ -15,6 +15,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => const activeDate = page.locator('ion-datetime .calendar-day-active'); await expect(activeDate).toHaveText('25'); }); + test('should update the active time when value is initially set', async ({ page }) => { await page.goto('/src/components/datetime/test/set-value', config); await page.waitForSelector('.datetime-ready'); @@ -27,6 +28,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => const activeDate = page.locator('ion-datetime .time-body'); await expect(activeDate).toHaveText('12:40 PM'); }); + test('should update active item when value is not initially set', async ({ page }) => { await page.setContent( ` @@ -64,5 +66,17 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => await expect(activeDayButton).toHaveAttribute('data-month', '10'); await expect(activeDayButton).toHaveAttribute('data-year', '2021'); }); + + test('should scroll to new month when value is initially set and then updated', async ({ page }) => { + await page.goto('/src/components/datetime/test/set-value', config); + await page.waitForSelector('.datetime-ready'); + + const datetime = page.locator('ion-datetime'); + await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-05-25T12:40:00.000Z')); + await page.waitForChanges(); + + const calendarHeader = datetime.locator('.calendar-month-year'); + await expect(calendarHeader).toHaveText(/May 2021/); + }); }); }); From fd9508ed37a50f170092e9512134dbdafc088173 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 10:41:25 -0500 Subject: [PATCH 18/45] await processValue in places it was already being called --- core/src/components/datetime/datetime.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 3cc1a34f4c4..66470a7bf7d 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -364,11 +364,11 @@ export class Datetime implements ComponentInterface { * Update the datetime value when the value changes */ @Watch('value') - protected valueChanged() { + protected async valueChanged() { const { value } = this; if (this.hasValue()) { - this.processValue(value, true); + await this.processValue(value, true); } this.emitStyle(); @@ -511,7 +511,7 @@ export class Datetime implements ComponentInterface { */ @Method() async reset(startDate?: string) { - this.processValue(startDate); + await this.processValue(startDate); } /** @@ -1290,7 +1290,7 @@ export class Datetime implements ComponentInterface { } }; - componentWillLoad() { + async componentWillLoad() { const { el, highlightedDates, multiple, presentation, preferWheel } = this; if (multiple) { @@ -1326,7 +1326,7 @@ export class Datetime implements ComponentInterface { const todayParts = (this.todayParts = parseDate(getToday())); this.defaultParts = getClosestValidDate(todayParts, monthValues, dayValues, yearValues, hourValues, minuteValues); - this.processValue(this.value); + await this.processValue(this.value); this.emitStyle(); } From 10b70ed1a19664e4575686013b028af681bb4428 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 10:42:51 -0500 Subject: [PATCH 19/45] tweak variable name to match language --- core/src/components/datetime/datetime.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 66470a7bf7d..8ca19d96930 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1269,8 +1269,8 @@ export class Datetime implements ComponentInterface { * Animate smoothly to the forced month. This will also update * workingParts and correct the surrounding months for us. */ - const targetMonthIsEarlier = isBefore(targetValue, workingParts); - targetMonthIsEarlier ? this.prevMonth() : this.nextMonth(); + const targetMonthIsBefore = isBefore(targetValue, workingParts); + targetMonthIsBefore ? this.prevMonth() : this.nextMonth(); await forceDateScrollingPromise; this.resolveForceDateScrolling = undefined; this.forceRenderDate = null; From dda4f0be0fdb59649673937f1a01adb7f6e109f4 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 10:43:15 -0500 Subject: [PATCH 20/45] clean up promise typing --- 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 8ca19d96930..6cf755f86ba 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1261,7 +1261,7 @@ export class Datetime implements ComponentInterface { * This is a replacement for making prev/nextMonth async, * since the logic we're waiting on is in a listener. */ - const forceDateScrollingPromise: Promise = new Promise((resolve) => { + const forceDateScrollingPromise = new Promise((resolve) => { this.resolveForceDateScrolling = resolve; }); From 7a3f92751372589c94d3cab63c915c64c4d60c77 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 10:47:06 -0500 Subject: [PATCH 21/45] don't animate if month/year picker is open --- core/src/components/datetime/datetime.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 6cf755f86ba..b0821098f1d 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1242,10 +1242,12 @@ export class Datetime implements ComponentInterface { * 1. We're using grid style (wheel style pickers should just jump to new value) * 2. The month and/or year actually changed (otherwise there's nothing to animate to) * 3. The datetime 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 !== workingParts.month || year !== workingParts.year; const elIsVisible = el.offsetParent !== null; - if (animate && this.isGridStyle && didChangeMonth && elIsVisible) { + const { isGridStyle, showMonthAndYear } = this; + if (animate && isGridStyle && didChangeMonth && elIsVisible && !showMonthAndYear) { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. From 3dd1732a9ce2628dec03ae3c33ee9f68004a4e66 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 11:39:40 -0500 Subject: [PATCH 22/45] fix error when setting value to improperly formatted string --- core/src/components/datetime/datetime.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index b0821098f1d..98f6f8f0064 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1186,6 +1186,14 @@ export class Datetime implements ComponentInterface { this.warnIfIncorrectValueUsage(); + /** + * Return early if the value wasn't parsed correctly, such as + * if an improperly formatted date string was provided. + */ + if (!valueToProcess) { + return; + } + /** * Datetime should only warn of out of bounds values * if set by the user. If the `value` is undefined, From 607f43cb6695f428b9ba413e790d6da402e63eb7 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 12:51:14 -0500 Subject: [PATCH 23/45] handle improper date formatting for multiple values --- core/src/components/datetime/utils/parse.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/utils/parse.ts b/core/src/components/datetime/utils/parse.ts index ad8f87f9a3f..12a7ea5467d 100644 --- a/core/src/components/datetime/utils/parse.ts +++ b/core/src/components/datetime/utils/parse.ts @@ -65,7 +65,21 @@ export function parseDate(val: string | string[]): DatetimeParts | DatetimeParts export function parseDate(val: string | string[] | undefined | null): DatetimeParts | DatetimeParts[] | undefined; export function parseDate(val: string | string[] | undefined | null): DatetimeParts | DatetimeParts[] | undefined { if (Array.isArray(val)) { - return val.map((valStr) => parseDate(valStr)); + // TODO: return early if anything is detected as undefined + const parsedArray = val.map((valStr) => parseDate(valStr)); + + /** + * If any of the values weren't parsed correctly, consider + * the entire batch incorrect. This simplifies the type + * signatures by having "undefined" be a general error case + * instead of returning (Datetime | undefined)[], which is + * harder for TS to perform type narrowing on. + */ + if ((parsedArray as (DatetimeParts | undefined)[]).includes(undefined)) { + return undefined; + } else { + return parsedArray; + } } // manually parse IS0 cuz Date.parse cannot be trusted From c57c6febf4f7f4f69db64ce615b95896afeb205e Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 13:04:11 -0500 Subject: [PATCH 24/45] update parseDate type sigs to include returning undefined when val couldn't be parsed --- .../datetime-button/datetime-button.tsx | 4 ++ core/src/components/datetime/datetime.tsx | 2 +- core/src/components/datetime/utils/parse.ts | 50 +++++++++++-------- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/core/src/components/datetime-button/datetime-button.tsx b/core/src/components/datetime-button/datetime-button.tsx index 46aa2a3b077..43467911c83 100644 --- a/core/src/components/datetime-button/datetime-button.tsx +++ b/core/src/components/datetime-button/datetime-button.tsx @@ -206,6 +206,10 @@ export class DatetimeButton implements ComponentInterface { */ const parsedDatetimes = parseDate(parsedValues.length > 0 ? parsedValues : [getToday()]); + if (!parsedDatetimes) { + return; + } + /** * If developers incorrectly use multiple="true" * with non "date" datetimes, then just select diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 98f6f8f0064..25901b1d363 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1334,7 +1334,7 @@ export class Datetime implements ComponentInterface { const yearValues = (this.parsedYearValues = convertToArrayOfNumbers(this.yearValues)); const dayValues = (this.parsedDayValues = convertToArrayOfNumbers(this.dayValues)); - const todayParts = (this.todayParts = parseDate(getToday())); + const todayParts = (this.todayParts = parseDate(getToday())!); this.defaultParts = getClosestValidDate(todayParts, monthValues, dayValues, yearValues, hourValues, minuteValues); await this.processValue(this.value); diff --git a/core/src/components/datetime/utils/parse.ts b/core/src/components/datetime/utils/parse.ts index 12a7ea5467d..6904462f849 100644 --- a/core/src/components/datetime/utils/parse.ts +++ b/core/src/components/datetime/utils/parse.ts @@ -58,28 +58,32 @@ export const getPartsFromCalendarDay = (el: HTMLElement): DatetimeParts => { * We do not use the JS Date object here because * it adjusts the date for the current timezone. */ -export function parseDate(val: string): DatetimeParts; -export function parseDate(val: string[]): DatetimeParts[]; +export function parseDate(val: string): DatetimeParts | undefined; +export function parseDate(val: string[]): DatetimeParts[] | undefined; export function parseDate(val: undefined | null): undefined; -export function parseDate(val: string | string[]): DatetimeParts | DatetimeParts[]; +export function parseDate(val: string | string[]): DatetimeParts | DatetimeParts[] | undefined; export function parseDate(val: string | string[] | undefined | null): DatetimeParts | DatetimeParts[] | undefined; export function parseDate(val: string | string[] | undefined | null): DatetimeParts | DatetimeParts[] | undefined { if (Array.isArray(val)) { - // TODO: return early if anything is detected as undefined - const parsedArray = val.map((valStr) => parseDate(valStr)); - - /** - * If any of the values weren't parsed correctly, consider - * the entire batch incorrect. This simplifies the type - * signatures by having "undefined" be a general error case - * instead of returning (Datetime | undefined)[], which is - * harder for TS to perform type narrowing on. - */ - if ((parsedArray as (DatetimeParts | undefined)[]).includes(undefined)) { - return undefined; - } else { - return parsedArray; + const parsedArray: DatetimeParts[] = []; + for (const valStr of val) { + const parsedVal = parseDate(valStr); + + /** + * If any of the values weren't parsed correctly, consider + * the entire batch incorrect. This simplifies the type + * signatures by having "undefined" be a general error case + * instead of returning (Datetime | undefined)[], which is + * harder for TS to perform type narrowing on. + */ + if (!parsedVal) { + return undefined; + } + + parsedArray.push(parsedVal); } + + return parsedArray; } // manually parse IS0 cuz Date.parse cannot be trusted @@ -149,8 +153,10 @@ export const parseAmPm = (hour: number) => { * For example, max="2012" would fill in the missing * month, day, hour, and minute information. */ -export const parseMaxParts = (max: string, todayParts: DatetimeParts): DatetimeParts => { - const { month, day, year, hour, minute } = parseDate(max); +export const parseMaxParts = (max: string, todayParts: DatetimeParts): DatetimeParts | undefined => { + const parsedMax = parseDate(max); + if (!parsedMax) return; + const { month, day, year, hour, minute } = parsedMax; /** * When passing in `max` or `min`, developers @@ -185,8 +191,10 @@ export const parseMaxParts = (max: string, todayParts: DatetimeParts): DatetimeP * For example, min="2012" would fill in the missing * month, day, hour, and minute information. */ -export const parseMinParts = (min: string, todayParts: DatetimeParts): DatetimeParts => { - const { month, day, year, hour, minute } = parseDate(min); +export const parseMinParts = (min: string, todayParts: DatetimeParts): DatetimeParts | undefined => { + const parsedMin = parseDate(min); + if (!parsedMin) return; + const { month, day, year, hour, minute } = parsedMin; /** * When passing in `max` or `min`, developers From f3679c132dc801c1e1aa55c28f0d94927e118f53 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 13:38:36 -0500 Subject: [PATCH 25/45] fix min/max parsing erroring out if year isn't provided --- core/src/components/datetime/datetime.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 25901b1d363..9ea5f0d85ca 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1326,8 +1326,6 @@ export class Datetime implements ComponentInterface { } } - this.processMinParts(); - this.processMaxParts(); const hourValues = (this.parsedHourValues = convertToArrayOfNumbers(this.hourValues)); const minuteValues = (this.parsedMinuteValues = convertToArrayOfNumbers(this.minuteValues)); const monthValues = (this.parsedMonthValues = convertToArrayOfNumbers(this.monthValues)); @@ -1336,6 +1334,10 @@ export class Datetime implements ComponentInterface { const todayParts = (this.todayParts = parseDate(getToday())!); this.defaultParts = getClosestValidDate(todayParts, monthValues, dayValues, yearValues, hourValues, minuteValues); + + this.processMinParts(); + this.processMaxParts(); + await this.processValue(this.value); this.emitStyle(); From 3fe32dc7a547c8bfd9c39353eb9859677d70a289 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 13:50:36 -0500 Subject: [PATCH 26/45] 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 9ea5f0d85ca..91b948004ba 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1337,7 +1337,7 @@ export class Datetime implements ComponentInterface { this.processMinParts(); this.processMaxParts(); - + await this.processValue(this.value); this.emitStyle(); From 53e288ed18642a8f9b2cdb2e8182146b7a4aac02 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 24 Jul 2023 15:30:20 -0500 Subject: [PATCH 27/45] avoid crash when manually setting value to empty array --- 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 91b948004ba..6ce7bd811f5 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1179,7 +1179,7 @@ export class Datetime implements ComponentInterface { } private processValue = async (value?: string | string[] | null, animate = false) => { - const hasValue = value !== null && value !== undefined; + const hasValue = value !== null && value !== undefined && (!Array.isArray(value) || value.length > 0); const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; const { minParts, maxParts, workingParts, el } = this; From 692a270e9a458efc4263aa0c2ddf0a65328583e1 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 25 Jul 2023 12:30:15 -0500 Subject: [PATCH 28/45] fix month scroll breaking when value is changed with time popover open --- core/src/components/datetime/datetime.tsx | 32 ++++++++++++++++++----- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 6ce7bd811f5..da1b4c111d2 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -841,17 +841,35 @@ export class Datetime implements ComponentInterface { const root = this.el!.shadowRoot!; /** - * Get the element that is in the center of the calendar body. - * This will be an element inside of the active month. + * Get the elements that are in the center of the calendar body. + * The topmost of these will be elements inside of the active month. + * + * We filter out popovers to exclude the time picker, if present, + * since it sits on top of the entire screen. This handles the + * time picker being open when the datetime's value is changed + * programmatically, for example. + */ + const elementsAtCenter = root + .elementsFromPoint(box.x + box.width / 2, box.y + box.height / 2) + .filter((el) => el.tagName !== 'ION-POPOVER'); + + /** + * If there are no elements, then the component may be + * re-rendering on a slow device. */ - const elementAtCenter = root.elementFromPoint(box.x + box.width / 2, box.y + box.height / 2); + if (elementsAtCenter.length === 0) { + return; + } + /** - * If there is no element then the - * component may be re-rendering on a slow device. + * elementsFromPoint always orders the returned elements starting + * from the topmost box, so it's safe to just grab the first one. + * This ensures we always pick an element inside the active month, + * rather than, say, the body element. */ - if (!elementAtCenter) return; + const topmostCenterEl = elementsAtCenter[0]; - const month = elementAtCenter.closest('.calendar-month'); + const month = topmostCenterEl.closest('.calendar-month'); if (!month) return; /** From 17224a19056609277089f06b5e1f07f711f8fc18 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 12:36:50 -0500 Subject: [PATCH 29/45] use undefined instead of null for forceRenderDate --- core/src/components/datetime/datetime.tsx | 6 +++--- core/src/components/datetime/utils/data.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index da1b4c111d2..b6ff37be02f 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -145,7 +145,7 @@ export class Datetime implements ComponentInterface { * * Applies to grid-style datetimes only. */ - @State() forceRenderDate: DatetimeParts | null = null; + @State() forceRenderDate?: DatetimeParts; /** * The color to use from your application's color palette. @@ -899,7 +899,7 @@ export class Datetime implements ComponentInterface { const dataMonth = firstDayEl?.getAttribute('data-month'); const dataYear = firstDayEl?.getAttribute('data-year'); if ( - forceRenderDate !== null && + forceRenderDate !== undefined && dataMonth && dataYear && parseInt(dataMonth) === forceRenderDate.month && @@ -1301,7 +1301,7 @@ export class Datetime implements ComponentInterface { targetMonthIsBefore ? this.prevMonth() : this.nextMonth(); await forceDateScrollingPromise; this.resolveForceDateScrolling = undefined; - this.forceRenderDate = null; + this.forceRenderDate = undefined; } else { /** * We only need to do this if we didn't just animate to a new month, diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index 9a0e83292ba..3a5adebda47 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -254,12 +254,12 @@ export const generateTime = ( * Given DatetimeParts, generate the previous, * current, and and next months. */ -export const generateMonths = (refParts: DatetimeParts, forcedDate: DatetimeParts | null = null): DatetimeParts[] => { +export const generateMonths = (refParts: DatetimeParts, forcedDate?: DatetimeParts): DatetimeParts[] => { /** * If we're forcing a month to appear, and it's different from the current month, * ensure it appears by replacing the next or previous month as appropriate. */ - if (forcedDate !== null && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { + if (forcedDate !== undefined && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { const current = { month: refParts.month, year: refParts.year, day: refParts.day }; const forced = { month: forcedDate.month, year: forcedDate.year, day: forcedDate.day }; From a07700502daab081e7402a45c42a7d686f1bdfd1 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 12:41:03 -0500 Subject: [PATCH 30/45] re-use current date in both branches of generateMonths --- core/src/components/datetime/utils/data.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index 3a5adebda47..cc7e60f73b0 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -255,14 +255,14 @@ export const generateTime = ( * current, and and next months. */ export const generateMonths = (refParts: DatetimeParts, forcedDate?: DatetimeParts): DatetimeParts[] => { + const current = { month: refParts.month, year: refParts.year, day: refParts.day }; + /** * If we're forcing a month to appear, and it's different from the current month, * ensure it appears by replacing the next or previous month as appropriate. */ if (forcedDate !== undefined && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { - const current = { month: refParts.month, year: refParts.year, day: refParts.day }; const forced = { month: forcedDate.month, year: forcedDate.year, day: forcedDate.day }; - const forcedMonthIsEarlier = isBefore(forced, current); return forcedMonthIsEarlier @@ -272,7 +272,7 @@ export const generateMonths = (refParts: DatetimeParts, forcedDate?: DatetimePar return [ getPreviousMonth(refParts), - { month: refParts.month, year: refParts.year, day: refParts.day }, + current, getNextMonth(refParts), ]; }; From bf67e7160588e53e0a01a5484a17ee25949de2b7 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 13:06:43 -0500 Subject: [PATCH 31/45] remove animate param from processValue --- core/src/components/datetime/datetime.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index b6ff37be02f..6317817d6ba 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -368,7 +368,7 @@ export class Datetime implements ComponentInterface { const { value } = this; if (this.hasValue()) { - await this.processValue(value, true); + await this.processValue(value); } this.emitStyle(); @@ -1196,7 +1196,7 @@ export class Datetime implements ComponentInterface { }); } - private processValue = async (value?: string | string[] | null, animate = false) => { + private processValue = async (value?: string | string[] | null) => { const hasValue = value !== null && value !== undefined && (!Array.isArray(value) || value.length > 0); const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; @@ -1273,7 +1273,7 @@ export class Datetime implements ComponentInterface { const didChangeMonth = month !== workingParts.month || year !== workingParts.year; const elIsVisible = el.offsetParent !== null; const { isGridStyle, showMonthAndYear } = this; - if (animate && isGridStyle && didChangeMonth && elIsVisible && !showMonthAndYear) { + if (isGridStyle && didChangeMonth && elIsVisible && !showMonthAndYear) { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. From 07426c6ebd40cef3c65cfd9c26cfd97311173449 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 13:09:42 -0500 Subject: [PATCH 32/45] set forceRenderDate to whole targetValue for consistency --- 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 6317817d6ba..68ef682f00c 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1280,7 +1280,7 @@ export class Datetime implements ComponentInterface { * Because this is a State variable, a rerender will be triggered * automatically, updating the rendered months. */ - this.forceRenderDate = { month, year, day }; + this.forceRenderDate = targetValue; /** * Flag that we've started scrolling to the forced date. From d2cd55958961b81765b4f1a5a349dc63aaa75541 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 13:10:32 -0500 Subject: [PATCH 33/45] also match isBefore language in generateMonths --- core/src/components/datetime/utils/data.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index cc7e60f73b0..232127de6cb 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -263,9 +263,9 @@ export const generateMonths = (refParts: DatetimeParts, forcedDate?: DatetimePar */ if (forcedDate !== undefined && (refParts.month !== forcedDate.month || refParts.year !== forcedDate.year)) { const forced = { month: forcedDate.month, year: forcedDate.year, day: forcedDate.day }; - const forcedMonthIsEarlier = isBefore(forced, current); + const forcedMonthIsBefore = isBefore(forced, current); - return forcedMonthIsEarlier + return forcedMonthIsBefore ? [forced, current, getNextMonth(refParts)] : [getPreviousMonth(refParts), current, forced]; } From aa9d1daf6dbb6dcf27109774ee4661c68226b37f Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 13:21:08 -0500 Subject: [PATCH 34/45] lint --- core/src/components/datetime/utils/data.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/src/components/datetime/utils/data.ts b/core/src/components/datetime/utils/data.ts index 232127de6cb..faf782f202e 100644 --- a/core/src/components/datetime/utils/data.ts +++ b/core/src/components/datetime/utils/data.ts @@ -270,11 +270,7 @@ export const generateMonths = (refParts: DatetimeParts, forcedDate?: DatetimePar : [getPreviousMonth(refParts), current, forced]; } - return [ - getPreviousMonth(refParts), - current, - getNextMonth(refParts), - ]; + return [getPreviousMonth(refParts), current, getNextMonth(refParts)]; }; export const getMonthColumnData = ( From cf4128fba02205ac210bb8cf040c65505892d875 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 1 Aug 2023 13:41:02 -0500 Subject: [PATCH 35/45] try adding animate param back in but also animating when calling reset --- core/src/components/datetime/datetime.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 68ef682f00c..8c1ccce5ec1 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -368,7 +368,7 @@ export class Datetime implements ComponentInterface { const { value } = this; if (this.hasValue()) { - await this.processValue(value); + await this.processValue(value, true); } this.emitStyle(); @@ -511,7 +511,7 @@ export class Datetime implements ComponentInterface { */ @Method() async reset(startDate?: string) { - await this.processValue(startDate); + await this.processValue(startDate, true); } /** @@ -1196,7 +1196,7 @@ export class Datetime implements ComponentInterface { }); } - private processValue = async (value?: string | string[] | null) => { + private processValue = async (value?: string | string[] | null, animate = false) => { const hasValue = value !== null && value !== undefined && (!Array.isArray(value) || value.length > 0); const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; @@ -1273,7 +1273,7 @@ export class Datetime implements ComponentInterface { const didChangeMonth = month !== workingParts.month || year !== workingParts.year; const elIsVisible = el.offsetParent !== null; const { isGridStyle, showMonthAndYear } = this; - if (isGridStyle && didChangeMonth && elIsVisible && !showMonthAndYear) { + if (animate && isGridStyle && didChangeMonth && elIsVisible && !showMonthAndYear) { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. From 5c842cf7c1960780ba142a40fc5c29935b905d54 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Thu, 3 Aug 2023 15:28:24 -0500 Subject: [PATCH 36/45] check if calendar body is ready before animating; take animate param back off --- core/src/components/datetime/datetime.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 8c1ccce5ec1..be669d6d03b 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -368,7 +368,7 @@ export class Datetime implements ComponentInterface { const { value } = this; if (this.hasValue()) { - await this.processValue(value, true); + await this.processValue(value); } this.emitStyle(); @@ -511,7 +511,7 @@ export class Datetime implements ComponentInterface { */ @Method() async reset(startDate?: string) { - await this.processValue(startDate, true); + await this.processValue(startDate); } /** @@ -1196,7 +1196,7 @@ export class Datetime implements ComponentInterface { }); } - private processValue = async (value?: string | string[] | null, animate = false) => { + private processValue = async (value?: string | string[] | null) => { const hasValue = value !== null && value !== undefined && (!Array.isArray(value) || value.length > 0); const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; @@ -1267,13 +1267,13 @@ export class Datetime implements ComponentInterface { * 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 (otherwise there's nothing to animate to) - * 3. The datetime is visible (prevents animation when in collapsed datetime-button, for example) + * 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 !== workingParts.month || year !== workingParts.year; - const elIsVisible = el.offsetParent !== null; + const bodyIsVisible = el.classList.contains('datetime-ready'); const { isGridStyle, showMonthAndYear } = this; - if (animate && isGridStyle && didChangeMonth && elIsVisible && !showMonthAndYear) { + if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { /** * Tell other render functions that we need to force the * target month to appear in place of the actual next/prev month. From 90a92d176377816c05b72d61e7426209b3f19d9c Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 4 Aug 2023 10:23:05 -0500 Subject: [PATCH 37/45] always assume we've scrolled to the forced date if it's set --- core/src/components/datetime/datetime.tsx | 32 +++++++++-------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index be669d6d03b..d570183c3ab 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -139,9 +139,10 @@ export class Datetime implements ComponentInterface { /** * When non-null, will force the datetime to render the month - * containing the specified date. This enables animating the - * transition to a new value, and should be reset to null once - * the transition is finished and the forced month is now in view. + * containing the specified date. Currently, this should only + * be used to enable immediately auto-scrolling to the new month, + * and should then be reset to undefined once the transition is + * finished and the forced month is now in view. * * Applies to grid-style datetimes only. */ @@ -886,25 +887,16 @@ export class Datetime implements ComponentInterface { if (Math.abs(monthBox.x - box.x) > 2) return; /** - * If we're force-rendering a month, and we've scrolled to - * that month, return that. - * - * Checking that we've actually scrolled to the forced month - * is mostly for safety; in theory, if there's a forced month, - * that means a new value was manually set, so we should have - * automatically animated directly to it. + * If we're force-rendering a month, assume we've + * scrolled to that and return it. + * + * If forceRenderDate is ever used in a context where the + * forced month is not immediately auto-scrolled to, this + * should be updated to also check whether `month` has the + * same month and year as the forced date. */ const { forceRenderDate } = this; - const firstDayEl = month.querySelector('.calendar-day'); - const dataMonth = firstDayEl?.getAttribute('data-month'); - const dataYear = firstDayEl?.getAttribute('data-year'); - if ( - forceRenderDate !== undefined && - dataMonth && - dataYear && - parseInt(dataMonth) === forceRenderDate.month && - parseInt(dataYear) === forceRenderDate.year - ) { + if (forceRenderDate !== undefined) { return { month: forceRenderDate.month, year: forceRenderDate.year, day: forceRenderDate.day }; } From bf0fc244953d721ef3339ed081558e09dc4b1f98 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 4 Aug 2023 12:02:41 -0500 Subject: [PATCH 38/45] fix forceRenderDate comment --- 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 d570183c3ab..34acb6116fb 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -138,7 +138,7 @@ export class Datetime implements ComponentInterface { @State() isTimePopoverOpen = false; /** - * When non-null, will force the datetime to render the month + * When defined, will force the datetime to render the month * containing the specified date. Currently, this should only * be used to enable immediately auto-scrolling to the new month, * and should then be reset to undefined once the transition is From 50703df665089fc474494ed1c851fb4c870b96e8 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 4 Aug 2023 12:34:05 -0500 Subject: [PATCH 39/45] pull scroll waiting stuff into separate method so processValue can be sync again --- core/src/components/datetime/datetime.tsx | 74 ++++++++++++----------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 34acb6116fb..19d843c7352 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -369,7 +369,7 @@ export class Datetime implements ComponentInterface { const { value } = this; if (this.hasValue()) { - await this.processValue(value); + this.processValue(value); } this.emitStyle(); @@ -512,7 +512,7 @@ export class Datetime implements ComponentInterface { */ @Method() async reset(startDate?: string) { - await this.processValue(startDate); + this.processValue(startDate); } /** @@ -1188,7 +1188,7 @@ export class Datetime implements ComponentInterface { }); } - private processValue = async (value?: string | string[] | null) => { + private processValue = (value?: string | string[] | null) => { const hasValue = value !== null && value !== undefined && (!Array.isArray(value) || value.length > 0); const valueToProcess = hasValue ? parseDate(value) : this.defaultParts; @@ -1266,38 +1266,11 @@ export class Datetime implements ComponentInterface { const bodyIsVisible = el.classList.contains('datetime-ready'); const { isGridStyle, showMonthAndYear } = this; if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { - /** - * Tell other render functions that we need to force the - * target month to appear in place of the actual next/prev month. - * Because this is a State variable, a rerender will be triggered - * automatically, updating the rendered months. - */ - this.forceRenderDate = targetValue; - - /** - * Flag that we've started scrolling to the forced date. - * The resolve function will be called by the datetime's - * scroll listener when it's done updating everything. - * This is a replacement for making prev/nextMonth async, - * since the logic we're waiting on is in a listener. - */ - const forceDateScrollingPromise = new Promise((resolve) => { - this.resolveForceDateScrolling = resolve; - }); - - /** - * Animate smoothly to the forced month. This will also update - * workingParts and correct the surrounding months for us. - */ - const targetMonthIsBefore = isBefore(targetValue, workingParts); - targetMonthIsBefore ? this.prevMonth() : this.nextMonth(); - await forceDateScrollingPromise; - this.resolveForceDateScrolling = undefined; - this.forceRenderDate = undefined; + this.animateToDate(targetValue); } else { /** * We only need to do this if we didn't just animate to a new month, - * since prevMonth/nextMonth call setWorkingParts for us. + * since that calls prevMonth/nextMonth which calls setWorkingParts for us. */ this.setWorkingParts({ month, @@ -1310,7 +1283,40 @@ export class Datetime implements ComponentInterface { } }; - async componentWillLoad() { + private animateToDate = async (targetValue: DatetimeParts) => { + const { workingParts } = this; + + /** + * Tell other render functions that we need to force the + * target month to appear in place of the actual next/prev month. + * Because this is a State variable, a rerender will be triggered + * automatically, updating the rendered months. + */ + this.forceRenderDate = targetValue; + + /** + * Flag that we've started scrolling to the forced date. + * The resolve function will be called by the datetime's + * scroll listener when it's done updating everything. + * This is a replacement for making prev/nextMonth async, + * since the logic we're waiting on is in a listener. + */ + const forceDateScrollingPromise = new Promise((resolve) => { + this.resolveForceDateScrolling = resolve; + }); + + /** + * Animate smoothly to the forced month. This will also update + * workingParts and correct the surrounding months for us. + */ + const targetMonthIsBefore = isBefore(targetValue, workingParts); + targetMonthIsBefore ? this.prevMonth() : this.nextMonth(); + await forceDateScrollingPromise; + this.resolveForceDateScrolling = undefined; + this.forceRenderDate = undefined; + }; + + componentWillLoad() { const { el, highlightedDates, multiple, presentation, preferWheel } = this; if (multiple) { @@ -1348,7 +1354,7 @@ export class Datetime implements ComponentInterface { this.processMinParts(); this.processMaxParts(); - await this.processValue(this.value); + this.processValue(this.value); this.emitStyle(); } From 8936c0290b185c74fc1bc1b990ff4222bae383e3 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 4 Aug 2023 12:35:26 -0500 Subject: [PATCH 40/45] 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 19d843c7352..b7975cd59d9 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -889,7 +889,7 @@ export class Datetime implements ComponentInterface { /** * If we're force-rendering a month, assume we've * scrolled to that and return it. - * + * * If forceRenderDate is ever used in a context where the * forced month is not immediately auto-scrolled to, this * should be updated to also check whether `month` has the From b1ccb75af0d70a2907d01e8f4a0f1b5470c551bf Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 4 Aug 2023 15:06:55 -0500 Subject: [PATCH 41/45] remove unneeded manual month/year selection from test --- .../datetime/test/set-value/datetime.e2e.ts | 15 --------------- 1 file changed, 15 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 82732fb3562..798b87144a1 100644 --- a/core/src/components/datetime/test/set-value/datetime.e2e.ts +++ b/core/src/components/datetime/test/set-value/datetime.e2e.ts @@ -41,23 +41,8 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => 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 ionChange = await page.spyOnEvent('ionChange'); await datetime.evaluate((el: HTMLIonDatetimeElement) => (el.value = '2021-10-05')); - - // Open month/year picker - await monthYearButton.click(); - await page.waitForChanges(); - - // Select October 2021 - // The year will automatically switch to 2021 when selecting 10 - await monthColumn.locator('.picker-item[data-value="10"]').click(); - await ionChange.next(); - - // Close month/year picker - await monthYearButton.click(); await page.waitForChanges(); // Check that correct day is highlighted From fbf2f9c4c526fb0b6047e977309bbba7b2440952 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 8 Aug 2023 11:18:26 -0500 Subject: [PATCH 42/45] don't animate if target month and/or year are undefined --- core/src/components/datetime/datetime.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 0192f83a901..ea023a3078c 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1237,14 +1237,15 @@ export class Datetime implements ComponentInterface { /** * 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 (otherwise there's nothing to animate to) + * 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 !== workingParts.month || year !== workingParts.year; + const monthYearDefined = month !== undefined && year !== undefined; const bodyIsVisible = el.classList.contains('datetime-ready'); const { isGridStyle, showMonthAndYear } = this; - if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { + if (isGridStyle && didChangeMonth && monthYearDefined && bodyIsVisible && !showMonthAndYear) { this.animateToDate(targetValue); } else { /** From b85c7df5f11226572e1424a858d106151c643f58 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 8 Aug 2023 13:38:07 -0500 Subject: [PATCH 43/45] combine didChangeMonth and monthYearDefined conditions --- core/src/components/datetime/datetime.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index ea023a3078c..c85b66ccd2b 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1241,11 +1241,11 @@ export class Datetime implements ComponentInterface { * 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 !== workingParts.month || year !== workingParts.year; - const monthYearDefined = month !== undefined && year !== undefined; + const didChangeMonth = + (month !== undefined && month !== workingParts.month) || (year !== undefined && year !== workingParts.year); const bodyIsVisible = el.classList.contains('datetime-ready'); const { isGridStyle, showMonthAndYear } = this; - if (isGridStyle && didChangeMonth && monthYearDefined && bodyIsVisible && !showMonthAndYear) { + if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) { this.animateToDate(targetValue); } else { /** From 3d79ec92833a8763be9808b8a4468f09bad25579 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 21 Aug 2023 10:21:17 -0500 Subject: [PATCH 44/45] add isGridStyle to destructure from `this` --- core/src/components/datetime/datetime.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index c85b66ccd2b..f516fad6d2b 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -2410,6 +2410,7 @@ export class Datetime implements ComponentInterface { preferWheel, presentation, size, + isGridStyle } = this; const mode = getIonMode(this); const isMonthAndYearPresentation = @@ -2437,7 +2438,7 @@ export class Datetime implements ComponentInterface { [`datetime-presentation-${presentation}`]: true, [`datetime-size-${size}`]: true, [`datetime-prefer-wheel`]: hasWheelVariant, - [`datetime-grid`]: this.isGridStyle, + [`datetime-grid`]: isGridStyle, }), }} > From 129b370d9323a1400f6bfd7d6e2f94d08a09ac50 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 21 Aug 2023 10:32:36 -0500 Subject: [PATCH 45/45] lint --- core/src/components/datetime/datetime.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 0ab81d4cf07..2a4a6cb8c9e 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -2397,7 +2397,19 @@ export class Datetime implements ComponentInterface { } render() { - const { name, value, disabled, el, color, readonly, showMonthAndYear, preferWheel, presentation, size, isGridStyle } = this; + const { + name, + value, + disabled, + el, + color, + readonly, + showMonthAndYear, + preferWheel, + presentation, + size, + isGridStyle, + } = this; const mode = getIonMode(this); const isMonthAndYearPresentation = presentation === 'year' || presentation === 'month' || presentation === 'month-year';