chore(button-group): update design

[dancormier]

STACKS-554

This PR contains updates to the button group component.

Updates to markup

Button styles

In order to leverage existing .s-btn styles as much as possible, this component does not significantly modify button borders. Consumers should remove any existing .s-btn__outlined from buttons within .s-btn-group.

Preventing layout shift w/ the .s-btn--text element

Consumers will need to wrap text in a span.s-btn--text[data-text="[button text]"] element to prevent layout shift. Without it, the component will mostly render as expected by may shrink/grow a few pixels horizontally.

Remaining concerns @CGuindon

1. This design is wider than the previous design, which could cause layout shifts/wrapping in for consumers
   • I found them to be 5-15% wider depending on the contents of the button group
2. Do we need to consider hover/active states specifically for these buttons in the context of button group?
   • Specifically, the hover state is very subtle for the .s-btn__muted with the border color removed
     • ✅ Addressed in this comment

---

Original notes for posterity (Feb 2, 2024)

@CGuindon This PR has a preliminary version of the updated button group component. There are a quirks that are probably worth discussing.

1. This design is wider than the previous design, which could cause layout shifts/wrapping in for consumers
   • I found them to be 5-15% wider depending on the contents of the button group
2. I haven't found an acceptable way to compensate for badges contained within buttons in a button group and I'm concerned there might not be a reasonable way to handle this
   • This is because the badge is a standalone element with it's own padding/margin that can't be reproduced in the :before pseudo-element approach that I've taken
   • The best option I see at this stage is to deal with the slight bit of layout shift (which I'm not into)
3. Do we need to consider hover/active states specifically for these buttons in the context of button group?
   • Specifically, the hover state is very subtle for the .s-btn__muted with the border color removed
4. Consumers will need to remove s-btn__outlined from any existing buttons within button groups.
5. The approach I took will require consumers add a data-text=[button text] attribute to the button
   • It won't be difficult, but possibly a little tedious
   • If this is omitted, the button group is likely to shift around slightly when selecting a button due to the bold text

Note
Note for Dan - The docs will need to be updated to detail data-attr usage

[netlify]

✅ Deploy Preview for stacks ready!

Name	Link
🔨 Latest commit	537defb
🔍 Latest deploy log	https://app.netlify.com/sites/stacks/deploys/65de61c62b2aee0008b2e340
😎 Deploy Preview	https://deploy-preview-1630--stacks.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[CGuindon]

@dancormier — if I understood the concern correctly, no, not as part of this change.
Hover state: we're matching muted button which is also very subtle but feels like it's own issue we can worry about at a later time
Hover + Active — looks like we don't do any visual changes for that on Buttons

[CGuindon]

Not sure if this is a Stacks thing or a Radio thing — in the last button group example, the hover state is different on the active button. We don't have that now... so is that on purpose? Our other active states don't have a hover, should this new version maintain that?

Screen.Recording.2024-02-06.at.4.15.25.PM.mov

[giamir]

I really struggle to accept that consumers will have to write the same text twice (see the data-text attribute) with this approach. I would be ok with it in a fully componentized world where those are only implementation details (but that's not the case for Stacks Classic - all of this markup will be copy/pasted in several places with the risk of typing mismatch between the 2 texts).

Have we considered alternative approaches like text-shadow? It's not exactly bold but it could serve the same purpose of distinguishing the active state in a way that should be way cheaper for us to implement. Perhaps something worth discussing with the design team as a compromise?

Thoughts?

[dancormier]

Have we considered alternative approaches like text-shadow?

I don't view text-shadow as the equivalent to bolded text. Bolded fonts are custom designs of a given font whereas text shadow is a bit of a blunt instrument. We'd lose crisp edges and wind up with kinda a blobby look to the selected element.

Using text shadow

Using bold

My desired approach (that's not yet supported) FWIW

Eventually, browsers will support content: contents which will just reproduce the contents of a given element as the pseudoelement. I don't know when this will be available in browsers though.

[dancormier]

Not sure if this is a Stacks thing or a Radio thing — in the last button group example, the hover state is different on the active button. We don't have that now... so is that on purpose? Our other active states don't have a hover, should this new version maintain that?
Screen.Recording.2024-02-06.at.4.15.25.PM.mov

Ah, this is a bug. We have hover styles applied when a button does not have the is-selected class on it. This works as expected with the standard button groups, but doesn't on radio button groups since the is-selected class is never applied. Nice catch @CGuindon. It'll be a tricky issue to solve but I'll poke around and see if I can figure it out in this PR.

[dancormier]

Ok, I figured out the hover issue on radio-based button groups (the solution is pretty simple but was surprising tricky to get to). This should be ready for review now 🎉

[giamir]

Looks good to me @dancormier. Nice work.
Once this PR is merged - what's the plan?

• Cut Stacks Classic 2.4.0
• Perform the upgrade in Core (ourselves given that this can be considered an a11y fix and we have this new mandate now?)
• (???) Upgrade the Razor library markup

[dancormier]

Looks good to me @dancormier. Nice work. Once this PR is merged - what's the plan?

• Cut Stacks Classic 2.4.0

Stacks Classic 2.3.0 I believe, but yep

• Perform the upgrade in Core (ourselves given that this can be considered an a11y fix and we have this new mandate now?)

I'm comfortable with us performing this upgrade

• (???) Upgrade the Razor library markup

Yep, though this is by no means a rush. Currently the ButtonGroup Razor component is not referenced in Core.