-
Notifications
You must be signed in to change notification settings - Fork 385
chore(CalendarMonth): provided explicit guidance for labeling an inline calendar month #8375
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
Changes from 3 commits
ce5a7cf
c94e47a
c791246
e888f1c
ffb9203
6267c75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,15 @@ export enum Weekday { | |||||
| Saturday | ||||||
| } | ||||||
|
|
||||||
| export interface InlineCalendarProps { | ||||||
| /** Component wrapping the calendar month when used inline. Recommended to be 'article'. */ | ||||||
| wrapperComponent?: keyof JSX.IntrinsicElements; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we just call this component? |
||||||
| /** Title of the calendar rendered above the inline calendar month. Recommended to be a 'title' component. */ | ||||||
| title?: React.ReactNode; | ||||||
| /** Id of the accessible label of the calendar month. Recommended to map to the title. */ | ||||||
| ariaLabeledby?: string; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just missing a lowercase
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thatblindgeye If the consumer actually passes
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One downside to making it required is that if the |
||||||
| } | ||||||
|
|
||||||
| /** Additional properties that extend from and can be passed to the main component. These | ||||||
| * properties allow customizing the calendar formatting and aria-labels. | ||||||
| */ | ||||||
|
|
@@ -49,6 +58,8 @@ export interface CalendarFormat { | |||||
| weekStart?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | Weekday; | ||||||
| /** Accessible label for the year input. */ | ||||||
| yearInputAriaLabel?: string; | ||||||
| /** */ | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing a description here |
||||||
| inlineProps?: InlineCalendarProps; | ||||||
| } | ||||||
|
|
||||||
| export interface CalendarProps extends CalendarFormat, Omit<React.HTMLProps<HTMLDivElement>, 'onChange'> { | ||||||
|
|
@@ -133,6 +144,7 @@ export const CalendarMonth = ({ | |||||
| yearInputAriaLabel = 'Select year', | ||||||
| cellAriaLabel, | ||||||
| isDateFocused = false, | ||||||
| inlineProps, | ||||||
| ...props | ||||||
| }: CalendarProps) => { | ||||||
| const longMonths = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11].map(monthNum => new Date(1990, monthNum)).map(monthFormat); | ||||||
|
|
@@ -232,7 +244,8 @@ export const CalendarMonth = ({ | |||||
| const isHoveredDateValid = isValidated(hoveredDate); | ||||||
| const monthFormatted = monthFormat(focusedDate); | ||||||
| const yearFormatted = yearFormat(focusedDate); | ||||||
| return ( | ||||||
|
|
||||||
| const calendarToRender = ( | ||||||
| <div className={css(styles.calendarMonth, className)} {...props}> | ||||||
| <div className={styles.calendarMonthHeader}> | ||||||
| <div className={css(styles.calendarMonthHeaderNavControl, styles.modifiers.prevMonth)}> | ||||||
|
|
@@ -386,4 +399,16 @@ export const CalendarMonth = ({ | |||||
| </table> | ||||||
| </div> | ||||||
| ); | ||||||
|
|
||||||
| if (inlineProps !== undefined) { | ||||||
| const Component = (inlineProps.wrapperComponent ? inlineProps.wrapperComponent : 'article') as any; | ||||||
| return ( | ||||||
| <Component {...(inlineProps.ariaLabeledby && { 'aria-labeledby': inlineProps.ariaLabeledby })}> | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| {inlineProps.title} | ||||||
| {calendarToRender} | ||||||
| </Component> | ||||||
| ); | ||||||
| } | ||||||
| return calendarToRender; | ||||||
| }; | ||||||
| CalendarMonth.displayName = 'CalendarMonth'; | ||||||
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 call it
CalendarMonthInlineProps