fix(Slider): added event param to onChange #8970
Conversation
|
Preview: https://patternfly-react-pr-8970.surge.sh A11y report: https://patternfly-react-pr-8970-a11y.surge.sh |
| @@ -55,6 +55,12 @@ export interface SliderProps extends Omit<React.HTMLProps<HTMLDivElement>, 'onCh | |||
| min?: number; | |||
| /** Value change callback. This is called when the slider value changes. */ | |||
| onChange?: ( | |||
| event: | |||
There was a problem hiding this comment.
Since you use type any in all these examples, is it maybe more worth our time here to create a new type that is a union of all these types that can be imported by consumers so they can be using that type rather than any?
I presume the only reason you used 'any' in the examples themselves was because the actual event type is so long?
There was a problem hiding this comment.
Ah good call. Added a type for the event param in the latest push
thatblindgeye
force-pushed
the
iss8963_sliderParams
branch
from
1d73eaa
to
6681b15
Compare
thatblindgeye
force-pushed
the
iss8963_sliderParams
branch
from
6681b15
to
4c55111
Compare
|
@nicolethoen couldn't add it to the propComponents currently, so put a section after "Examples" to describe it |
| @@ -19,6 +19,13 @@ export interface SliderStepObject { | |||
| value: number; | |||
| } | |||
|
|
|||
| export type SliderOnChangeEvent = | |||
There was a problem hiding this comment.
I think this is ok... I don't know that is necessary. Would we want to update other event type definitions elsewhere to be the same for consistency? What ever we decide, I would like to revisit the slider docs once this org issue gets resolve. I don't love the hand generated "Types" section in the docs.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #8963
Additional issues: