Skip to content
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

Navigation block: Add link color support #34648

Closed
scruffian opened this issue Sep 8, 2021 · 4 comments · Fixed by #35725
Closed

Navigation block: Add link color support #34648

scruffian opened this issue Sep 8, 2021 · 4 comments · Fixed by #35725
Assignees
Labels
[Block] Navigation Link Affects the Navigation Link Block [Block] Navigation Affects the Navigation Block [Block] Site Title Affects the Site Title Block [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement.

Comments

@scruffian
Copy link
Contributor

What problem does this address?

Themes need a way to control the colors of links inside the navigation block. Ideally sub nav items have separate controls too. It would be really good to be able to control the colors of the reponsive menu too.

What is your proposed solution?

We should add support for link color to the block.

@scruffian scruffian added [Feature] Blocks Overall functionality of blocks [Block] Navigation Affects the Navigation Block labels Sep 8, 2021
@carolinan carolinan added [Block] Navigation Link Affects the Navigation Link Block [Block] Site Title Affects the Site Title Block labels Oct 5, 2021
@carolinan
Copy link
Contributor

Related to this is that the site title block in the navigation block has a link color option, but the option doesn't work.

@carolinan carolinan added [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement. labels Oct 5, 2021
@ockham
Copy link
Contributor

ockham commented Oct 13, 2021

Related to this is that the site title block in the navigation block has a link color option, but the option doesn't work.

I had a brief look into this. The reason is the following selector that includes the Navigation block class name twice, thus increasing specificity over the (elements-based) link color selector:

image

@getdave thankfully pointed me to the relevant code and comment:

// Menu item link.
// By targetting the markup directly we enable greater global styles compatibility.
// The extra container specificity is due to global styles outputting link styles that need overriding.
&.wp-block-navigation a {

@jasmussen added that the specificity balance between themes and global styles is delicate. So we probably shouldn't simply remove the duplicate class name, since it'll likely break other things 😬

Not sure how to proceed, but wanted to document these findings for posterity.

@jasmussen
Copy link
Contributor

The inheritance, as I recall, is there to inherit colors set on the navigation block itself. I'll poke around a bit when I have a moment, see what we can do.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 18, 2021
@jasmussen
Copy link
Contributor

I think #35725 fixes this one, and per @ockham's suggestion, it simply removes the extra specificity, which is always good to remove. The only gotcha is that I recall that specificity as being necessary, but I don't recall why, other than it being related to how colors were output by global styles at the time. But hopefully there are no ill side effects and this one gets addressed in the process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Block] Navigation Affects the Navigation Block [Block] Site Title Affects the Site Title Block [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants