-
Notifications
You must be signed in to change notification settings - Fork 82
[WIP] Reduce base link specificity #1054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
45aa3a8
78812ed
1dd40e8
459e73d
eacd7da
003c1a6
aacae3b
763299c
9de9e96
631f613
97cc65c
17b6cf4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,61 +4,51 @@ | |
|
|
||
| @use '../../includes/lib' as *; | ||
|
|
||
| :link { | ||
| color: $link-color; | ||
| // Rely on source order for the stateful style overrides | ||
| // 1. Visited | ||
| // 2. Hover | ||
| // 3. Active | ||
| // 4. Focus | ||
|
|
||
|
|
||
| a { | ||
| color: var(--link-color); | ||
| text-decoration: underline; | ||
|
|
||
| &:hover, | ||
| &:focus, | ||
| &:active { | ||
| color: $link-color-hover; | ||
| &:visited { | ||
| color: var(--link-color-visited); | ||
| } | ||
|
|
||
| &:hover, &:active, &:focus { | ||
| /* stylelint-disable-next-line color-function-notation */ | ||
| color: var(--link-color-with-state, hsl(from var(--link-color) h s calc(l - 10))); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would allow us to drop support for |
||
| text-decoration: none; | ||
| } | ||
|
|
||
| &:active { | ||
| background-color: rgba(0, 0, 0, 0.05); | ||
| } | ||
|
|
||
| @supports (--css: variables) { | ||
| color: var(--link-color); | ||
|
|
||
| &:hover, | ||
| &:focus, | ||
| &:active { | ||
| color: var(--link-color-hover); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| :visited { | ||
| color: $link-color-visited; | ||
|
|
||
| &:hover, | ||
| &:focus, | ||
| &:active { | ||
| color: $link-color-visited-hover; | ||
| } | ||
|
|
||
| @supports (--css: variables) { | ||
| color: var(--link-color-visited); | ||
|
|
||
| &:hover, | ||
| &:focus, | ||
| &:active { | ||
| color: var(--link-color-visited-hover); | ||
| } | ||
| } | ||
| // any descendant link will inherit dark theming (replace light-links mixin) | ||
| // this might belong in root file? site-wide dark theme class | ||
| .mzp-t-dark { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| --link-color: var(--link-color-inverse); | ||
| --link-color-visited: var(--link-color-visited-inverse); | ||
| /* stylelint-disable-next-line color-function-notation */ | ||
| --link-color-with-state: hsl(from var(--link-color-inverse) h s calc(l + 10)); | ||
| } | ||
|
|
||
| .mzp-t-dark { | ||
| @include light-links; | ||
| // any descendant link will inherit white theming (replace white-links mixin) | ||
| // link-specific theme class | ||
| .mzp-t-white-links { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I love the move the variables here! We need to be careful we also move to variables for the background colours at the same time so we don't get in a circumstance where the background is defined as dark using a hex colour but the component depends on variables for the links. Perhaps the prudent thing to do is to bundle the CSS variable changes into one big point release version that we test on un-supporting browsers. |
||
| --link-color: #{$color-white}; | ||
| --link-color-visited: #{$color-white}; | ||
| --link-color-with-state: #{$color-white}; | ||
| } | ||
|
|
||
| // apply directly to link | ||
| .mzp-c-cta-link { | ||
| font-family: $button-font-family; | ||
| font-family: var(--button-font-family); | ||
| font-weight: bold; | ||
|
|
||
| @supports (--css: variables) { | ||
| font-family: var(--button-font-family); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -76,8 +76,7 @@ | |||
| // * -------------------------------------------------------------------------- */ | ||||
| // Button shape and size | ||||
|
|
||||
| .mzp-c-button, /* stylelint-disable-line no-duplicate-selectors */ | ||||
| a.mzp-c-button { | ||||
| .mzp-c-button { | ||||
| @include border-box; | ||||
| @include font-size(16px); | ||||
| @include transition(background-color 100ms, box-shadow 100ms, color 100ms); | ||||
|
|
@@ -139,8 +138,7 @@ a.mzp-c-button { | |||
| // Primary | ||||
|
|
||||
| /* stylelint-disable-next-line no-duplicate-selectors */ | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||
| .mzp-c-button, | ||||
| a.mzp-c-button { | ||||
| .mzp-c-button { | ||||
| background-color: $color-black; | ||||
| border: 2px solid $color-black; | ||||
| color: $color-white; | ||||
|
|
@@ -160,32 +158,27 @@ a.mzp-c-button { | |||
| // * -------------------------------------------------------------------------- */ | ||||
| // Secondary | ||||
|
|
||||
| /* stylelint-disable-next-line no-duplicate-selectors */ | ||||
| .mzp-c-button, | ||||
| a.mzp-c-button { | ||||
| &.mzp-t-secondary { | ||||
| background-color: transparent; | ||||
| border-color: $color-black; | ||||
| color: $color-black; | ||||
| .mzp-c-button.mzp-t-secondary { | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is so much nicer, I want this to work 😭 |
||||
| background-color: transparent; | ||||
| border-color: $color-black; | ||||
| color: $color-black; | ||||
|
|
||||
| @include default-states; | ||||
| @include default-states; | ||||
|
|
||||
| // dark | ||||
| &.mzp-t-dark { | ||||
| background-color: transparent; | ||||
| border-color: $color-white; | ||||
| color: $color-white; | ||||
| // dark | ||||
| &.mzp-t-dark { | ||||
| background-color: transparent; | ||||
| border-color: $color-white; | ||||
| color: $color-white; | ||||
|
|
||||
| @include default-states-dark; | ||||
| } | ||||
| @include default-states-dark; | ||||
| } | ||||
| } | ||||
|
|
||||
| // * -------------------------------------------------------------------------- */ | ||||
| // Product Buttons | ||||
|
|
||||
| .mzp-c-button.mzp-t-product, | ||||
| a.mzp-c-button.mzp-t-product { | ||||
| .mzp-c-button.mzp-t-product { | ||||
| background-color: $color-blue-50; | ||||
| border-color: transparent; | ||||
| color: $color-white; | ||||
|
|
@@ -216,51 +209,47 @@ a.mzp-c-button.mzp-t-product { | |||
| // * -------------------------------------------------------------------------- */ | ||||
| // Neutral Buttons | ||||
|
|
||||
| /* stylelint-disable-next-line no-duplicate-selectors */ | ||||
| .mzp-c-button, | ||||
| a.mzp-c-button { | ||||
| &.mzp-t-neutral { | ||||
| background-color: transparent; | ||||
| border-color: $color-marketing-gray-30; | ||||
| .mzp-c-button.mzp-t-neutral { | ||||
| background-color: transparent; | ||||
| border-color: $color-marketing-gray-30; | ||||
| color: $color-marketing-gray-70; | ||||
|
|
||||
| &:focus { | ||||
| border-color: forms.$button-border-color-focus; | ||||
| } | ||||
|
|
||||
| &:hover { | ||||
| background-color: $color-marketing-gray-20; | ||||
| border-color: $color-marketing-gray-40; | ||||
| color: $color-marketing-gray-70; | ||||
| } | ||||
|
|
||||
| &:active { | ||||
| background-color: $color-marketing-gray-20; | ||||
| border-color: $color-marketing-gray-50; | ||||
| color: $color-marketing-gray-70; | ||||
| } | ||||
|
|
||||
| // dark | ||||
| &.mzp-t-dark { | ||||
| background-color: rgba($color-white, 0.1); | ||||
| border-color: rgba($color-white, 0.6); | ||||
| color: $color-white; | ||||
|
|
||||
| &:focus { | ||||
| border-color: forms.$button-border-color-focus; | ||||
| } | ||||
|
|
||||
| &:hover { | ||||
| background-color: $color-marketing-gray-20; | ||||
| border-color: $color-marketing-gray-40; | ||||
| color: $color-marketing-gray-70; | ||||
| background-color: rgba($color-white, 0.2); | ||||
| border-color: rgba($color-white, 0.6); | ||||
| color: $color-white; | ||||
| } | ||||
|
|
||||
| &:active { | ||||
| background-color: $color-marketing-gray-20; | ||||
| border-color: $color-marketing-gray-50; | ||||
| color: $color-marketing-gray-70; | ||||
| } | ||||
|
|
||||
| // dark | ||||
| &.mzp-t-dark { | ||||
| background-color: rgba($color-white, 0.1); | ||||
| border-color: rgba($color-white, 0.6); | ||||
| background-color: rgba($color-white, 0.2); | ||||
| border-color: rgba($color-white, 0.4); | ||||
| color: $color-white; | ||||
|
|
||||
| &:focus { | ||||
| border-color: forms.$button-border-color-focus; | ||||
| } | ||||
|
|
||||
| &:hover { | ||||
| background-color: rgba($color-white, 0.2); | ||||
| border-color: rgba($color-white, 0.6); | ||||
| color: $color-white; | ||||
| } | ||||
|
|
||||
| &:active { | ||||
| background-color: rgba($color-white, 0.2); | ||||
| border-color: rgba($color-white, 0.4); | ||||
| color: $color-white; | ||||
| } | ||||
| } | ||||
| } | ||||
| } | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,7 @@ | |
| --link-color-hover: #{tokens.$color-link-hover}; | ||
| --link-color-inverse: #{tokens.$color-blue-10}; | ||
| --link-color-visited-hover-inverse: #{tokens.$color-violet-05}; | ||
| --link-color-visited-hover: #{tokens.$color-link-hover}; | ||
| --link-color-visited-hover: #{tokens.$color-link-visited-hover}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another place that would be simplified by having the same hover state for both visited and unvisited links. |
||
| --link-color-visited-inverse: #{tokens.$color-violet-10}; | ||
| --link-color-visited: #{tokens.$color-link-visited}; | ||
| --link-color: #{tokens.$color-link}; | ||
|
|
||






Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was interesting. I thought I could redeclare the
--link-colorvar within the:visitedselector scope, but it didn't seem to take effect as computed style (tested with and without the:where()selector). Dev tools seems to show both the correct variable and the wrong computed style 🤷♀️Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pseudo-class variable correctly identified

computed value still default color
root inheritance correctly overruled

^unrelated, looks like we have a mixed up value declaration on
:root, visited hover color should be #952bb9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm worried about using
:wheresince the support for it is less wide spread than support for dynamic properties. Can you help me understand what would happen if a visitor had variable support but not where support? Links are visible they just don't get hover/focus? Or something more dire?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into CSS layers as a possible solution (I am so excited for these!) but the support for them is about the same as support for :where()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When custom properties are supported but

:where()is not, browsers will ignore any:where()selectors, so you get default user agent styles for hover/focus etc.vs. supported and applying the

:where()rulesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So on a background set to black with a CSS variable they will get blue/purple links?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will check tomorrow. I think they will get the
--link-coloror--link-color-visitedvalue. If we usemzp-t-darkfor dark background, the links will still have good contrast (those variables will respond to the dark theme), but their color will not change on hover/focus.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so older browser links would always get the
--link-colorregardless of state or:visitedstatus (all those rules are ignored). The correct link color will apply according to light/dark theme.