-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Switch] Fix DOM warning when type
isn't checkbox
or radio
#36170
Conversation
Netlify deploy previewhttps://deploy-preview-36170--material-ui.netlify.app/ Bundle size report |
@dani-mp Hey, thanks a lot. The fix looks good to me but could you add a test to verify? |
@siriwatknp the fix is pretty small and IMO it should have been avoided using a linter rule or something like that. Not sure you want tests for every prop you pass to a component in your library. In any case, I'm curious, how would you test this, given that it's a DOM warning? Can you point me to a place where you do this already? |
We generally want tests for every fixed issue.
Our tests are set up to fail if there's any error on the console during execution. Simply rendering a switch with a type other than |
@dani-mp This should work: diff --git a/packages/mui-material/src/Switch/Switch.test.js b/packages/mui-material/src/Switch/Switch.test.js
index 1c9a99278c..df8724a5c2 100644
--- a/packages/mui-material/src/Switch/Switch.test.js
+++ b/packages/mui-material/src/Switch/Switch.test.js
@@ -137,4 +137,8 @@ describe('<Switch />', () => {
});
});
});
+
+ it('should not show warnings when custom `type` is provided', () => {
+ expect(() => render(<Switch type="submit" />)).not.toErrorDev();
+ });
});
|
type
isn't checkbox
or radio
Done! ✅ |
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.
Thanks a lot! It's a great first PR in Material UI!
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.
👍 Thanks for your time and welcome to MUI contributors. Looking forward to the next PR.
Fixes #36111.