Skip to content

Commit 32244fb

Browse files
fix(datetime): scroll to newly selected date when value changes (#27806)
Issue number: Resolves #26391 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> When updating the `value` programmatically on an `ion-datetime` after it has already been created: - With grid style: The selected date visually updates, but the calendar does not scroll to the newly selected month. - With wheel style: The selected date does not visually update, i.e. the wheels do not move to show the newly selected date. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Grid style datetimes now scroll to the selected date using the same animation as when clicking the next/prev month buttons. - This animation mirrors the behavior in both MUI and native iOS. See the [design doc](https://github.com/ionic-team/ionic-framework-design-documents/blob/main/projects/ionic-framework/components/datetime/0003-datetime-async-value.md) for more information and screen recordings. - The animation will not occur if the month/year did not change, or when the datetime is hidden. - Wheel style datetimes now visually update to the selected date. No animation occurs, also mirroring native. - The `parseDate` util has also had its type signatures updated to account for returning `undefined` when the date string is improperly formatted. This was missed when the util was refactored to support multiple date selection. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> - Docs PR: ionic-team/ionic-docs#3053 - While this can technically be considered a bug fix, we are merging it into a feature branch for safety; it's a fairly significant change to how datetime behaves, and may interfere with custom logic when updating a datetime's value async. - Jumping to the newly selected value is handled by replacing everything [here](https://github.com/ionic-team/ionic-framework/pull/27806/files#diff-4a407530c60e3cf72bcc11acdd21c4803a94bf47ea81b99e757db1c93d2735b8L364-L407) with `processValue()`. This covers both wheel and grid datetimes. - `activePartsClone` as a whole was also removed. It was added in #24244 to enable changing `activeParts` without triggering a rerender (and thus jumping to the new value) but since we now want to do that jump, the clone is no longer needed. - The animation code might be tricky to follow, so I recorded going through it: https://github.com/ionic-team/ionic-framework/assets/90629384/1afa5762-f493-441a-b662-f0429f2d86a7
1 parent ae9f1ab commit 32244fb

File tree

6 files changed

+232
-121
lines changed

6 files changed

+232
-121
lines changed

core/src/components/datetime-button/datetime-button.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,10 @@ export class DatetimeButton implements ComponentInterface {
206206
*/
207207
const parsedDatetimes = parseDate(parsedValues.length > 0 ? parsedValues : [getToday()]);
208208

209+
if (!parsedDatetimes) {
210+
return;
211+
}
212+
209213
/**
210214
* If developers incorrectly use multiple="true"
211215
* with non "date" datetimes, then just select

core/src/components/datetime/datetime.tsx

Lines changed: 140 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,7 @@ export class Datetime implements ComponentInterface {
117117

118118
private prevPresentation: string | null = null;
119119

120-
/**
121-
* Duplicate reference to `activeParts` that does not trigger a re-render of the component.
122-
* Allows caching an instance of the `activeParts` in between render cycles.
123-
*/
124-
private activePartsClone: DatetimeParts | DatetimeParts[] = [];
120+
private resolveForceDateScrolling?: () => void;
125121

126122
@State() showMonthAndYear = false;
127123

@@ -140,6 +136,17 @@ export class Datetime implements ComponentInterface {
140136

141137
@State() isTimePopoverOpen = false;
142138

139+
/**
140+
* When defined, will force the datetime to render the month
141+
* containing the specified date. Currently, this should only
142+
* be used to enable immediately auto-scrolling to the new month,
143+
* and should then be reset to undefined once the transition is
144+
* finished and the forced month is now in view.
145+
*
146+
* Applies to grid-style datetimes only.
147+
*/
148+
@State() forceRenderDate?: DatetimeParts;
149+
143150
/**
144151
* The color to use from your application's color palette.
145152
* Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`.
@@ -221,6 +228,12 @@ export class Datetime implements ComponentInterface {
221228
*/
222229
@Prop() presentation: DatetimePresentation = 'date-time';
223230

231+
private get isGridStyle() {
232+
const { presentation, preferWheel } = this;
233+
const hasDatePresentation = presentation === 'date' || presentation === 'date-time' || presentation === 'time-date';
234+
return hasDatePresentation && !preferWheel;
235+
}
236+
224237
/**
225238
* The text to display on the picker's cancel button.
226239
*/
@@ -302,11 +315,6 @@ export class Datetime implements ComponentInterface {
302315
this.parsedMinuteValues = convertToArrayOfNumbers(this.minuteValues);
303316
}
304317

305-
@Watch('activeParts')
306-
protected activePartsChanged() {
307-
this.activePartsClone = this.activeParts;
308-
}
309-
310318
/**
311319
* The locale to use for `ion-datetime`. This
312320
* impacts month and day name formatting.
@@ -356,54 +364,11 @@ export class Datetime implements ComponentInterface {
356364
* Update the datetime value when the value changes
357365
*/
358366
@Watch('value')
359-
protected valueChanged() {
360-
const { value, minParts, maxParts, workingParts } = this;
367+
protected async valueChanged() {
368+
const { value } = this;
361369

362370
if (this.hasValue()) {
363-
this.warnIfIncorrectValueUsage();
364-
365-
/**
366-
* Clones the value of the `activeParts` to the private clone, to update
367-
* the date display on the current render cycle without causing another render.
368-
*
369-
* This allows us to update the current value's date/time display without
370-
* refocusing or shifting the user's display (leaves the user in place).
371-
*/
372-
const valueDateParts = parseDate(value);
373-
if (valueDateParts) {
374-
warnIfValueOutOfBounds(valueDateParts, minParts, maxParts);
375-
376-
if (Array.isArray(valueDateParts)) {
377-
this.activePartsClone = [...valueDateParts];
378-
} else {
379-
const { month, day, year, hour, minute } = valueDateParts;
380-
const ampm = hour != null ? (hour >= 12 ? 'pm' : 'am') : undefined;
381-
382-
this.activePartsClone = {
383-
...this.activeParts,
384-
month,
385-
day,
386-
year,
387-
hour,
388-
minute,
389-
ampm,
390-
};
391-
392-
/**
393-
* The working parts am/pm value must be updated when the value changes, to
394-
* ensure the time picker hour column values are generated correctly.
395-
*
396-
* Note that we don't need to do this if valueDateParts is an array, since
397-
* multiple="true" does not apply to time pickers.
398-
*/
399-
this.setWorkingParts({
400-
...workingParts,
401-
ampm,
402-
});
403-
}
404-
} else {
405-
printIonWarning(`Unable to parse date string: ${value}. Please provide a valid ISO 8601 datetime string.`);
406-
}
371+
this.processValue(value);
407372
}
408373

409374
this.emitStyle();
@@ -596,18 +561,18 @@ export class Datetime implements ComponentInterface {
596561
* data. This should be used when rendering an
597562
* interface in an environment where the `value`
598563
* may not be set. This function works
599-
* by returning the first selected date in
600-
* "activePartsClone" and then falling back to
601-
* defaultParts if no active date is selected.
564+
* by returning the first selected date and then
565+
* falling back to defaultParts if no active date
566+
* is selected.
602567
*/
603568
private getActivePartsWithFallback = () => {
604569
const { defaultParts } = this;
605570
return this.getActivePart() ?? defaultParts;
606571
};
607572

608573
private getActivePart = () => {
609-
const { activePartsClone } = this;
610-
return Array.isArray(activePartsClone) ? activePartsClone[0] : activePartsClone;
574+
const { activeParts } = this;
575+
return Array.isArray(activeParts) ? activeParts[0] : activeParts;
611576
};
612577

613578
private closeParentOverlay = () => {
@@ -627,7 +592,7 @@ export class Datetime implements ComponentInterface {
627592
};
628593

629594
private setActiveParts = (parts: DatetimeParts, removeDate = false) => {
630-
const { multiple, minParts, maxParts, activePartsClone } = this;
595+
const { multiple, minParts, maxParts, activeParts } = this;
631596

632597
/**
633598
* When setting the active parts, it is possible
@@ -643,16 +608,7 @@ export class Datetime implements ComponentInterface {
643608
this.setWorkingParts(validatedParts);
644609

645610
if (multiple) {
646-
/**
647-
* We read from activePartsClone here because valueChanged() only updates that,
648-
* so it's the more reliable source of truth. If we read from activeParts, then
649-
* if you click July 1, manually set the value to July 2, and then click July 3,
650-
* the new value would be [July 1, July 3], ignoring the value set.
651-
*
652-
* We can then pass the new value to activeParts (rather than activePartsClone)
653-
* since the clone will be updated automatically by activePartsChanged().
654-
*/
655-
const activePartsArray = Array.isArray(activePartsClone) ? activePartsClone : [activePartsClone];
611+
const activePartsArray = Array.isArray(activeParts) ? activeParts : [activeParts];
656612
if (removeDate) {
657613
this.activeParts = activePartsArray.filter((p) => !isSameDay(p, validatedParts));
658614
} else {
@@ -908,6 +864,20 @@ export class Datetime implements ComponentInterface {
908864
const monthBox = month.getBoundingClientRect();
909865
if (Math.abs(monthBox.x - box.x) > 2) return;
910866

867+
/**
868+
* If we're force-rendering a month, assume we've
869+
* scrolled to that and return it.
870+
*
871+
* If forceRenderDate is ever used in a context where the
872+
* forced month is not immediately auto-scrolled to, this
873+
* should be updated to also check whether `month` has the
874+
* same month and year as the forced date.
875+
*/
876+
const { forceRenderDate } = this;
877+
if (forceRenderDate !== undefined) {
878+
return { month: forceRenderDate.month, year: forceRenderDate.year, day: forceRenderDate.day };
879+
}
880+
911881
/**
912882
* From here, we can determine if the start
913883
* month or the end month was scrolled into view.
@@ -976,6 +946,10 @@ export class Datetime implements ComponentInterface {
976946

977947
calendarBodyRef.scrollLeft = workingMonth.clientWidth * (isRTL(this.el) ? -1 : 1);
978948
calendarBodyRef.style.removeProperty('overflow');
949+
950+
if (this.resolveForceDateScrolling) {
951+
this.resolveForceDateScrolling();
952+
}
979953
});
980954
};
981955

@@ -1193,13 +1167,21 @@ export class Datetime implements ComponentInterface {
11931167
}
11941168

11951169
private processValue = (value?: string | string[] | null) => {
1196-
const hasValue = value !== null && value !== undefined;
1170+
const hasValue = value !== null && value !== undefined && (!Array.isArray(value) || value.length > 0);
11971171
const valueToProcess = hasValue ? parseDate(value) : this.defaultParts;
11981172

1199-
const { minParts, maxParts } = this;
1173+
const { minParts, maxParts, workingParts, el } = this;
12001174

12011175
this.warnIfIncorrectValueUsage();
12021176

1177+
/**
1178+
* Return early if the value wasn't parsed correctly, such as
1179+
* if an improperly formatted date string was provided.
1180+
*/
1181+
if (!valueToProcess) {
1182+
return;
1183+
}
1184+
12031185
/**
12041186
* Datetime should only warn of out of bounds values
12051187
* if set by the user. If the `value` is undefined,
@@ -1218,19 +1200,11 @@ export class Datetime implements ComponentInterface {
12181200
* that the values don't necessarily have to be in order.
12191201
*/
12201202
const singleValue = Array.isArray(valueToProcess) ? valueToProcess[0] : valueToProcess;
1203+
const targetValue = clampDate(singleValue, minParts, maxParts);
12211204

1222-
const { month, day, year, hour, minute } = clampDate(singleValue, minParts, maxParts);
1205+
const { month, day, year, hour, minute } = targetValue;
12231206
const ampm = parseAmPm(hour!);
12241207

1225-
this.setWorkingParts({
1226-
month,
1227-
day,
1228-
year,
1229-
hour,
1230-
minute,
1231-
ampm,
1232-
});
1233-
12341208
/**
12351209
* Since `activeParts` indicates a value that
12361210
* been explicitly selected either by the
@@ -1258,6 +1232,67 @@ export class Datetime implements ComponentInterface {
12581232
*/
12591233
this.activeParts = [];
12601234
}
1235+
1236+
/**
1237+
* Only animate if:
1238+
* 1. We're using grid style (wheel style pickers should just jump to new value)
1239+
* 2. The month and/or year actually changed, and both are defined (otherwise there's nothing to animate to)
1240+
* 3. The calendar body is visible (prevents animation when in collapsed datetime-button, for example)
1241+
* 4. The month/year picker is not open (since you wouldn't see the animation anyway)
1242+
*/
1243+
const didChangeMonth =
1244+
(month !== undefined && month !== workingParts.month) || (year !== undefined && year !== workingParts.year);
1245+
const bodyIsVisible = el.classList.contains('datetime-ready');
1246+
const { isGridStyle, showMonthAndYear } = this;
1247+
if (isGridStyle && didChangeMonth && bodyIsVisible && !showMonthAndYear) {
1248+
this.animateToDate(targetValue);
1249+
} else {
1250+
/**
1251+
* We only need to do this if we didn't just animate to a new month,
1252+
* since that calls prevMonth/nextMonth which calls setWorkingParts for us.
1253+
*/
1254+
this.setWorkingParts({
1255+
month,
1256+
day,
1257+
year,
1258+
hour,
1259+
minute,
1260+
ampm,
1261+
});
1262+
}
1263+
};
1264+
1265+
private animateToDate = async (targetValue: DatetimeParts) => {
1266+
const { workingParts } = this;
1267+
1268+
/**
1269+
* Tell other render functions that we need to force the
1270+
* target month to appear in place of the actual next/prev month.
1271+
* Because this is a State variable, a rerender will be triggered
1272+
* automatically, updating the rendered months.
1273+
*/
1274+
this.forceRenderDate = targetValue;
1275+
1276+
/**
1277+
* Flag that we've started scrolling to the forced date.
1278+
* The resolve function will be called by the datetime's
1279+
* scroll listener when it's done updating everything.
1280+
* This is a replacement for making prev/nextMonth async,
1281+
* since the logic we're waiting on is in a listener.
1282+
*/
1283+
const forceDateScrollingPromise = new Promise<void>((resolve) => {
1284+
this.resolveForceDateScrolling = resolve;
1285+
});
1286+
1287+
/**
1288+
* Animate smoothly to the forced month. This will also update
1289+
* workingParts and correct the surrounding months for us.
1290+
*/
1291+
const targetMonthIsBefore = isBefore(targetValue, workingParts);
1292+
targetMonthIsBefore ? this.prevMonth() : this.nextMonth();
1293+
await forceDateScrollingPromise;
1294+
this.resolveForceDateScrolling = undefined;
1295+
this.forceRenderDate = undefined;
12611296
};
12621297

12631298
componentWillLoad() {
@@ -1286,16 +1321,18 @@ export class Datetime implements ComponentInterface {
12861321
}
12871322
}
12881323

1289-
this.processMinParts();
1290-
this.processMaxParts();
12911324
const hourValues = (this.parsedHourValues = convertToArrayOfNumbers(this.hourValues));
12921325
const minuteValues = (this.parsedMinuteValues = convertToArrayOfNumbers(this.minuteValues));
12931326
const monthValues = (this.parsedMonthValues = convertToArrayOfNumbers(this.monthValues));
12941327
const yearValues = (this.parsedYearValues = convertToArrayOfNumbers(this.yearValues));
12951328
const dayValues = (this.parsedDayValues = convertToArrayOfNumbers(this.dayValues));
12961329

1297-
const todayParts = (this.todayParts = parseDate(getToday()));
1330+
const todayParts = (this.todayParts = parseDate(getToday())!);
12981331
this.defaultParts = getClosestValidDate(todayParts, monthValues, dayValues, yearValues, hourValues, minuteValues);
1332+
1333+
this.processMinParts();
1334+
this.processMaxParts();
1335+
12991336
this.processValue(this.value);
13001337

13011338
this.emitStyle();
@@ -2042,7 +2079,7 @@ export class Datetime implements ComponentInterface {
20422079
const { isActive, isToday, ariaLabel, ariaSelected, disabled, text } = getCalendarDayState(
20432080
this.locale,
20442081
referenceParts,
2045-
this.activePartsClone,
2082+
this.activeParts,
20462083
this.todayParts,
20472084
this.minParts,
20482085
this.maxParts,
@@ -2151,7 +2188,7 @@ export class Datetime implements ComponentInterface {
21512188
private renderCalendarBody() {
21522189
return (
21532190
<div class="calendar-body ion-focusable" ref={(el) => (this.calendarBodyRef = el)} tabindex="0">
2154-
{generateMonths(this.workingParts).map(({ month, year }) => {
2191+
{generateMonths(this.workingParts, this.forceRenderDate).map(({ month, year }) => {
21552192
return this.renderMonth(month, year);
21562193
})}
21572194
</div>
@@ -2360,15 +2397,26 @@ export class Datetime implements ComponentInterface {
23602397
}
23612398

23622399
render() {
2363-
const { name, value, disabled, el, color, readonly, showMonthAndYear, preferWheel, presentation, size } = this;
2400+
const {
2401+
name,
2402+
value,
2403+
disabled,
2404+
el,
2405+
color,
2406+
readonly,
2407+
showMonthAndYear,
2408+
preferWheel,
2409+
presentation,
2410+
size,
2411+
isGridStyle,
2412+
} = this;
23642413
const mode = getIonMode(this);
23652414
const isMonthAndYearPresentation =
23662415
presentation === 'year' || presentation === 'month' || presentation === 'month-year';
23672416
const shouldShowMonthAndYear = showMonthAndYear || isMonthAndYearPresentation;
23682417
const monthYearPickerOpen = showMonthAndYear && !isMonthAndYearPresentation;
23692418
const hasDatePresentation = presentation === 'date' || presentation === 'date-time' || presentation === 'time-date';
23702419
const hasWheelVariant = hasDatePresentation && preferWheel;
2371-
const hasGrid = hasDatePresentation && !preferWheel;
23722420

23732421
renderHiddenInput(true, el, name, formatValue(value), disabled);
23742422

@@ -2387,7 +2435,7 @@ export class Datetime implements ComponentInterface {
23872435
[`datetime-presentation-${presentation}`]: true,
23882436
[`datetime-size-${size}`]: true,
23892437
[`datetime-prefer-wheel`]: hasWheelVariant,
2390-
[`datetime-grid`]: hasGrid,
2438+
[`datetime-grid`]: isGridStyle,
23912439
}),
23922440
}}
23932441
>

0 commit comments

Comments
 (0)