Remove :hover pseudo-class from link-style-hover mixin #1792
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gabalafou
commented
Apr 29, 2024
Comment on lines
-36
to
-41
| // Exception here - keep visited in the default link colour | ||
| // it still should hover on the default link hover colour | ||
| &:visited p.prev-next-title { | ||
| color: var(--pst-color-link); | ||
| &:hover { | ||
| color: var(--pst-color-link-hover); |
There was a problem hiding this comment.
This block of style rule exceptions isn't needed because:
- line 41 is incorporated in lines 34 and 36 on the right - the
link-style-hovermixin (modified and shown above in this diff) sets color to--pst-color-link-hover - line 39 is incorporated in lines 27 and 28 on the right - the link-style-default mixin sets color to --pst-color-link
| &:hover { | ||
| @include link-style-hover; |
There was a problem hiding this comment.
Cody tidiness bonus: I like how this change allows everything to be lexically grouped
There was a problem hiding this comment.
Nice, it's definitely neater than my original approach!
trallard
reviewed
May 7, 2024
| &:hover { | ||
| @include link-style-hover; |
There was a problem hiding this comment.
Nice, it's definitely neater than my original approach!
trallard
approved these changes
May 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
Removing the pseudo-class from within the link-style-hover makes the mixin a bit more flexible. For example, I realized that I wanted to change the "next" and "previous" links in the footer so that when any part of the
aelement is hovered, the link text is underlined. It was complicated to do that with the pseudo-class in the mixin, but easy to do afterwards.Before:
After: