-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Slider] onChange
handler should be called only when value has changed
#36706
Conversation
Co-authored-by: seunexplicit <[email protected]>
Netlify deploy previewhttps://deploy-preview-36706--material-ui.netlify.app/ Bundle size report |
Co-authored-by: seunexplicit <[email protected]>
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.
We would need to update the two failing tests, as the event handlers are now not being invoked because of the change. We can set the value
prop to some other initial value.
@@ -224,6 +224,12 @@ export default function useSlider(parameters: UseSliderParameters): UseSliderRet | |||
name: 'Slider', | |||
}); | |||
|
|||
const isValueUnchanged = (newValue: number | Array<number>): boolean => { |
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.
const isValueUnchanged = (newValue: number | Array<number>): boolean => { | |
const areValuesEqual = (oldValue: number | Array<number>, newValue: number | Array<number>): boolean => { |
Let's rename the util, it has negation in the name, and we use another negation when invoking. Also, let's change the signature and move it outside of the component.
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.
This has been implemented, please check again.
Thanks for the suggestion @mnajdova, the tests are passing now. |
Co-authored-by: seunexplicit <[email protected]>
Co-authored-by: seunexplicit <[email protected]>
@ZeeshanTamboli your comments have been implemented, please check again. |
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.
Looks good. Thanks for the work, @gitstart!
onChange
handler should be called only when value has changed
Description
Fixing handleChange being called even when value remains the same.
Closes #36614
This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.