v5: Move from JS input button group toggling to all CSS

[mdo]

Takes #25185 and starts work on the docs and JS to drop code that was previously handling active states.

/cc @ysds

• make sure the CSS changes are OK
• make sure the JS changes are OK, tests pass, visual tests are also OK
• Check if we need to revisit bundlesize limits
• Finally, make sure all functionality we had before is still there, especially accessibility-wise /CC @patrickhlauke

[ysds]

The CSS LGTM👍 IMO, it is better to move the documents to the button group page.

[XhmikosR]

Do not merge yet, @mdo, see the first post for TODO.

[XhmikosR]

And who's going to handle aria pressed?

…

On Tue, Mar 12, 2019, 22:48 Mark Otto ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In js/src/button.js <#28463 (comment)>: > - if (triggerChangeEvent) { - this._element.classList.toggle(ClassName.ACTIVE) - } + this._element.classList.toggle(ClassName.ACTIVE) Yeah, we might be able to nix all the JS—can anyone confirm that? Only reason I could see us leaving some JS for a non-input implementation is quickly toggling active on/off, but I feel like most folks could do that themselves with very little code and their own syntax. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#28463 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVVtVsLpVRuedaqHbVJKgo5IdqJ9DvSks5vWBK3gaJpZM4bp4Wd> .

[ysds]

To handle the aria-pressed, it will be difficult to solve the #25122 and #27600 issues as long as we use <input type="checkbox"> and <input type="radio">. And I'm not sure that mix of :checked and aria-pressed is valid.

According to @scottaohara's explanation, there is a radio button group implementation pattern without aria-pressed.

Also, his comment at the end of the article suggests a grouping of <button>s. It is probably something like the WAI-ARIA practice.

Alternatively, a grouping of button elements that are aware of each other's state, and utilize aria-pressed="true/false" to indicate their own state, may be more appropriate and understandable to users.

So in terms of Pure CSS and accessibility, we'll need decide to which one:

• A. Implement by Pure CSS (<input>) without aria-pressed
• B. Implement with <button> using JavaScript to control the aria attributes

[scottaohara]

Also, his comment at the end of the article suggests a grouping of <button>s. It is probably something like the WAI-ARIA practice.

that could be one method but not necessarily the only method, in that a grouping could be just a group of buttons, rather than a toolbar. More importantly though, each would provide different user expectations to the manner in which they were keyboard accessible, as would checkboxes and radios.

As I'm not deeply familiar with what it is you're trying to solve for here, and honestly i'm a bit confused reading through just this thread with the number of different elements being referenced, I'd definitely defer to @patrickhlauke's guidance.

[patrickhlauke]

coming in late on this...so: skimming over the changes, it looks like we're going for clean CSS version here? just layering over texisting <input> elements? in that case, if those inputs are there and focusable (and not suppressed), there won't be need for any JS or aria or anything, unless I'm missing something.

[XhmikosR]

Yeah, the idea is to use only CSS for buttons. If we don't need to handle aria, I can update the branch dropping more stuff.

[patrickhlauke]

ah, hang on...i think the confusion here is because there's two separate things going on...there's the "checkboxes and radio buttons that look like a toolbar/button strip" https://getbootstrap.com/docs/4.3/components/buttons/#checkbox-and-radio-buttons (which is what wouldn't need any JS nor aria-pressed or anything, and can be solved purely with CSS), and then there's the "set of buttons that acts as mutually exclusive toggles" https://getbootstrap.com/docs/4.3/components/button-group/ (which DO need aria-pressed to denote their current toggle state)

I'll be honest, i find these two things related, but confusing. Most confusing that the checkbox/radio ones are under the "Buttons" documentation - should they perhaps be under their own component topic for checkbox/radio button group?

[XhmikosR]

Hmm then I'm also lost. Not sure how to proceed. Maybe we should rename button to something else and only keep what we need in JS?

[patrickhlauke]

incidentally, am i missing something, or are the groups in https://getbootstrap.com/docs/4.3/components/button-group/ not actually doing anything fancy JS wise at the moment? are they supposed to have aria-pressed and act like sets of radio buttons or not? the "button plugin" mention on there simply points back to the buttons page...so seems we're making some kind of circular reference here.

worth taking a step back and deciding overall what we want. certainly the easiest part is:

• we can style checkboxes and radio buttons with CSS to appear button-like; behind the scenes, it's still just an <input> and a <label>, and uses CSS - no need for JS/ARIA. this is probably a special case of form element styling, not necessarily/logically something that should fall under "button" component documentation?
• for <button> elements to act as on/off type toggles, they need JS and aria-pressed handling

