-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: add Switches #4268
feat: add Switches #4268
Conversation
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.
tests + types?
@@ -0,0 +1,13 @@ | |||
<Form> | |||
<Form.Check // prettier-ignore |
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.
why?
* | ||
* @default 'custom-control' | ||
*/ | ||
bsCustomPrefix: PropTypes.string, |
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.
How is this intended to be used? I don't see it getting hit at all.
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 used below in the useBootstrap thing
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 mean i don't see this ever getting overridden or anything demonstrating this getting overridden
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.
O it's more an escape hatch for a user, same as bsPrefix, since we are using custom-control
in the same way it seemed good to also expose 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.
might be good to document/demo
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 documented i guess. I didn't want to make a big deal out of it and confuse normal users i guess!
is this already push to the current version? |
Nope, will be included in the next one. |
+1 for release to beta |
closes: #4226