fix(button): ensure visited link button renders legibly

[dancormier]

In the course of reviewing https://github.com/StackEng/StackOverflow/pull/18700#issuecomment-1936535576, a bug was discovered where a visited link with .s-btn would be illegible in HC mode. This was due to a selector being mis-scoped in https://github.com/StackExchange/Stacks/pull/1613. This PR resolves that mistake.

Questions page sort button text color doesn't change when selected/visited in HC mode

Sidenote: @giamir This is a situation where visual regression tests for interaction states may have caught the bug before it was shipped. A nice real-world case that proves the value of interaction state visual regression test.

[netlify[bot]]

✅ Deploy Preview for stacks ready!

Name	Link
🔨 Latest commit	808f8c57e6ca6b3f78d3fcd3fd5bedbce5334738
🔍 Latest deploy log	https://app.netlify.com/sites/stacks/deploys/65ca49a425786f0008564483
😎 Deploy Preview	https://deploy-preview-1640--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.

[dancormier]

@dancormier Thanks for this fix. Unfortunately I had a hard time manually testing this change. Initially I did not understand why we were using the :visited pseudoclass on button. It looks like this is for button groups with anchor elements (instead of actual button elements). I took the liberty to create a small extra section in the docs about this use case where the s-btn is an a tag so that I could test easily.

@giamir Sorry for the difficulty testing. Thank you for reviewing and adding the anchor-based example.

Sidenote: @giamir This is a situation where visual regression tests for interaction states may have caught the bug before it was shipped. A nice real-world case that proves the value of interaction state visual regression test.

Yes, tests would also be nice as a follow up steps but I think in this specific instance we were missing also the docs describing the use case.

Sidenote:

There is a small style bug when we wrap the first or the last button group items in a form element (see screenshot).

Yep, that's a long-standing docs bug. I did a quick scan of Core to see if there's an instance of use applying form elements like this and I didn't find any. think we could safely remove the child form elements from the s-btn-group examples. Do you agree @giamir?

This was already there before but you might want to consider creating another small PR to address the issue or log a bug ticket. I was also confused about the use case for wrapping button groups items in a form element and I also find the role="button" applied to many examples redundant. What am I missing? 🤔

That's cruft. It should instead be type="button". From MDN:

Adding role="button" tells the screen reader the element is a button, but provides no button functionality. Use or with type="button" instead.

I'll take this branch as an opportunity to make that change.

[giamir]

Do you agree @giamir?

Yes let's remove the form elements from the examples. Thank you.