Checkbox with label is rendered wrong

[seriema]

Issue

According to the readme it's possible to just add a standard <label> element after the checkbox.

        <Checkbox
          nativeControlId='my-checkbox'
          checked={this.state.checked}
          indeterminate={this.state.indeterminate}
          onChange={(e) => this.setState({
            checked: e.target.checked,
            indeterminate: e.target.indeterminate})
          }
        />
        <label htmlFor='my-checkbox'>My Checkbox</label>

Will render a checkbox looking like this, with the label wrongly positioned and wrong font:

Example code here:
https://stackblitz.com/edit/react-wpvkxv

Possible solution

It's missing the Form Field CSS. There is no Form Field React component so this is the best solution I found but it seems wrong to have to do it like this:

• add form field CSS from package @material/form-field
• use a wrapping div around the <Checkbox> and <label> with the class "mdc-form-field"

Then it looks as expected:

Example code here:
https://stackblitz.com/edit/react-stbwsz?file=Checkbox.js

[moog16]

@seriema yes, adding the form field component is the correct solution. We never created the form-field component since it is just styles. But if you'd like to create a component for it please feel free :)

[seriema]

@moog16 Something like this? https://stackblitz.com/edit/mcwr-form-field?file=form-field%2Findex.tsx
Not sure what to do with the input expected by form fields: https://github.com/material-components/material-components-web/tree/master/packages/mdc-form-field#mdcformfield-properties-and-methods
When is there a form field with an input? The text field already handles its own label and animation. Checkbox and Radio button don't have inputs.

[moog16]

@seriema I see what you mean - it seems to work without those adapter methods. I don't see why we need them as its handled by the checkbox and radio. I think for now we can leave it as you have it. FormField was not designed with React in mind. This component is really more about the styles/scss.

PS: I took a look at RMWC. James also doesn't implement the adapter. This leads me to be more confident in my opinion above.

[seriema]

I just noticed that Radio already wraps itself in 'mdc-form-field'. So either:

1. Checkbox should be like Radio: having 'mdc-form-field' wrapping div and optional label element
2. Radio should be like Checkbox: not have a wrapping 'mdc-form-field' and no label element
   They're also different in that Radio needs a NativeRadio child, while Checkbox just renders the native checkbox (+ it's stylized checkmark) for us.

Both Radio and Checkbox MCW "Basic Usage" says:

We recommend using MDC Radio with MDC Form Field for enhancements such as label alignment, label activation of the ripple interaction effect, and RTL-awareness.

I'd go with nr 1 for solving my original issue, as the issue doesn't exist on Radio. What do you think @moog16?

[moog16]

@seriema gotcha - when we chatted in discord, I thought you were talking about MDCW. I took a look at MDCReact Radio/Checkbox. I think #1 also makes sense to me as well. Sorry for the miscommunication.

[seriema]

I noticed issue #438, where you say the checkbox should be built through composition with a <CheckboxInput>. Radio calls it <NativeRadioControl>. That means extracting another component as well as the changes discussed in 1. A much simpler interface would be this:

1A

But it's usually better with composition (as noted in #438) in the long run, so I'd propose this:

1B

Switch is a similar component and currently looks like Checkbox so whatever we choose here should be applied to Switch.

Need to keep #308 in mind. Are we aiming for option 1B then @moog16 ?

[moog16]

#308 is actually pretty easy as we have added it to drawer (see attachRef method). That can be addressed at any time.

I believe that all components are better to be used through composition. So switch, radio, checkbox should all be built the same. So it'd be great to have 1B, however I think converting checkbox over is a lot more work than you probably asked for.

[lbell]

import "@material/form-field/dist/mdc.form-field.css";

for css -- apparently.

(Found this all woefully confusing.)