halfmoonui / halfmoon

Halfmoon is a highly customizable, drop-in Bootstrap replacement. It comes with three built-in core themes, with dark mode support for all themes and components.
https://www.gethalfmoon.com
MIT License
3.03k stars 118 forks source link

Allow wrapping custom checkbox and radio inside label #47

Open MuhsinFatih opened 3 years ago

MuhsinFatih commented 3 years ago

The custom checkbox and radio classes require the use of the labels' for attribute: https://www.gethalfmoon.com/docs/checkbox/

The id="..." attribute of the input and the for="..." attribute of the label should have the same value.

Is it possible to support the following syntax?:

<label>
<input type="checkbox" name="checkbox" value="value">
Text
</label>

The advantage being that this avoids accidental duplicate id's, i.e.: if you copy paste the form, you have to rename the checkboxes when using label for, otherwise the label clicks will toggle the wrong checkboxes

halfmoonui commented 3 years ago

I like the syntax that you proposed, but is the lack of IDs on checkboxes a good idea? Seems like it could lead to accessibility issues. I know the docs don't have things like aria-labelledby at this moment, but I intend to add them in the near future hopefully, so even if only for accessibility issues, you would still need an ID on a checkbox.

MuhsinFatih commented 3 years ago

I couldn't figure the purpose of id for accessibility where there already exists name, but upon reading about this accessibility tutorial on label for, it seems it does indeed help. Funnily though before reading that, this was the workaround I implemented in my code 😄

<input
    type=checkbox
    bind:checked={todo.done}
    id={`u0MVRY4-${i}`}
>

(I generated a random base64 string and prepended the id with it so that I don't accidentally give same id's even while trying to avoid that). I didn't know about the accessibility implications of course 🙂 . Now it's head-scratcher though: do I choose accessibility or maintainability? 😄 I'm not very knowledgable about the details, but if I understand correctly, aria-labelledby can be used with implicit labelling to achieve the same accessibility?

Edit: This example code is svelte's jsx syntax by the way for anyone wondering

halfmoonui commented 3 years ago

I am no accessibility expert (trying to get better though), but from what I understand, aria-labelledby is used to establish a relationship between an element and its label.

May I ask though, wouldn't you be better off using semantic IDs for your checkboxes? For example, a remember password checkbox could have a remember-password-checkbox ID. Do you have a lot of checkboxes on your page that it becomes difficult to give them all semantic IDs?

MuhsinFatih commented 3 years ago

I don't have any complicated projects right now, I'm just speculating. I can see it being a problem especially when using different components. Two different components could easily have a checkbox with similar functionality, but their id's are not scoped so they can easily clash. Although now that I think, the id would be needed in vanilla js for app logic anyway so I can see this being a non-issue normally. But I guess this struck me as a problem because I'm thinking with component-based app mentality where with svelte I use bindings which are scoped inside each component instead of id's which are not scoped and hence can clash