Revisit Button Group HTML

[kwinters]

In 7727341 the button group code was changed from buttons to labels, Closes #1323. Closes #1482

But I think it's time to revisit that decision (2013).

http://getbootstrap.com/javascript/#buttons-checkbox-radio uses a label and an input, and none of the examples in http://getbootstrap.com/components/#btn-groups have labels at all.

The above commit's message is "Align with BS3 markup." but the code does not meet that goal, at least anymore. From the other threads, it looks like the example started with the input + label format and then determined that the inputs were not required, ending up with just label. However it appears that the typical usage is to use either actual buttons or divs, not orphaned labels.

And I think that the use of labels in this way should be discouraged in general. Labels tend to have styling associated with them that are not appropriate for an element that acts like a button.

[rvanbaalen]

@kwinters I think you have a totally valid point there. @chrisirhc Any idea why the buttons where changed to labels back then?

If the labels would contain a checkbox/radio input it would make sense but they don't.

Btw; we're looking into full Bootstrap compatibility for milestone 0.14.0.

[chrisirhc]

This refers to the demo site's code. We can improve on this by including the input checkboxes but state in that they're not required. As it's a lot more code but it's good for accessibility.

Edit: See below.

[rvanbaalen]

After digging in and checking bootstrap's source I would say for semantical and accessibility reasons we should add an <input> inside the <label> tag.

But, after I checked out Bootstrap's CSS I noticed that they target the internal <input> element with a css selector prefix:

[data-toggle=buttons]>.btn input[type=checkbox] {
  position: absolute;
  clip: rect(0,0,0,0);
  pointer-events: none;
}

So either we have to add data-toggle="buttons" to the wrapping div or we have to handle the hiding of the checkbox / radio button ourselves within the directive.

@chrisirhc beat me to it.

[rvanbaalen]

I would say adding [data-toggle="buttons"] adds no added value to our templates and could possibly result in unwanted behavior. My vote goes to hiding the <input> ourselves within the btnCheckbox directive.

[chrisirhc]

I see, that sounds reasonable. If a btn-checkbox element contains any input elements within, hide them. Encourages better accessibility code. However, take note of the notes written around the styles: https://github.com/twbs/bootstrap/blob/master/less/button-groups.less#L221-L243

// In order to support the browser's form validation feedback, powered by the
// required attribute, we have to "hide" the inputs via clip. We cannot use
// display: none; or visibility: hidden; as that also hides the popover.
// Simply visually hiding the inputs via opacity would leave them clickable in
// certain cases which is prevented by using clip and pointer-events.
// This way, we ensure a DOM element is visible to position the popover from.

So we might want to use the same styles.

[rvanbaalen]

Good point! I'll take care of this after 0.13 is out.