fix(btn): don't render visited colors on outlined btn

[dancormier]

Quick bug fix for unexpected border color being applied to .s-btn.s-btn__outlined:hover:visited. When an outlined button is being hovered and is visited, it shows a black border when it should show a theme color border. This happens in Chrome but not Firefox and seems to be an issue with how a given browser honors the cascade.

From slack thread:

[abovedave]

Thanks @dancormier. How can we test this? I'm wondering if it's worth adding <a href="..." class="s-btn"> example to the docs so we have that variation represented?

[giamir]

It would be nice to add visual regression tests also for hover and visited states, although I think it would create a huge amount of screenshots even if we generate them in a separate test file. An option for snowflakes like this could be to just test that specific variation as a one off in a separate file (e,g, button.snowflakes.visual.test.ts) manually without our test-utils method.

I will leave it up to you Dan if you want to spend some time experimenting with it or save it for later.

[dancormier]

Thanks @dancormier. How can we test this? I'm wondering if it's worth adding <a href="..." class="s-btn"> example to the docs so we have that variation represented?

@abovedave there are so many variants of the button that to properly illustrate anchor-based s-btns, we'd bloat the docs significantly. We could add a single base button example, but that wouldn't show the issue this PR resolves because it's on a variant of s-btn and adding that specific variant would prompt a discussion of what anchor+variant combos to include or omit.

As far as testing goes, I only have a clunky suggestion of running this locally and temporarily modifying the docs page to include this variant, then resetting the change.

It would be nice to add visual regression tests also for hover and visited states, although I think it would create a huge amount of screenshots even if we generate them in a separate test file. An option for snowflakes like this could be to just test that specific variation as a one off in a separate file (e,g, button.snowflakes.visual.test.ts) manually without our test-utils method.

I will leave it up to you Dan if you want to spend some time experimenting with it or save it for later.

@giamir I considered it, but I think it's a similar issue as with what I said above about the docs. Of the ~2600 images currently generated, over 900 of them are for the button component already. If they ran really quickly, I'd consider adding a test to generate hover/visited states, but I'm apprehensive to add more visual button tests since they already take a while to render.

[giamir]

What about the snowflake idea? Just testing this specific case combo where we observed the issue? That should only add 3 more images I guess - one per browser. Not sure if it's worthy though.

[dancormier]

@giamir I spent a moment today trying to create a test to generate an image of this button in its hover and visited state. Just speaking of hover, I tried using fireEvent (mouseEnter, mouseOver), userEvent (user.hover(button);) and through dispatching an event (document.querySelector("a").dispatchEvent(new MouseEvent('mouseover', {…}))).

None of these approaches worked to have simulate a hover state. I also tried a few ways of simulating visited (such as setting window.location.href to the match the hash of the link href), but that also didn't work. I remember trying to test pseudoclasses in the past and struggling. There's a chance I'm missing some obvious way to test these states, but if I'm not I think I'd rather try and tackle testing pseudoclasses separately at some point.

[giamir]

Agree. We can merge this PR as is. I have spent some time as well, checking how easy would be to test :hover and :visited pseudoclasses. I got to test :hover styles reliably but I found a blocker on :visited given that browsers tend to make it tricky for privacy concerns. I even got playwright/chromium to start in a non-incognito window but it did not help somehow. I will clean up the spike I did and store it in a branch so that we don't start from scratch when we want to start supporting this type of testing.