-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Calendar and DatePicker audits and chromatic stories #3046
Conversation
export type {AriaDateFieldProps, DateFieldAria} from './useDateField'; | ||
export type {DatePickerAria} from './useDatePicker'; | ||
export type {DateRangePickerAria} from './useDateRangePicker'; | ||
export type {DateSegmentAria} from './useDateSegment'; |
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.
New pattern? We aren't currently very consistent about what types we export from our packages. I was thinking we could try to export all input and output types from the hooks explicitly. Note that this also includes some re-exports from @react-types
as it avoids users needing to install another package separately to use them.
title: 'Calendar', | ||
parameters: { | ||
chromaticProvider: { | ||
locales: ['en-US'/* , 'ar-EG', 'ja-JP' */] |
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.
Waiting on translations
@@ -32,6 +32,11 @@ const OPEN_STATES = { | |||
*/ | |||
|
|||
export function OpenTransition(props) { | |||
// Do not apply any transition if in chromatic. | |||
if (process.env.CHROMATIC) { |
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 is stripped by Parcel during the build so won't be published
Build successful! 🎉 |
Build successful! 🎉 |
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.
Keyboard navigation seems to work as expected now. PageUp/PageDown always increments/decrements month, regardless of the visible range.
Separate issue: this is definitely an edge case, perhaps only of interest to peculiar folk who want to figure out on what day of the week Christmas in 127 BCE would fall, but I encounter the error “Too many re-renders. React limits the number of renders to prevent an infinite loop,” when navigating moving between some eras using the Calendar. It happens in the Gregorian calendar, but it’s easier to test with the Japanese calendar and it seems to happen with all calendar systems.
|
@majornista interesting find. The problem in that case is that the Japanese calendar implementation currently doesn't support dates before the Meiji era, which begins September 8, 1868 Gregorian. Therefore, April of that Gregorian year doesn't exist according to the implementation. Japanese eras get fuzzy before then, a lot of data is required to support them, and I didn't think it was terribly important for our current use cases to support. For now, I've fixed the issue by constraining the date within supported eras. If you go back to September 1 Meiji, you'll see the month appears to start on day 8 and navigating to previous dates does nothing. I didn't see the issue with the Gregorian calendar. How did you reproduce? |
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.
Tested locally, confirmed the new PageUp/Down/Home/End keyboard functionality + the Japanese date constraints. Still going through the chromatic stories but approving for now
Build successful! 🎉 |
…cleanup # Conflicts: # packages/@react-aria/calendar/src/types.ts # packages/@react-aria/calendar/src/useCalendarBase.ts # packages/@react-aria/calendar/src/useCalendarGrid.ts # packages/@react-aria/calendar/src/useRangeCalendar.ts # packages/@react-spectrum/calendar/src/CalendarBase.tsx # packages/@react-spectrum/calendar/src/CalendarMonth.tsx # packages/@react-spectrum/calendar/test/CalendarBase.test.js
Build successful! 🎉 |
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.
looks fine after merge :) gave it a quick run in chrome
|
Fixes #2751. Fixes #1682.
isOpen
props andshouldFlip
to DatePicker