ButtonGroup doesn't apply margin when children are different HTML elements

[larimaza]

Component to amend

ButtonGroup

Visual

First button is an a	Both buttons are button

Context

The ButtonGroup should apply margins between the buttons inside it for all cases.
However, because of selectors like this:

.css-185ogu-button-group-button-group--right > button:not(:last-of-type), .css-185ogu-button-group-button-group--right > a:not(:last-of-type) {
    margin-bottom: 16px;

...when the first button is an anchor and the other is a button, the margin is not applied, since the a is the last of its type.

The proposal would be to use the broader selector :last-child, since it's unlikely that the ButtonGroup will have another child that isn't an a or a button (and if it has, it's a wrong use of the component).

[connor-baer]

Thanks for the report @larimaza! This is a tricky one: Emotion puts constraints on the use of :first-child and other CSS child selectors (emotion-js/emotion#1178). Circuit UI needs to support SSR, so that's not an option.

The alternative is to iterate over the child elements with React's Children.map, clone each child, and add the styles to them explicitly. The problem here is that Emotion's css prop doesn't work with React's cloneElement function.

Another option would be to insert a spacer div between the buttons. I'm not sure what the limitations would be here 🤔

[larimaza]

Tricky indeed 😬 TBH, I don't love the idea of adding a spacer div any more than I love the idea of customizing margins in usage 🙈

The other solution is also not very nice, but... We could add a margin to every child and then counter the last one's margin in the button group itself.

[connor-baer]

That's better indeed. Emotion doesn't restrict the :last-child selector. Very clever! 🧠

[robinjimenez]

@connor-baer I can work on work on this!
I've been checking the code and doing some tests, replacing :last-of-type with :last-child seems to do the trick for both bottom and right margins

[connor-baer]

@robinjimenez Awesome! 🙌 Could you open a PR with the changes, please?

[github-actions]

🎉 This issue has been resolved in version 2.4.0-canary.6 🎉

The release is available on:

• npm package (@canary dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 2.4.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