-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: various S2 bug fixes from docs testing #9272
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
base: main
Are you sure you want to change the base?
Conversation
… multiple selection
|
Build successful! 🎉 |
## API Changes
@react-spectrum/picker/@react-spectrum/picker:Picker Picker <T extends {}> {
UNSAFE_className?: string
UNSAFE_style?: CSSProperties
align?: Alignment = 'start'
alignSelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'center' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'stretch'>
- allowsEmptyCollection?: boolean
aria-describedby?: string
aria-details?: string
aria-label?: string
aria-labelledby?: string
autoFocus?: boolean
bottom?: Responsive<DimensionValue>
children: CollectionChildren<{}>
contextualHelp?: ReactNode
defaultOpen?: boolean
defaultSelectedKey?: Key
description?: ReactNode
direction?: 'bottom' | 'top' = 'bottom'
disabledKeys?: Iterable<Key>
end?: Responsive<DimensionValue>
errorMessage?: ReactNode | (ValidationResult) => ReactNode
excludeFromTabOrder?: boolean
flex?: Responsive<string | number | boolean>
flexBasis?: Responsive<number | string>
flexGrow?: Responsive<number>
flexShrink?: Responsive<number>
form?: string
gridArea?: Responsive<string>
gridColumn?: Responsive<string>
gridColumnEnd?: Responsive<string>
gridColumnStart?: Responsive<string>
gridRow?: Responsive<string>
gridRowEnd?: Responsive<string>
gridRowStart?: Responsive<string>
height?: Responsive<DimensionValue>
id?: string
isDisabled?: boolean
isHidden?: Responsive<boolean>
isInvalid?: boolean
isLoading?: boolean
isOpen?: boolean
isQuiet?: boolean
isRequired?: boolean
items?: Iterable<{}>
justifySelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'center' | 'left' | 'right' | 'stretch'>
label?: ReactNode
labelAlign?: Alignment = 'start'
labelPosition?: LabelPosition = 'top'
left?: Responsive<DimensionValue>
margin?: Responsive<DimensionValue>
marginBottom?: Responsive<DimensionValue>
marginEnd?: Responsive<DimensionValue>
marginStart?: Responsive<DimensionValue>
marginTop?: Responsive<DimensionValue>
marginX?: Responsive<DimensionValue>
marginY?: Responsive<DimensionValue>
maxHeight?: Responsive<DimensionValue>
maxWidth?: Responsive<DimensionValue>
menuWidth?: DimensionValue
minHeight?: Responsive<DimensionValue>
minWidth?: Responsive<DimensionValue>
name?: string
necessityIndicator?: NecessityIndicator = 'icon'
onBlur?: (FocusEvent<Target>) => void
onFocus?: (FocusEvent<Target>) => void
onFocusChange?: (boolean) => void
onKeyDown?: (KeyboardEvent) => void
onKeyUp?: (KeyboardEvent) => void
onLoadMore?: () => any
onOpenChange?: (boolean) => void
onSelectionChange?: (Key | null) => void
order?: Responsive<number>
placeholder?: string
position?: Responsive<'static' | 'relative' | 'absolute' | 'fixed' | 'sticky'>
right?: Responsive<DimensionValue>
selectedKey?: Key | null
shouldFlip?: boolean = true
start?: Responsive<DimensionValue>
top?: Responsive<DimensionValue>
validate?: (ValidationType<SelectionMode>) => ValidationError | boolean | null | undefined
validationBehavior?: 'aria' | 'native' = 'aria'
width?: Responsive<DimensionValue>
zIndex?: Responsive<number>
}/@react-spectrum/picker:SpectrumPickerProps SpectrumPickerProps <T> {
UNSAFE_className?: string
UNSAFE_style?: CSSProperties
align?: Alignment = 'start'
alignSelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'center' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'stretch'>
- allowsEmptyCollection?: boolean
aria-describedby?: string
aria-details?: string
aria-label?: string
aria-labelledby?: string
autoFocus?: boolean
bottom?: Responsive<DimensionValue>
children: CollectionChildren<T>
contextualHelp?: ReactNode
defaultOpen?: boolean
defaultSelectedKey?: Key
description?: ReactNode
direction?: 'bottom' | 'top' = 'bottom'
disabledKeys?: Iterable<Key>
end?: Responsive<DimensionValue>
errorMessage?: ReactNode | (ValidationResult) => ReactNode
excludeFromTabOrder?: boolean
flex?: Responsive<string | number | boolean>
flexBasis?: Responsive<number | string>
flexGrow?: Responsive<number>
flexShrink?: Responsive<number>
form?: string
gridArea?: Responsive<string>
gridColumn?: Responsive<string>
gridColumnEnd?: Responsive<string>
gridColumnStart?: Responsive<string>
gridRow?: Responsive<string>
gridRowEnd?: Responsive<string>
gridRowStart?: Responsive<string>
height?: Responsive<DimensionValue>
id?: string
isDisabled?: boolean
isHidden?: Responsive<boolean>
isInvalid?: boolean
isLoading?: boolean
isOpen?: boolean
isQuiet?: boolean
isRequired?: boolean
items?: Iterable<T>
justifySelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'center' | 'left' | 'right' | 'stretch'>
label?: ReactNode
labelAlign?: Alignment = 'start'
labelPosition?: LabelPosition = 'top'
left?: Responsive<DimensionValue>
margin?: Responsive<DimensionValue>
marginBottom?: Responsive<DimensionValue>
marginEnd?: Responsive<DimensionValue>
marginStart?: Responsive<DimensionValue>
marginTop?: Responsive<DimensionValue>
marginX?: Responsive<DimensionValue>
marginY?: Responsive<DimensionValue>
maxHeight?: Responsive<DimensionValue>
maxWidth?: Responsive<DimensionValue>
menuWidth?: DimensionValue
minHeight?: Responsive<DimensionValue>
minWidth?: Responsive<DimensionValue>
name?: string
necessityIndicator?: NecessityIndicator = 'icon'
onBlur?: (FocusEvent<Target>) => void
onFocus?: (FocusEvent<Target>) => void
onFocusChange?: (boolean) => void
onKeyDown?: (KeyboardEvent) => void
onKeyUp?: (KeyboardEvent) => void
onLoadMore?: () => any
onOpenChange?: (boolean) => void
onSelectionChange?: (Key | null) => void
order?: Responsive<number>
placeholder?: string
position?: Responsive<'static' | 'relative' | 'absolute' | 'fixed' | 'sticky'>
right?: Responsive<DimensionValue>
selectedKey?: Key | null
shouldFlip?: boolean = true
start?: Responsive<DimensionValue>
top?: Responsive<DimensionValue>
validate?: (ValidationType<SelectionMode>) => ValidationError | boolean | null | undefined
validationBehavior?: 'aria' | 'native' = 'aria'
width?: Responsive<DimensionValue>
zIndex?: Responsive<number>
}@react-spectrum/s2/@react-spectrum/s2:Picker Picker <M extends SelectionMode = 'single', T extends {}> {
UNSAFE_className?: UnsafeClassName
UNSAFE_style?: CSSProperties
align?: 'start' | 'end' = 'start'
- allowsEmptyCollection?: boolean
aria-describedby?: string
aria-details?: string
aria-label?: string
aria-labelledby?: string
autoFocus?: boolean
children: ReactNode | ({}) => ReactNode
contextualHelp?: ReactNode
defaultOpen?: boolean
defaultValue?: ValueType<SelectionMode>
dependencies?: ReadonlyArray<any>
description?: ReactNode
direction?: 'bottom' | 'top' = 'bottom'
disabledKeys?: Iterable<Key>
errorMessage?: ReactNode | (ValidationResult) => ReactNode
excludeFromTabOrder?: boolean
form?: string
id?: string
isDisabled?: boolean
isInvalid?: boolean
isOpen?: boolean
isRequired?: boolean
items?: Iterable<T>
label?: ReactNode
labelAlign?: Alignment = 'start'
labelPosition?: LabelPosition = 'top'
loadingState?: LoadingState
menuWidth?: number
name?: string
necessityIndicator?: NecessityIndicator = 'icon'
onBlur?: (FocusEvent<Target>) => void
onChange?: (T) => void
onFocus?: (FocusEvent<Target>) => void
onFocusChange?: (boolean) => void
onKeyDown?: (KeyboardEvent) => void
onKeyUp?: (KeyboardEvent) => void
onLoadMore?: () => any
onOpenChange?: (boolean) => void
placeholder?: string = 'Select an item' (localized)
selectionMode?: SelectionMode = 'single'
shouldFlip?: boolean = true
size?: 'S' | 'M' | 'L' | 'XL' = 'M'
slot?: string | null
styles?: StylesProp
validate?: (ValidationType<SelectionMode>) => ValidationError | boolean | null | undefined
validationBehavior?: 'native' | 'aria' = 'native'
value?: ValueType<SelectionMode>
}/@react-spectrum/s2:PickerProps PickerProps <M extends SelectionMode = 'single', T extends {}> {
UNSAFE_className?: UnsafeClassName
UNSAFE_style?: CSSProperties
align?: 'start' | 'end' = 'start'
- allowsEmptyCollection?: boolean
aria-describedby?: string
aria-details?: string
aria-label?: string
aria-labelledby?: string
autoFocus?: boolean
children: ReactNode | ({}) => ReactNode
contextualHelp?: ReactNode
defaultOpen?: boolean
defaultValue?: ValueType<SelectionMode>
dependencies?: ReadonlyArray<any>
description?: ReactNode
direction?: 'bottom' | 'top' = 'bottom'
disabledKeys?: Iterable<Key>
errorMessage?: ReactNode | (ValidationResult) => ReactNode
excludeFromTabOrder?: boolean
form?: string
id?: string
isDisabled?: boolean
isInvalid?: boolean
isOpen?: boolean
isRequired?: boolean
items?: Iterable<T>
label?: ReactNode
labelAlign?: Alignment = 'start'
labelPosition?: LabelPosition = 'top'
loadingState?: LoadingState
menuWidth?: number
name?: string
necessityIndicator?: NecessityIndicator = 'icon'
onBlur?: (FocusEvent<Target>) => void
onChange?: (T) => void
onFocus?: (FocusEvent<Target>) => void
onFocusChange?: (boolean) => void
onKeyDown?: (KeyboardEvent) => void
onKeyUp?: (KeyboardEvent) => void
onLoadMore?: () => any
onOpenChange?: (boolean) => void
placeholder?: string = 'Select an item' (localized)
selectionMode?: SelectionMode = 'single'
shouldFlip?: boolean = true
size?: 'S' | 'M' | 'L' | 'XL' = 'M'
slot?: string | null
styles?: StylesProp
validate?: (ValidationType<SelectionMode>) => ValidationError | boolean | null | undefined
validationBehavior?: 'native' | 'aria' = 'native'
value?: ValueType<SelectionMode>
} |
snowystinger
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.
run chromatic?
| }} | ||
| onTouchEnd={e => { | ||
| if (!(e.target as Element).closest('button,input,textarea')) { | ||
| let target = e.target as HTMLElement; |
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.
test for this change?
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 added a test for textarea, but couldn't get the datepicker test to quite simulate the proper touch end event on the content editable for some reason... Using user event doesn't trigger the touchEnd properly and firing events on the spinbuttons directly doesn't seem to work either
|
https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=1074 ugh we have too many button variants... I'll move those to a separate story |
| // don't mark unavaliable + invalid dates in range calendars as selected, that case only comes up via allowsNoncontiguousRanges | ||
| // and thus those unavailable dates shouldn't be selected. For single select calendars, we mark unavailble + invalid days as selected for styling reasons | ||
| if (!('highlightedRange' in state) || !isUnavailable) { | ||
| isSelected = true; | ||
| } |
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.
behavior open to debate, but the single select calendar case had a prior test for it hence why the special casing. I think this makes sense, range calendars don't actually include unavailable days that fall inside their range selection, but a single select calendar might have a value/default value set that is invalid + unavailable
Title sucks sorry, but see the below for what got fixed:
allowEmptyCollectionprop from Picker in RSP and S2✅ Pull Request Checklist:
📝 Test Instructions:
Test the above mentioned bugs via docs and storybook.
For the RangeCalendar bug, you can reproduce manually via any of the isDateUnavailable stories for RAC/RSP/S2 on main if you turn on both
isInvalidandallowsNonContiguousRangesvia controls and select a range that contains an unavailable date (e.g. http://reactspectrum.blob.core.windows.net/reactspectrum/7e5427c9d4f1bc6b21b3cdf1d78d37ad0a364acc/storybook/index.html?path=/story/date-and-time-rangecalendar--date-unavailable&args=allowsNonContiguousRanges:!true;isInvalid:!true&providerSwitcher-express=false).🧢 Your Project:
RSP