Use -of-type instead of -child to accommodate hidden inputs

[mermop]

Hello friends! Long-time listener, first-time caller 📞

This PR changes the selector for resetting rounded corners of btn elements in btn-groups from first-child to first-of-type to accommodate hidden inputs in sets of checkboxes or radio buttons.

The Rails collection_check_box or Simple Form or Formtastic all add a hidden input either at the beginning or end of a collection of check boxes or radio buttons. This makes implementing groups of checkboxes or radio buttons as per the documentation either painful or slightly ugly.

I initially put this PR in the bootstrap-rubygem repo

Markup using Rails collection_check_boxes

Screenshot using first-child/last-child

Screenshot using first-of-type/last-of-type

[MartijnCuppens]

Hi @mermop, welcome here!

I'm not eager to merge this though, we shouldn't change our code because another framework renders their markup different, especially if it's just a hidden input.

I guess you better suggest a PR for Rails collection_check_box to fix this.

Ping @twbs/css-review for second opinion.

[patrickhlauke]

I'd be in favor of this change, as first-child really is quite limiting/specific to a particular markup structure, while first-of-type is more forgiving of authors wanting/needing to have a slightly different structure to achieve the same thing.

[mermop]

My impression is that it's reasonably common to have a hidden input alongside a checkbox or group of checkboxes so that the form submits a null value if they are unchecked, not just in Rails - but it's a commonly used framework and where I came across this, so I thought it was worth including references to behaviour there.

There are other use cases though - there are lots of reasons that the first or last child of a btn-group might not actually be the btn you're aiming to target.

There are some examples of how -child can behave unexpectedly on the CSS Tricks article on the difference between these.

[MartijnCuppens]

This change won't break anything in normal circumstances and if it could fix your problem (and possibly other problems) it's fine by me.

[ysds]

See : https://deploy-preview-28997--twbs-bootstrap.netlify.com/docs/4.3/components/dropdowns/#dropleft

[MartijnCuppens]

Nice catch, @ysds!

[mermop]

Hmm, interesting!

So in that case, there's a btn-group nested inside another btn-group, which I think doesn't reflect the actual usage of components.

With this change I think you'd be able to instead use:

  <div class="btn-group dropleft" role="group">
    <button type="button" class="btn btn-secondary dropdown-toggle dropdown-toggle-split" data-toggle="dropdown" aria-expanded="false">
      <span class="sr-only">Toggle Dropleft</span>
    </button>
    <ul class="dropdown-menu">
      <!-- Dropdown menu links -->
    </ul>
    <button type="button" class="btn btn-secondary">
      Split dropleft
    </button>
  </div>

That only uses one btn-group component, which seems more appropriate for what you see.

But doing that I still see:

Which is also what it looks like without this change - because of the not(:dropdown-toggle) part of the selector.

I've removed that, I can't quite figure out what it's there for, but I'm sure it was there for a reason and will break something somewhere else - do you have a visual diff tool or anything?

[MartijnCuppens]

do you have a visual diff tool or anything?

Nope

I'm ok with unnesting the btn-groups. @ysds, what do you think?

[mdo]

So in that case, there's a btn-group nested inside another btn-group, which I think doesn't reflect the actual usage of components.

We support this and making any changes here would be a breaking change. https://getbootstrap.com/docs/4.3/components/button-group/#nesting

[MartijnCuppens]

I'm going to close this, since the solution provided will break the droplefts or nested button groups. On the other hand this feels like a fix for a case we don't support.