Skip to content

Navigation block: Allow themes to override block library text-decoration rule #63406

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.

Sign up for GitHub

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

Merged
talldan merged 3 commits into from
Jul 17, 2024
Merged

Navigation block: Allow themes to override block library text-decoration rule #63406

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be5ae55
Reduce specificity of navigation text decoration rule
talldan Jul 11, 2024
0f9669a
Apply Aaron suggestion to also lower specificity for focus/active
talldan Jul 12, 2024
56fcc6f
Remove `:not([class*="has-text-decoration"])` from selector
talldan Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Reduce specificity of navigation text decoration rule
  • Loading branch information
@talldan
talldan committed Jul 12, 2024
commit be5ae55caae1a4b88637dbbb4bc096d770a53d65
12 changes: 5 additions & 7 deletions packages/block-library/src/navigation/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,12 @@ $navigation-icon-size: 24px;
}
}

&:where(:not([class*="has-text-decoration"])) {
a {
text-decoration: none;
&:where(:not([class*="has-text-decoration"])) :where(a) {
Copy link
Member

@ellatrix ellatrix Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these be combined into a single where?

Copy link
Member

@ellatrix ellatrix Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question: do we still need the :not([class*="has-text-decoration"]) if the specificity is lowered? Ideally if it has that class, it should just override this rule and the exception should no longer be needed.

Copy link
Contributor Author

@talldan talldan Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these be combined into a single where?

I had already tried this:

&:where(:not([class*="has-text-decoration"]) a)

But that selector doesn't work. Is there another way?

Copy link
Contributor Author

@talldan talldan Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question: do we still need the :not([class*="has-text-decoration"]) if the specificity is lowered? Ideally if it has that class, it should just override this rule and the exception should no longer be needed.

It does seem needed, it overrides a general rule for links that are not buttons which gives them underlines:
Screenshot 2024-07-12 at 3 24 33 PM

That should be a bit more obvious when testing now that I've rebased to include the fix from #63403.

Copy link
Contributor Author

@talldan talldan Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've applied Aaron's suggestion now so it looks a little different - #63406 (comment)

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw Jul 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does seem needed, it overrides a general rule for links that are not buttons which gives them underlines:

Removing the :where(:not([class*=has-text-decoration])) won't change the specificity. So the selector would remain 0-1-0 specificity and continue to override a:where(:not(.wp-element-button)) which is only 0-0-1.

Copy link
Contributor Author

@talldan talldan Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I misunderstood what was meant in the original comment. I've gone ahead and removed that. 👍

text-decoration: none;

&:focus,
&:active {
text-decoration: none;
}
&:focus,
&:active {
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These pseudo styles get appended so the resulting specificity is 0-2-0 which is still higher than global styles.

To my knowledge there isn't an easy way in SCSS to insert the inner :focus/:active into a CSS function in the parent selector i.e. :where(). So I think they need to be written out in full.

	&:where(a),
	&:where(a:focus),
	&:where(a:active) {
		text-decoration: none;
	}

Copy link
Contributor Author

@talldan talldan Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, updated in 0f9669a

text-decoration: none;
}
}

Expand Down