-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: Regressions from DOM events PR #8525
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
| } | ||
|
|
||
| let childRef = 'ref' in children ? children.ref as any : children.props.ref; | ||
| let childRef = 'ref' in children && !Object.getOwnPropertyDescriptor(children, 'ref')?.get ? children.ref as any : children.props.ref; |
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.
Fixes crash in storybook in React 19. React emits a warning when accessing element.ref. This checks if that is defined as a getter.
|
Build successful! 🎉 |
## API Changes
@react-spectrum/s2/@react-spectrum/s2:ComboBoxSection ComboBoxSection <T extends {}> {
aria-label?: string
- children?: ReactNode | ({}) => ReactElement
+ children?: ReactNode | (T) => ReactElement
className?: string
dependencies?: ReadonlyArray<any>
id?: Key
- items?: Iterable<{}>
+ items?: Iterable<T>
style?: CSSProperties
- value?: {}
+ value?: T
}/@react-spectrum/s2:MenuSection MenuSection <T extends {}> {
aria-label?: string
- children?: ReactNode | ({}) => ReactElement
+ children?: ReactNode | (T) => ReactElement
className?: string
defaultSelectedKeys?: 'all' | Iterable<Key>
dependencies?: ReadonlyArray<any>
disabledKeys?: Iterable<Key>
disallowEmptySelection?: boolean
id?: Key
- items?: Iterable<{}>
+ items?: Iterable<T>
onSelectionChange?: (Selection) => void
selectedKeys?: 'all' | Iterable<Key>
selectionMode?: SelectionMode
style?: CSSProperties
- value?: {}
+ value?: T
}/@react-spectrum/s2:PickerSection PickerSection <T extends {}> {
aria-label?: string
- children?: ReactNode | ({}) => ReactElement
+ children?: ReactNode | (T) => ReactElement
className?: string
dependencies?: ReadonlyArray<any>
id?: Key
- items?: Iterable<{}>
+ items?: Iterable<T>
style?: CSSProperties
- value?: {}
+ value?: T
}/@react-spectrum/s2:ComboBoxSectionProps ComboBoxSectionProps <T extends {}> {
aria-label?: string
- children?: ReactNode | ({}) => ReactElement
+ children?: ReactNode | (T) => ReactElement
className?: string
dependencies?: ReadonlyArray<any>
id?: Key
- items?: Iterable<{}>
+ items?: Iterable<T>
style?: CSSProperties
- value?: {}
+ value?: T
}/@react-spectrum/s2:MenuSectionProps MenuSectionProps <T extends {}> {
aria-label?: string
- children?: ReactNode | ({}) => ReactElement
+ children?: ReactNode | (T) => ReactElement
className?: string
defaultSelectedKeys?: 'all' | Iterable<Key>
dependencies?: ReadonlyArray<any>
disabledKeys?: Iterable<Key>
disallowEmptySelection?: boolean
id?: Key
- items?: Iterable<{}>
+ items?: Iterable<T>
onSelectionChange?: (Selection) => void
selectedKeys?: 'all' | Iterable<Key>
selectionMode?: SelectionMode
style?: CSSProperties
- value?: {}
+ value?: T
}/@react-spectrum/s2:PickerSectionProps PickerSectionProps <T extends {}> {
aria-label?: string
- children?: ReactNode | ({}) => ReactElement
+ children?: ReactNode | (T) => ReactElement
className?: string
dependencies?: ReadonlyArray<any>
id?: Key
- items?: Iterable<{}>
+ items?: Iterable<T>
style?: CSSProperties
- value?: {}
+ value?: T
} |
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.
happy to get in and we can discuss the order without chromatic crashing
| return ( | ||
| <div | ||
| {...mergeProps(DOMProps, renderProps, colorSwatchProps)} | ||
| {...mergeProps(DOMProps, colorSwatchProps, renderProps)} |
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.
It's opposite order in other components, ie ProgressBar
Are these special cases? or should they all be in this order?
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.
it's because useColorSwatch returns style, and that gets passed into useRenderProps as defaultStyle, so the user can override it.
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.
gotcha, should the others be flipped then 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.
I checked all the hooks that return styles. I think flipping the order by default would not be enough, because the defaultStyle would have to be passed into useRenderProps anyway.
Fixes regressions introduced in #8327. ColorArea and ColorSwatch had their prop merge order changed which broke in chromatic. Also omitting DOM attributes from S2 for now. This was causing storybook controls to crash.