-
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
Add support for custom checkboxes and radios #3343
Add support for custom checkboxes and radios #3343
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.
Looks great! @taion how do you feel about adding the custom
prop vs a different set of components?
src/FormCheck.js
Outdated
@@ -93,6 +97,7 @@ class FormCheck extends React.Component { | |||
{...props} | |||
type={type} | |||
ref={inputRef} | |||
custom={custom} |
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 think may make sense to put custom
on the Context object below with controlId, that way when using the individual components you don't need to explicitly pass custom
to the input and label
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.
Great feedback! I wasn't aware of the Context option. This helps to reduce code and makes the customize render option a lot easier to use.
www/src/pages/components/forms.js
Outdated
</LinkedHeading> | ||
<p> | ||
Each checkbox and radio is wrapped in a <code>{'<div>'}</code> with a | ||
sibling <code>FormCheckInput</code> to create our custom control and a{' '} |
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 would shorten this all to something like:
Custom checkbox and radio styles are achieved with a resourceful use of the
:checked
selector and::after
psuedo elements, but are Structurally similar to the defaultFormCheck
. By default the checked and indeterminate icons use embedded svg icons from Open Iconic...
af558d3
to
c890043
Compare
It seems like this is slightly easier than adding e.g. |
gah i clicked the wrong button i meant if we had support for is that too magical? probably? (though i like having that sort of in which case i think a |
@taion hmm I can try overriding bsPrefix in the render method. I thought about something like: const activeBsPrefix = custom ? 'custom-control' : bsPrefix;
<div
style={style}
className={classNames(
className,
activeBsPrefix,
custom && `custom-${type}`,
inline && `${activeBsPrefix}-inline`,
)}
> Is there a better way to override |
no, i was sort of speculating that a "neat" API would be to do something like i'm okay with this API |
So... @jquense Do you think this PR can be merged? |
Can you rebase? |
71d189d
to
59e516c
Compare
Sure. I've rebased latest master on top of my branch so no dirty merge commits will appear. |
Implemented custom checkboxes and radios mode as discussed in 3047.
Includes tests and documentation