-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Support entering invalid dates in DateField and constrain on blur #9510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Build successful! 🎉 |
| case 'hour': { | ||
| // TODO: in the case of a "fall back" DST transition, the 1am hour repeats twice. | ||
| // With this logic, it's no longer possible to select the second instance. | ||
| // Using cycle from ZonedDateTime works as expected, but requires the date already be complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edge case! Previously you could enter 11/7/2021, 1:00 AM PST by first entering 11/7/2021, 1:00 AM PDT and then incrementing the hour field to switch from PDT to PST (try it here), but now that's impossible.
We could potentially check if the value is already complete (meaning we have all of the date fields and the hour), and in that case delegate to ZonedDateTime to handle the cycling. But it's impossible if we don't know the date yet – the user would need to enter everything and then go back and press the up arrow. Should we handle this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe related, but in the TimeField 12hr, if it's empty with just the "AM", then clicking that and pressing "up arrow" causes it to set the hours to "12" and takes a second press to get it to cycle to "PM"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was also the case before but I refactored the way time is stored anyway to support entering zeros.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In TimeField s2 storybook it's displaying "AM" as "am" lowercase. In the s2 docs though, it's displaying correctly
Locale difference moving from US to AUS
In TimeField 12hr, aria-valuemin is 0 and valuemax is 11, but i think it should be 1-12? This is pre-existing.
<TimeField hourCycle={12} placeholderValue={new Time(20, 30, 0)} />
For some reason this is displaying blank, just dashes. Using ArrowUp on the hour field will show 8 though, and the display of pm is correct
| case 'hour': { | ||
| // TODO: in the case of a "fall back" DST transition, the 1am hour repeats twice. | ||
| // With this logic, it's no longer possible to select the second instance. | ||
| // Using cycle from ZonedDateTime works as expected, but requires the date already be complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe related, but in the TimeField 12hr, if it's empty with just the "AM", then clicking that and pressing "up arrow" causes it to set the hours to "12" and takes a second press to get it to cycle to "PM"
I did change this to be 1-12 but it's sort of incorrect because really 12 is the minimum and 11 is the maximum (12 hour time is weird). Noticed that chrome's native date picker does set it like this though.
I think because you set a placeholderValue, not a value/defaultValue, so it doesn't display. |
| displayValue = new IncompleteDate(calendar, hourCycle, calendarValue); | ||
| setLastValue(calendarValue); | ||
| setLastCalendar(calendar); | ||
| setLastHourCycle(hourCycle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to support the case where the user starts entering a date/time, and then changes the calendar/hour cycle before completing it, we will need more complex logic here. Right now, this would simply reset the display value back to the current props.value rather than converting the partially entered value (which may be impossible in some cases).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feel like that's fine. don't think a lot of people are switching calendars or hour cycles in between
|
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:DateSegmentRenderProps DateSegmentRenderProps {
isDisabled: boolean
isFocusVisible: boolean
isFocused: boolean
isHovered: boolean
isInvalid: boolean
isPlaceholder: boolean
isReadOnly: boolean
maxValue?: number
minValue?: number
placeholder: string
text: string
type: SegmentType
- value?: number
+ value?: number | null
}/react-aria-components:DateFieldState DateFieldState {
calendar: Calendar
clearSegment: (SegmentType) => void
commitValidation: () => void
confirmPlaceholder: () => void
dateFormatter: DateFormatter
dateValue: Date
decrement: (SegmentType) => void
decrementPage: (SegmentType) => void
+ decrementToMin: (SegmentType) => void
defaultValue: DateValue | null
displayValidation: ValidationResult
formatValue: (FieldOptions) => string
getDateFormatter: (string, FormatterOptions) => DateFormatter
granularity: Granularity
increment: (SegmentType) => void
incrementPage: (SegmentType) => void
+ incrementToMax: (SegmentType) => void
isDisabled: boolean
isInvalid: boolean
isReadOnly: boolean
isRequired: boolean
realtimeValidation: ValidationResult
resetValidation: () => void
segments: Array<DateSegment>
setSegment: (SegmentType, number) => void
setValue: (DateValue | null) => void
updateValidation: (ValidationResult) => void
value: DateValue | null
}/react-aria-components:TimeFieldState TimeFieldState {
calendar: Calendar
clearSegment: (SegmentType) => void
commitValidation: () => void
confirmPlaceholder: () => void
dateFormatter: DateFormatter
dateValue: Date
decrement: (SegmentType) => void
decrementPage: (SegmentType) => void
+ decrementToMin: (SegmentType) => void
defaultValue: DateValue | null
displayValidation: ValidationResult
formatValue: (FieldOptions) => string
getDateFormatter: (string, FormatterOptions) => DateFormatter
granularity: Granularity
increment: (SegmentType) => void
incrementPage: (SegmentType) => void
+ incrementToMax: (SegmentType) => void
isDisabled: boolean
isInvalid: boolean
isReadOnly: boolean
isRequired: boolean
realtimeValidation: ValidationResult
resetValidation: () => void
segments: Array<DateSegment>
setSegment: (SegmentType, number) => void
setValue: (DateValue | null) => void
timeValue: Time
updateValidation: (ValidationResult) => void
value: DateValue | null
}@internationalized/date/@internationalized/date:Calendar Calendar {
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getEras: () => Array<string>
getFormattableMonth: (AnyCalendarDate) => CalendarDate
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMinimumDayInMonth: (AnyCalendarDate) => number
getMinimumMonthInYear: (AnyCalendarDate) => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: (AnyCalendarDate) => number
isEqual: (Calendar) => boolean
toJulianDay: (AnyCalendarDate) => number
}/@internationalized/date:GregorianCalendar GregorianCalendar {
balanceDate: (Mutable<AnyCalendarDate>) => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: (AnyCalendarDate) => number
identifier: CalendarIdentifier
isInverseEra: (AnyCalendarDate) => boolean
}/@internationalized/date:JapaneseCalendar JapaneseCalendar {
balanceDate: (Mutable<AnyCalendarDate>) => void
constrainDate: (Mutable<AnyCalendarDate>) => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMinimumDayInMonth: (AnyCalendarDate) => number
getMinimumMonthInYear: (AnyCalendarDate) => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: (AnyCalendarDate) => number
isInverseEra: (AnyCalendarDate) => boolean
toJulianDay: (AnyCalendarDate) => number
}/@internationalized/date:BuddhistCalendar BuddhistCalendar {
balanceDate: () => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: (AnyCalendarDate) => number
identifier: CalendarIdentifier
isInverseEra: (AnyCalendarDate) => boolean
}/@internationalized/date:TaiwanCalendar TaiwanCalendar {
balanceDate: (Mutable<AnyCalendarDate>) => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: (AnyCalendarDate) => number
identifier: CalendarIdentifier
isInverseEra: (AnyCalendarDate) => boolean
}/@internationalized/date:PersianCalendar PersianCalendar {
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: () => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:IndianCalendar IndianCalendar {
balanceDate: () => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: () => number
identifier: CalendarIdentifier
isInverseEra: (AnyCalendarDate) => boolean
}/@internationalized/date:IslamicCivilCalendar IslamicCivilCalendar {
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: () => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:IslamicTabularCalendar IslamicTabularCalendar {
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: () => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:IslamicUmalquraCalendar IslamicUmalquraCalendar {
constructor: () => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: () => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:HebrewCalendar HebrewCalendar {
balanceYearMonth: (Mutable<AnyCalendarDate>, AnyCalendarDate) => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: (AnyCalendarDate) => number
getYearsInEra: () => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:EthiopicCalendar EthiopicCalendar {
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: (AnyCalendarDate) => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:EthiopicAmeteAlemCalendar EthiopicAmeteAlemCalendar {
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: () => number
identifier: CalendarIdentifier
toJulianDay: (AnyCalendarDate) => number/@internationalized/date:CopticCalendar CopticCalendar {
balanceDate: (Mutable<AnyCalendarDate>) => void
fromJulianDay: (number) => CalendarDate
getDaysInMonth: (AnyCalendarDate) => number
getDaysInYear: (AnyCalendarDate) => number
getEras: () => Array<string>
+ getMaximumDaysInMonth: () => number
+ getMaximumMonthsInYear: () => number
getMonthsInYear: () => number
getYearsInEra: (AnyCalendarDate) => number
identifier: CalendarIdentifier
isInverseEra: (AnyCalendarDate) => boolean
}@react-stately/datepicker/@react-stately/datepicker:DateFieldState DateFieldState {
calendar: Calendar
clearSegment: (SegmentType) => void
commitValidation: () => void
confirmPlaceholder: () => void
dateFormatter: DateFormatter
dateValue: Date
decrement: (SegmentType) => void
decrementPage: (SegmentType) => void
+ decrementToMin: (SegmentType) => void
defaultValue: DateValue | null
displayValidation: ValidationResult
formatValue: (FieldOptions) => string
getDateFormatter: (string, FormatterOptions) => DateFormatter
granularity: Granularity
increment: (SegmentType) => void
incrementPage: (SegmentType) => void
+ incrementToMax: (SegmentType) => void
isDisabled: boolean
isInvalid: boolean
isReadOnly: boolean
isRequired: boolean
realtimeValidation: ValidationResult
resetValidation: () => void
segments: Array<DateSegment>
setSegment: (SegmentType, number) => void
setValue: (DateValue | null) => void
updateValidation: (ValidationResult) => void
value: DateValue | null
}/@react-stately/datepicker:DateSegment DateSegment {
isEditable: boolean
isPlaceholder: boolean
maxValue?: number
minValue?: number
placeholder: string
text: string
type: SegmentType
- value?: number
+ value?: number | null
}/@react-stately/datepicker:TimeFieldState TimeFieldState {
calendar: Calendar
clearSegment: (SegmentType) => void
commitValidation: () => void
confirmPlaceholder: () => void
dateFormatter: DateFormatter
dateValue: Date
decrement: (SegmentType) => void
decrementPage: (SegmentType) => void
+ decrementToMin: (SegmentType) => void
defaultValue: DateValue | null
displayValidation: ValidationResult
formatValue: (FieldOptions) => string
getDateFormatter: (string, FormatterOptions) => DateFormatter
granularity: Granularity
increment: (SegmentType) => void
incrementPage: (SegmentType) => void
+ incrementToMax: (SegmentType) => void
isDisabled: boolean
isInvalid: boolean
isReadOnly: boolean
isRequired: boolean
realtimeValidation: ValidationResult
resetValidation: () => void
segments: Array<DateSegment>
setSegment: (SegmentType, number) => void
setValue: (DateValue | null) => void
timeValue: Time
updateValidation: (ValidationResult) => void
value: DateValue | null
} |
| case 'hour': { | ||
| // TODO: in the case of a "fall back" DST transition, the 1am hour repeats twice. | ||
| // With this logic, it's no longer possible to select the second instance. | ||
| // Using cycle from ZonedDateTime works as expected, but requires the date already be complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeating my previous comment since the code moved: Edge case! Previously you could enter 11/7/2021, 1:00 AM PST by first entering 11/7/2021, 1:00 AM PDT and then incrementing the hour field to switch from PDT to PST (try it here), but now that's impossible.
We could potentially check if the value is already complete (meaning we have all of the date fields and the hour), and in that case delegate to ZonedDateTime to handle the cycling. But it's impossible if we don't know the date yet – the user would need to enter everything and then go back and press the up arrow. Should we handle this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably can't quite test this the same way due to my locale/timezone, but for me, that link, if i press up on the hour cycle multiple times, it'll change like this and eventually move on:




I'm ok with not doing it, but if we get a bug report, I think we should be ready to patch it in some way or have a good reason we chose not to do it.
For instance, it could be more confusing that there are some cases we can figure out, but some that are impossible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that's correct, that's the old behavior. but now it's not possible to enter the second time at all. incrementing from 1 goes to 2 without repeating.
LFDanLu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic looks good to me, seems to work well. The only other changed use case that I found was w/ regards to the Japanese calendar when set to the Heisei era (aka H) and navigating to the subsequent year via keyboard that should increment it to Reiwa (aka R).
The old behavior when you start at 30/12/28 and then increment the year up by one via ArrowUp is that it then changes to 1/12/28 (see the before and after screenshots)
However, now the current behavior for the same testing steps is that you hit 31/12/28 when pressing ArrowUp, upon which on blur will change to 31/4/28 aka the last day of the Heisei era.
Not sure this is a huge deal, but still digging to see why exactly this behavior changed
| // use a NumberFormatter to manually format segments directly from raw numbers. | ||
| // When the user blurs the date field, the invalid segments will be constrained. | ||
| let numberFormatter = new NumberFormatter(locale, {useGrouping: false}); | ||
| let twoDigitFormatter = new NumberFormatter(locale, {useGrouping: false, minimumIntegerDigits: 2}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that we use NumberFormatter for the segments, we could theoretically support leading zeros for the year segment right? I don't think it is very necessary persay (not sure how common people typing leading zeros for years would be) but it would bring the year segment in line with the month and days for shouldForceLeadingZeros
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's uncommon to force leading zeroes for years, but I don't know about other locales/calendars
| // Use CalendarDate to cycle so that we update the era when going between 1 AD and 1 BC. | ||
| let date = new CalendarDate(this.calendar, this.era ?? placeholder.era, this.year ?? placeholder.year, 1, 1); | ||
| date = date.cycle(field, amount, {round: field === 'year'}); | ||
| res.era = date.era; | ||
| res.year = date.year; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh actually is this why the era handling for the japanese eras changed (and perhaps other calendar dates that had eras)? Would we need to special case those as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I guess it's because we're hard coding the month and day to 1 here, but the Reiwa era began on May 1st so R 1/1/1 never existed. We could use the actual values but only if they have already been entered - if only the year/era have been entered we'd still get this behavior. Similar problem to the timezone issue I mentioned above. So we have to decide between cycling behaving consistently independent from other fields, or sometimes depending on entering other fields first.
|
Gonna merge this and we can decide how we want to handle the edge cases after. |
Fixes #3256, fixes #5965, closes #8385, fixes #6004
This is a slimmed down version of #8385 (thanks @boutahlilsoufiane for getting it started!). It refactors the way state is stored in
useDateFieldStateto use a newIncompleteDateclass instead of types from@internationalized/date. This lets us temporarily store incomplete or invalid date values where some of the fields are null or represent dates or times that don't exist (e.g. February 31st, or 2am during a forward DST transition).Instead of constraining the value immediately as the user types, these values are not emitted via onChange until the user blurs, at which point we constrain to a valid date. This lets users more easily edit dates that are temporarily invalid, e.g. when changing the day before the month. We still emit onChange in real time whenever possible, but not when the displayed date is invalid or incomplete.
Since
Intl.DateTimeFormatcannot format invalid dates, we only use it to get the expected order of the fields, and then format the individual segments withIntl.NumberFormat.This refactor also enabled entering zeros in all fields, even when zero is not a valid value (e.g. month/day). Doing this for 12 hour time required making
IncompleteDaterepresent time in the user displayed hour cycle rather than always storing it in 24 hour time. This way we can store zero as a value and distinguish it from 12am.