Now, button groups are like an extended version of the single button that acts as a toggle. A group can be mutually exclusive (acting like a set of radio buttons) or not (like a toolbar in a wysiwyg tool, where each on/off toggle is independent - set bold, or italic, or underline, independently. Those, again, need JS. and for the mutually exclusive capability, they need JS beyond just the pure toggle JS functionality, as it needs to not only toggle aria-pressed on what the user selected, but also unset it on any others in the same group.

Conceptually then, the single toggle button plugin thing, and button groups, probably should go into their own component documentation page together, as they're thematically related (groups build on the single toggle). Whereas the implementation using radio/checkbox is just a differently styled set of form controls?

[mdo]

Answering some questions here...

• Buttons in button groups are not intended to include JS functionality. Button groups are a component for controlling layout of buttons. None of these buttons need JS from us—anyone can write their own JS to toggle the .active class.
• My proposal earlier is to drop our button JS plugin and use only <input>s and <label class="btn"> combos.
• Given that, and per @ysds's feedback, we can then move the input/label examples to the button group docs page.

If all that's true and agreeable, it sounds like we don't need any JS here or any aria attributes. If that doesn't sound right, let's reframe the conversation around "how do we remove all this JS?" and answer any questions that come from that.

[patrickhlauke]

not a fan of the use for dropdowns either, but can more easily rationalise that in that it groups a button and a dropdown together. less so when it's grouping only one thing (and a hidden radio/checkbox)

[patrickhlauke]

incidentally, the .btn-input-wrapper class i proposed here could be renamed to something more generic like .btn-wrapper, and applying that to those dropdowns would have the same effect as well.

or perhaps taking a step back, renaming .btn-group to .btn-wrapper, and patching in any extra styles/selectors created here for .btn-input-wrapper and applying them to that, could also work. gives the class itself a more generic name, and it's then applied in different contexts where it "wraps" various things relating to a button-like look.

[ysds]

I don't stick to the name of the .btn-group. I just don't want to increase the complexity of selectors and CSS. The changes to the _button-group.scss are very complex for now.

Looking for a wrapper class name (alt .btn-group) that everyone is convinced might be a great solution.

[patrickhlauke]

btw note that even if we just reused .btn-group it wouldn't remove the need for all of the new selectors (like .btn-input), as they cover scenarios that weren't covered before just through .btn-group

[patrickhlauke]

@mdo let me know if you want me to have a crack at renaming btn-group (and what your preference would be, if any) and re-simplifying some of the css selectors?

[ysds]

btw note that even if we just reused .btn-group it wouldn't remove the need for all of the new selectors (like .btn-input)

Yep, I showed that (just adding one new class) in my original PR #25185.

[Johann-S]

Hey @twbs/css-review and @twbs/js-review I removed QUnit and switch to Jasmine, so now your unit tests should go there: https://github.com/twbs/bootstrap/blob/master/js/src/button/button.spec.js

js/tests/unit/button.js have been removed, so if you need any help do not hesitate 😉

[patrickhlauke]

@mdo just bumping this to see if it's likely to be used/picked up/worked on further...

[patrickhlauke]

so...coming back to this, as i worked my socks off at the time to reorganise stuff, create some more explanations and examples, etc ... what's happening? should this all be closed? is it too far gone to salvage the bits that aren't controversial (since the creation/renaming of the class wasn't to everybody's liking)? /cc @mdo

[patrickhlauke]

so... 5 months later, and still tumbleweed on this? makes me regret having put time and effort into trying to rationalise/improve stuff here to be honest...

[XhmikosR]

Who's up to do a proper rebase for this branch? 😇

[patrickhlauke]

cursed branch :)

[ffoodd]

Just discovering this one! Looks interesting but pretty confused with all the things going on here, so if anyone's able to summarize the need, I'd be glad to help.

And if not, this topic might not be clear enough!

[patrickhlauke]

in very broad strokes, from what i remember, this change does the following:

• kills off the clunky JS-based button groups for checkbox/radio button groups, instead relying purely on using the underlying <input type="radio"> / <input type="checkbox"> ... avoiding a LOT of the extra messing around we have had with these components in the past (all to make them behave like real radios/checkboxes)
• renames (currently, though that was then a big point of contention) the button group class
• reorders and expands examples to clarify and more clearly distinguish between the styles to make button groups and then the actual dynamic component to handle ARIA-based toggles

[ffoodd]

If I understood correctly, we have three features that kind of interact with each others:

1. styling a button group — without any specific behaviour, just to style multiple appended buttons of any kind;
2. first behaviour: using radios or checkboxes;
3. second behaviour: using JS and aria-pressed.

And after reading this dicussion carefully, I have a question: the first need for this PR was to remove JS. I can't see anyone in this discussion arguing to keep this JS part (except the fact that it already exists…). Am I wrong?

@patrickhlauke IMHO both behaviours are to be accomplished with inputs.

Is there any reason to keep mentionning the ARIA pattern here? Am I missing something?

[patrickhlauke]

ARIA is still needed for toggle buttons (<button aria-pressed="true">). faking them using, say, an <input type="checkbox"> is not ideal/recommended.

[patrickhlauke]

not to keep flogging a dead horse, but... @mdo et al, any idea on this whether we want to proceed or not?

[MartijnCuppens]

@patrickhlauke, I'll have a look in another PR with another approach. So you're ok with dropping the js think with active classes and just rely on the CSS only label-buttons for v5?

[patrickhlauke]

yes, the move to CSS only (except for controls/toggles that need aria-pressed) is non-controversial. but this PR also contains reordering/reorganisation of some docs aspects (moving the radio button group stuff, making up new classes for those situations, and lots more specific usage examples). it's that stuff as well that i'm quite keen to get a decision on

[mdo]

Closing for #30650.