Checkbox: Labels don't have htmlFor

[bmsterling]

Steps

Using the wave toolbar

1. Navigate to https://react.semantic-ui.com/maximize/checkbox-example-checkbox
2. Run the wave toolbar, notice the "missing form label error"

Expected Result

All labels are connected to inputs via the for attribute

Actual Result

No for attribute

Version

0.71.3

I'm working on a fix for this that is similar to these lines of code but wanted to ask some questions before I submit a PR.

1. The for attribute value should be the id of the input that it relates to, but id is not a required proptype. I'd like to make that required but I'm not sure what the ramifications will be since as prop allows for a checkbox to be some other type of html element. Does anyone see a scenario where a required id will cause issues?
2. In the same thought, the id, if it is passed in, is applied to the div and not the input, again, I don't think there is any ill affects to having it applied to the input, but wanted to confirm?

[brianespinosa]

@bmsterling Here's a codesandbox showing how you can set the htmlFor property on the label. https://codesandbox.io/s/82wnl3np48

[bmsterling]

@brianespinosa That's an interesting approach but it causes the below warning and doesn't add the id to the input, continuing the original issue:

Warning: Unknown prop input on <div> tag. Remove this prop from the element.

[bmsterling]

@brianespinosa I believe partitionHTMLInputProps(unhandled, { htmlProps: [] }) is what prevents your example from adding the id to the input.

[brianespinosa]

@bmsterling fair point. I didn't see that in the console when I threw that together for you. Let me go take a look at how I handled this in my own project. I'd tend to agree with you that we might want to add a way to more easily handle this. Using id and passing that down might be a breaking change for some people though.

@levithomason or @layershifter thoughts on this? Maybe there's another solution that is already there that works better?

[bmsterling]

passing that down might be a breaking change for some people though

True. I didn't consider this.

Very roughly, and just an fyi, the changes I made to Checkbox.js are as followed:

const [htmlInputProps, rest] = partitionHTMLInputProps(unhandled, { htmlProps: ['id'] })
const id = _.get(htmlInputProps, 'id')

and

{createHTMLLabel(label, { defaultProps: { htmlFor: id } }) || <label htmlFor={id} />}

Then I can do:

<Checkbox label='Make my profile visible' id='myid' />

[layershifter]

@bmsterling Thanks for the report. The proposed solution looks correct, howerer I think that we can pass all htmlProps to the input, so it should be:

const [htmlInputProps, rest] = partitionHTMLInputProps(unhandled)

Feel free to open a PR 👍