Button groups does not support focus customization

[vicb]

Bug description:

Button groups do not support styling the :focus state

Link to minimally-working plunker that reproduces the issue:

https://ng-bootstrap.github.io/#/components/buttons

Click on any "button" and press tab, the next button show no focus decoration

Root cause

Compare the ng-bs markup:

<div class="btn-group" data-toggle="buttons">
  <label class="btn btn-primary" [class.active]="model.left">
    <input type="checkbox" [(ngModel)]="model.left"> Left (pre-checked)
  </label>
</div>

with the bs4 markup

<div class="btn-group" role="group" aria-label="Basic example">
  <button type="button" class="btn btn-secondary">Left</button>
</div>

bs4 uses the focus style of the button - I'm not sure there is anything we can do with the current example in ngbs ?

Version of Angular, ng-bootstrap, and Bootstrap:

ng: 2.0.0
bs: 4.0.0-alpha.4
ng-bs: 1.0.0-alpha.7

[wesleycho]

I wonder if Bootstrap 4 changed its approach to button group markup at some point...I know Bootstrap 3 went the route of using the input element. We should probably considering changing it too to better match the HTML for styling reasons.

[pkozlowski-opensource]

@wesleycho @vicb there are 2 different styling for "buttons" being used:

• regular button groups, ex.: http://v4-alpha.getbootstrap.com/components/button-group/#basic-example
• radio / checkbox buttons: http://v4-alpha.getbootstrap.com/components/buttons/#checkbox-and-radio-buttons

The reason for existence of those 2 different markups is accessibility. Indeed, radio / check buttons are semantically different from regular buttons and different HTML elements need to be used for those 2 different use-cases.

Now, coming back to the issue reported by @vicb - it is a valid issue - we are not adding properly the focus CSS class on a label when an input gets focused. This should be an easy fix though. Thnx for reporting.

[alex321]

@pkozlowski-opensource Let me know if there is anything you want to improve in your commit 23d412b. It seems good to me.

[alex321]

Sorry for the noise. I can see that your commit has already gone in. What else needs to be done?

[thbt]

I would just like to point out that focusing the button using shift+tab (so focusing it from the next focused element) will put the focus on the last radio button, while in bootstrap, focus is always put on the first radio button. It's not really important and it works fine this way, is this worth a PR ?

[wesleycho]

There are two scenarios still left to explore @alex321 - checkbox buttons, and regular buttons

@thbt that sounds incorrect, PR welcome

[wesleycho]

Actually, looks like we only support radio buttons here. Going to close as it looks like this main issue is done.

[pkozlowski-opensource]

@wesleycho actually I wanted to keep this one open to revisit checkbox support before beta.