-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[SwitchBase] Prepare v5 removal of the second argument of onChange #20541
[SwitchBase] Prepare v5 removal of the second argument of onChange #20541
Conversation
Details of bundle changes.Comparing: 73a5ddd...167d046 Details of page changes
|
The `onChange` function signature in SwitchBase api documentation is missing the second `checked` parameter. SwitchBase is used by Radio and Switch components so this updates their api descriptions.
a2f72ad
to
90f8677
Compare
@samuliasmala Thanks for the pull request, the argument was removed from the documentation in #20342. I think that we should remove this argument in v5. @eps1lon was this the end-goal? |
Changed the SwitchBase api documentation which propagates the change to both Switch and Radio components. |
Was it removed or moved to SwitchBase? |
Most likely by accident (but totally intended if we decide to remove it in v5 😆 ). Though I'm not sure why we need the checked state as a second parameter. Seems like it's just a mirror of |
@eps1lon Ok, great. A few months ago, I have started to update all the demos to use
@samuliasmala What do you think? We could fully migrate the codebase to stop using this second arg, keep it undocumented, add a TODO v5 removal notice. |
@oliviertassinari I agree with all of your reasoning to remove the second argument going forward so let's not merge this PR. I see you changed the title bu should I still close this PR? |
@samuliasmala Let's keep it open, I will commit a patch to align with this new direction. |
@samuliasmala Thanks for raising the issue! |
The
onChange
function signature in Switch api documentation was missing the secondchecked
parameter.