Skip to content

chore: s2 date followup #8537

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

Merged
merged 11 commits into from
Jul 16, 2025
Merged

chore: s2 date followup #8537

merged 11 commits into from
Jul 16, 2025

Conversation

snowystinger
Copy link
Member

@snowystinger snowystinger commented Jul 14, 2025

Contains review follow up and bugs found in testing

Outstanding comments from #8428 (comment)

Open questions:

  1. useful to add isToday to renderProps for cells
  2. useful to add weekIndex/dayIndex to renderProps for cells, see aa7701d#diff-08206439985c4bfe014f71e8aa5df9fbe4cbcf4c478f4923e9f551cb2b4b2af7R473 and thread

Bugs from testing:

  • ColorPicker needs padding on popover
  • Collapsed label needs translations
  • If you put an invalid date (e.g. start date is after the end date) and try to open the calendar, you get an intl message error, see screenshot
  • hide timezone doesn't do anything? no timezone?
  • looks like I forgot to Omit the props for timefield that we omitted for all the other components
  • If you have multiple months, a range that traverses multiple months won't include all the dates, see the screenshot 8, note how the bottom dates in the months aren't selected
  • listbox dnd images are massive

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

@rspbot
Copy link

rspbot commented Jul 14, 2025

reidbarber
reidbarber previously approved these changes Jul 14, 2025
@rspbot
Copy link

rspbot commented Jul 14, 2025

@rspbot
Copy link

rspbot commented Jul 14, 2025

@rspbot
Copy link

rspbot commented Jul 14, 2025

@rspbot
Copy link

rspbot commented Jul 14, 2025

@rspbot
Copy link

rspbot commented Jul 15, 2025

# Conflicts:
#	packages/react-aria-components/stories/ColorArea.stories.tsx
#	packages/react-aria-components/stories/ColorSlider.stories.tsx
#	packages/react-aria-components/stories/ColorSwatch.stories.tsx
#	packages/react-aria-components/stories/Popover.stories.tsx
#	packages/react-aria-components/stories/Table.stories.tsx
#	packages/react-aria-components/stories/Tooltip.stories.tsx
@rspbot
Copy link

rspbot commented Jul 15, 2025

@rspbot
Copy link

rspbot commented Jul 15, 2025

@rspbot
Copy link

rspbot commented Jul 15, 2025

@rspbot
Copy link

rspbot commented Jul 15, 2025

## API Changes

@react-spectrum/s2

/@react-spectrum/s2:DatePicker

 DatePicker <T extends DateValue> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoComplete?: string
   autoFocus?: boolean
   contextualHelp?: ReactNode
   createCalendar?: (CalendarIdentifier) => Calendar
   defaultOpen?: boolean
   defaultValue?: DateValue | null
   description?: ReactNode
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   firstDayOfWeek?: 'sun' | 'mon' | 'tue' | 'wed' | 'thu' | 'fri' | 'sat'
   form?: string
   granularity?: Granularity
   hideTimeZone?: boolean = false
   hourCycle?: number | number
   id?: string
   isDateUnavailable?: (DateValue) => boolean
   isDisabled?: boolean
   isInvalid?: boolean
   isOpen?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   maxValue?: DateValue | null
+  maxVisibleMonths?: number = 1
   minValue?: DateValue | null
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (MappedDateValue<DateValue> | null) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean) => void
   pageBehavior?: PageBehavior = visible
   placeholderValue?: DateValue | null
   shouldCloseOnSelect?: boolean | () => boolean = true
   shouldForceLeadingZeros?: boolean
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
   validate?: (MappedDateValue<DateValue>) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'
   value?: DateValue | null
-  visibleMonths?: number = 1
 }

