chore(links): update visited links colors and application

dancormier commented 1 month ago

This PR updates the visited link style to purple-500 (and purple-600 when visited and hovering) where appropriate.

Changes of note

This PR changes the visited link style for all anchors, with the exception anchors of Stacks components that should not ~~(see this change for the list of unaffected elements)~~. I'll be testing this in Core, but would appreciate feedback on this approach in the meantime.
I've applied styles based on the LVHA-order (:link — :visited — :hover — :active). See the :visited MDN page for more details

Testing these changes

[!TIP] The below seems to not work in incognito/private browser modes since (TIL) these modes do not honor visited link styles (at least in Mac Firefox and Chrome)

Visit the link component docs page
Observe the color of the "Visited" link (this one has forced visited style)
Go to https://deploy-preview-1735--stacks.netlify.app//product/components/links/# (note the # at the end. Observe that the appropriate links are now purple-500. Ensure this renders as expected in all modes.
Visit other components that include links to ensure they only gain this purple-500 when expected

netlify[bot] commented 1 month ago

Deploy Preview for stacks ready!

Name	Link
Latest commit	6cc395e02ffcabe81433300864ac51b89e2b74ba
Latest deploy log	https://app.netlify.com/sites/stacks/deploys/6647d3bf370643000895111e
Deploy Preview	https://deploy-preview-1735--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.

giamir commented 1 month ago

Is it just me or is there no more hover color change on the Visited link? I think it use to go up/down a stop depending on dark/light mode. I'm looking at the Stacks docs now and in the PR env here

I can reproduce the same. It looks like now visited links are always purple-500 no matter if hovered on or not. Before they went from theme-secondary-500 (visited, not hovered) to theme-secondary-400 (visited, hovered). The figma comp doesn't specify what to use for visited+hover status though.

CGuindon commented 1 month ago

I would keep the same logic of color stops purple-400 for visited, hovered

dancormier commented 1 month ago

I would keep the same logic of color stops purple-400 for visited, hovered

I've added purple-400 for visited+hovered links in https://github.com/StackExchange/Stacks/pull/1735/commits/20f10fb1552d47b4d07bae87855a171792343e9c

input message

I noticed tests were failing for the s-input-message component and realized that the visited link style was being applied to the link within that element (for some reason, our visual tests render those links as if they're visited no matter what 🤷‍♂️). I found that we use a different color pattern than our usual 400/500/600 pattern for default/hover/visited.

@CGuindon would we be okay to punt on visited link styles for anchors within .s-input-message? I can add it but I'm hesitant to introduce more complication at this moment.

CGuindon commented 1 month ago

@dancormier

weird that the the s-input-message component automatically uses visited links — but that's a problem for another day
Ideally these remain the colors they are now — warning message link stays red. If we punt on this component, would those links be purple or would they stay as they are now?

dancormier commented 1 month ago

@CGuindon

1. weird that the [the s-input-message component](https://deploy-preview-1735--stacks.netlify.app/product/components/inputs/#validation-examples) automatically uses visited links — but that's a problem for another day

To be clear, the links only show up as visited in the visual tests images, not by default when the component is rendered.

2. Ideally these remain the colors they are now — warning message link stays red. If we punt on this component, would those links be purple or would they stay as they are now?

They would stay as they are now (visited state will not change the color of the link within the input messages)

CGuindon commented 1 month ago

I can see in the inspector that _li-fc-hover-visited: is set to purple-400 but when I hover, the color isn't changing. Checking this in the PR env.

Also... (and this is my fault for not catching before) but since we are going darker for blue links on hover:

default blue-400
hover blue-500

shouldn't we go darker for purple as well on hover?

visited default purple-500
visited hover-purple-600

dancormier commented 1 month ago

@CGuindon @giamir I decided to ditch the :not(.s-badge):not(… approach I was taking before in https://github.com/StackExchange/Stacks/pull/1735/commits/28657166a01cb456c13498006683bf82fdfcd70b and instead go with what's in https://github.com/StackExchange/Stacks/pull/1735/commits/aecad2a75ea2796fb369cbcda02b39ca92713ade.

This is the most significant change:

a {
    &:visited {
        &:not([class*="s-"]),
        &.s-link,
        &.s-sidebarwidget--action,
        &.s-user-card--link {
            &:hover {
                color: var(--_li-fc-hover-visited);
            }

            color: var(--_li-fc-visited);
        }
    }
}

This is specifying to apply these styles to anchors that don't include a class starting with s-, with the exception of s-link, s-sidebarwidget--action, or s-user-card--link. IMO, this is way safer and simpler than the previous because it no longer requires this long list of :not exclusions that increases the specificity of the rule.

@giamir I'd like to get your thoughts on this approach and it's relative safety when compared to the previous approach when you have a moment.

The stuff below is nonsense, so feel free to ignore it.

Story time (aka TIL about properties allowed in `:visited`)

The big problem was with specificity. If we applied styles to a:visited, most visited links would gain that style even if we have other non-visited styles on that element (think a .s-btn on an anchor). This is not what we want.

I had a lightbulb moment: what if we specificy CSS variables on a:visited so the variable is a high specificity but the style being applied would stay only on the a selector so it could be easily overwritten.

a,
.s-link {
  --_li-fc: var(--theme-link-color, var(--theme-secondary-400));
  …

  &:visited {
    --_li-fc-override: var(--purple-500);
  }

  color: var(--_li-fc-override, var(--_li-fc));
  …
}

This seemed like a winner. I implemented it and was… very confused. The CSS variable was being picked up by the browser. The style said it was applied in the inspector. But, when I looked at the page, the color defined as a CSS variable in :visited wasn't applying. What's the deal?

The TIL

You can only set specific CSS properties within :visited for security/privacy reasons 🙃. MDN gives more details, but for this case the big takeaway is:

[!TIP] You cannot define or redefine CSS variables within the :visited selector.

C'mon now, CSS!

StackExchange / Stacks