-
Notifications
You must be signed in to change notification settings - Fork 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
feat(Checkbox|FormField) support label
shorthand
#967
Conversation
Current coverage is 100% (diff: 100%)@@ master #967 diff @@
====================================
Files 137 137
Lines 2283 2281 -2
Methods 0 0
Messages 0 0
Branches 0 0
====================================
- Hits 2283 2281 -2
Misses 0 0
Partials 0 0
|
@@ -41,6 +37,15 @@ describe('Checkbox', () => { | |||
}) | |||
}) | |||
|
|||
describe('defaultChecked', () => { | |||
it('sets the initial checked state', () => { | |||
// consoleUtil.disableOnce() |
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.
Is this meant to be uncommented or removed?
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 like cruft.
<div> | ||
<Checkbox label={{ children: 'Prop label', className: 'foo' }} /> | ||
<Divider /> | ||
<Checkbox label={<label><b>Element label</b></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.
Though this solves the Checkbox usage, I don't see the FormField updates as the title states. |
I'm thinking if a user does this: <Form.Field label={<MyLabel />} control={<MyControl />} />
<Form.Field label={{ children: 'Name' }} control={<input name='name' />} /> That we should pass that ^ value through the factory as well. Currently, we assume it is a string and render it nested in a We can do this on another PR if you like and just leave this one to the Checkbox label updates. LMK. |
I've added updates for |
I think we don't need touch |
Awesome work, thanks :D I updated the PR description to not close #939 since the PR adding shorthand to the FormField label should close that one. |
Hm... It seems there happened misunderstanding. This PR improved <FormField label='my label' />
<FormField label={123} />
<FormField label={{ children: 'foo' }} />
<FormField label={<div className='label' />} />
<FormField label={<MyComponent />} /> But, we can't do this: <FormField control={<input name='name' />} /> In this case So, I think #939 can be closed, but we need to open new issue about |
Indeed, thanks for clarifying. I've closed the other issue. |
Addresses part of #939.
This PR:
label
shorthand;label
shorthand forCheckbox
andFormField
components.