/@react-spectrum/s2:DateRangePicker

 DateRangePicker <T extends DateValue> extends RangeCalendarProps {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   allowsNonContiguousRanges?: boolean
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   contextualHelp?: ReactNode
   defaultOpen?: boolean
   defaultValue?: RangeValue<DateValue> | null
   description?: ReactNode
   endName?: string
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   firstDayOfWeek?: 'sun' | 'mon' | 'tue' | 'wed' | 'thu' | 'fri' | 'sat'
   form?: string
   granularity?: Granularity
   hideTimeZone?: boolean = false
   hourCycle?: number | number
   id?: string
   isDateUnavailable?: (DateValue) => boolean
   isDisabled?: boolean
   isInvalid?: boolean
   isOpen?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   maxValue?: DateValue | null
+  maxVisibleMonths?: number = 1
   minValue?: DateValue | null
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (RangeValue<MappedDateValue<DateValue>> | null) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean) => void
   pageBehavior?: PageBehavior = visible
   placeholderValue?: DateValue | null
   shouldCloseOnSelect?: boolean | () => boolean = true
   shouldForceLeadingZeros?: boolean
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   startName?: string
   styles?: StylesProp
   validate?: (RangeValue<MappedDateValue<DateValue>>) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'
   value?: RangeValue<DateValue> | null
 }

/@react-spectrum/s2:DatePickerProps

 DatePickerProps <T extends DateValue> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoComplete?: string
   autoFocus?: boolean
   contextualHelp?: ReactNode
   createCalendar?: (CalendarIdentifier) => Calendar
   defaultOpen?: boolean
   defaultValue?: DateValue | null
   description?: ReactNode
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   firstDayOfWeek?: 'sun' | 'mon' | 'tue' | 'wed' | 'thu' | 'fri' | 'sat'
   form?: string
   granularity?: Granularity
   hideTimeZone?: boolean = false
   hourCycle?: number | number
   id?: string
   isDateUnavailable?: (DateValue) => boolean
   isDisabled?: boolean
   isInvalid?: boolean
   isOpen?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   maxValue?: DateValue | null
+  maxVisibleMonths?: number = 1
   minValue?: DateValue | null
   name?: string
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (MappedDateValue<DateValue> | null) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean) => void
   pageBehavior?: PageBehavior = visible
   placeholderValue?: DateValue | null
   shouldCloseOnSelect?: boolean | () => boolean = true
   shouldForceLeadingZeros?: boolean
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   styles?: StylesProp
   validate?: (MappedDateValue<DateValue>) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'
   value?: DateValue | null
-  visibleMonths?: number = 1
 }

/@react-spectrum/s2:DateRangePickerProps

 DateRangePickerProps <T extends DateValue> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   allowsNonContiguousRanges?: boolean
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   contextualHelp?: ReactNode
   defaultOpen?: boolean
   defaultValue?: RangeValue<DateValue> | null
   description?: ReactNode
   endName?: string
   errorMessage?: ReactNode | (ValidationResult) => ReactNode
   firstDayOfWeek?: 'sun' | 'mon' | 'tue' | 'wed' | 'thu' | 'fri' | 'sat'
   form?: string
   granularity?: Granularity
   hideTimeZone?: boolean = false
   hourCycle?: number | number
   id?: string
   isDateUnavailable?: (DateValue) => boolean
   isDisabled?: boolean
   isInvalid?: boolean
   isOpen?: boolean
   isReadOnly?: boolean
   isRequired?: boolean
   label?: ReactNode
   labelAlign?: Alignment = 'start'
   labelPosition?: LabelPosition = 'top'
   maxValue?: DateValue | null
+  maxVisibleMonths?: number = 1
   minValue?: DateValue | null
   necessityIndicator?: NecessityIndicator = 'icon'
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (RangeValue<MappedDateValue<DateValue>> | null) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onOpenChange?: (boolean) => void
   pageBehavior?: PageBehavior = visible
   placeholderValue?: DateValue | null
   shouldCloseOnSelect?: boolean | () => boolean = true
   shouldForceLeadingZeros?: boolean
   size?: 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   startName?: string
   styles?: StylesProp
   validate?: (RangeValue<MappedDateValue<DateValue>>) => ValidationError | boolean | null | undefined
   validationBehavior?: 'native' | 'aria' = 'native'
   value?: RangeValue<DateValue> | null
 }

Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge so we can get a build of the starter.

@snowystinger snowystinger enabled auto-merge July 16, 2025 10:52
@snowystinger snowystinger added this pull request to the merge queue Jul 16, 2025
Merged via the queue into main with commit 514384f Jul 16, 2025
31 checks passed
@snowystinger snowystinger deleted the s2-date-followup branch July 16, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants