Adding props to Checkbox component #1371
Conversation
…put-props-to-components
xman343
changed the title
xman343
requested review from
gretanausedaite and
adamhock
and removed request for
a team
|
Just remembered I need to update the entry in Docs - I'll do that very soon! |
…put-props-to-components
Done |
.changeset/empty-rockets-try.md
Outdated
| '@itwin/itwinui-react': major | ||
| --- | ||
|
|
||
| Added wrapperProps to allow for styling of CheckBox wrapper. |
There was a problem hiding this comment.
Update changeset with all changes you've made
| as='span' | ||
| className={ | ||
| (cx('iui-checkbox-label'), | ||
| labelClassName ? labelClassName : className) |
There was a problem hiding this comment.
With this code, if user has labeled checkbox and adds custom-classname, but not labelClassName/wrapperClassName, html structure would be this:
There was a problem hiding this comment.
I wrote it like that so that the label and wrapper would share a className with the parent component by default, unless the user goes out of their way to specify custom classNames for label and wrapper. Should their classNames both be different by default?
There was a problem hiding this comment.
Why they need to share className?
There was a problem hiding this comment.
They don't have to, I just figured some users might want them to. If they shouldn't share classNames by default, then I'll change the code so they don't
There was a problem hiding this comment.
If user wants to add styles to certain part of component, they need to use props for that part of component.
Remove this check.
There was a problem hiding this comment.
Modified "Should isomorphically apply class on %s" to only test input, and also removed the final failing test.
There was a problem hiding this comment.
it is no longer isomorphic so it's not testing anything special and can be removed
imo you should have a single unit test that tests adding separate custom classes all three elements (className, labelProps, wrapperProps)
There was a problem hiding this comment.
you're only testing two cases in there, missing a test for className which gets applied on the input.
Co-authored-by: Mayank <[email protected]>
…put-props-to-components
Co-authored-by: Mayank <[email protected]>
done! |
Changes
Adding the option for customizable wrapper props to be added to the Checkbox component. Issue: #1368
Testing
React unit testing and visual testing undertaken.
Docs
Docs will be updated to outline wrapperProps utility.