Navigation block: text color not appearing in the editor

[annezazu]

Description

When assigning a text color to the main navigation block using TT2, the color isn't shown properly in the editor but does appear correctly when viewed on the front end of the site. This occurs with 6.0 RC3. cc @ndiego as a heads up, @getdave to perhaps help address, and @scruffian for possibility of this being a TT2 issue.

Step-by-step reproduction instructions

1. Add a navigation block.
2. Select a text color.
3. Notice no changes in editor.
4. Publish changes.
5. View frontend of the site and see changes there.

Screenshots, screen recording, code snippet

text.color.nav.blcok.mov

Environment info

• WordPress 6.0 RC 3
• TT2
• No GB installed

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[annezazu]

Adding to 6.0.1 conservatively. Would love to see this addressed for RC4 planned for this Friday but I recognize that that's a tight timeline :)

[carolinan]

This problem seems to be escalating

[getdave]

Screen.Capture.on.2022-05-20.at.11-56-44.mov

Tested and confirmed. Seems to be another inline style overriding this text color style 🤔

Doesn't happen on Archeo Theme though.

I wonder if there's a specific Theme JSON setting that's causing the cascade to be overriden. Certainly the style that's overriding the text color seems to come from Global Styles.

Doesn't happen in the Post Editor either. Only the Site Editor which makes me think Global Styles:

Screen.Capture.on.2022-05-20.at.12-04-03.mov

[getdave]

Some more debugging reveals the culprit to be the text color set on a wrapper Group block whose styles are cascading on to every a in it's container.

Screen.Capture.on.2022-05-20.at.12-07-43.mov

You can see this at

https://github.com/WordPress/twentytwentytwo/blob/27054dbcb95f5d72cd943a0f3318c3ad97599f85/parts/header-small-dark.html#L1

<!-- wp:group {"align":"full","style":{"elements":{"link":{"color":{"text":"var:preset|color|background"}}},

The cascade is how CSS works so this would seem to be legit. However, I'd expect the Nav block CSS to be more specific.

The temporary "fix" would be to have a specific style on the Navigation block a or to remove the styles on the wrapping Group.

We should look into why the Nav block CSS isn't more specific and/or overiding this. My instinct is that the a on the Nav are set to inherit their color from the parent Nav block. Therefore the Global Styles selector will override this.

[getdave]

@annezazu @carolinan A few contributors have discussed this and it looks like the problem is due to CSS specificity introduced by the fact that the Nav block has (for historical and legitimate reasons) a custom implementation of the color picking system.

We could extend this system to affect a patch but as a few folks have noted would be compounding existing technical debt which will have to be "undone" later.

Ideally we would take time to do a wider refactor which would:

• extend Global Styles to allow the ability to define custom color properties to be handed by Globsl Styles (i.e. not just text, link and background).
• refactor the Nav block to use the updated system
• fix any remaining specificity issues

I believe this issue "only" affects the Editor and not the front end. How impactful do you feel this bug is? If it's causing real pain and cannot be "managed" then we might need to go for the fix and try to undo the tech debt later. If not then we could take our time and get a solid fix in place which should also protect the block for further regressions as it would then be coupled to the wider system.

No decisions have been made at this point and so I'd really appreciate your feedback should you be able to offer it 🙇‍♂️

[annezazu]

@getdave thanks for summarizing the discussion and laying out the option. This feels like a very common UX bug to resolve that would impact the basics of using the navigation block (updating the colors of the links). My vote is to have an interim fix for 6.0.x and then figure out a longer term solution for 6.1 that is in line with what you described, which should work well since the Global Styles Engine work will continue to mature. cc @priethor for thoughts too :)

[getdave]

Thanks @annezazu 👍

Also @draganescu and @scruffian do you have any thoughts about short term vs longer term fix?

[scruffian]

My concern with a short term fix is that it will make the long term fix harder, as we'll have to undo the mess that it will create.

[priethor]

I'm also concerned about adding a lot of technical debt to fix this. I would evaluate a short-term fix for 6.0.1 but would only proceed if the effort required is not too big and there is confidence the patch doesn't compromise the long-term solution.

[getdave]

Update: myself and @scruffian took some time to pair on the problem here. This is what we found:

---

• On trunk the Nav block uses non-standards text color and background controls.
• There is now a standards based Block Supports for Text, Background and Link colors which it should be using.

The Bug:

• The styles for the Nav block tell the links inside it to inherit their color from the parent element (color: inherit ).
• In the case that a parent block specifies a link color (e.g. a containing Group block), Navigation links will use that block's Link color setting rather than the Text color set on the Navigation block itself.
• The Nav block is relying on implicit CSS inheritance to set the a (link) color based on the value of Text Color .
• Moreover, because it uses non-standard controls the Block Supports output has no knowledge of the CSS that is being output.
• The link color CSS rules from parent blocks are also output later in the DOM and thus override the custom rules in the Nav block (even though the specificity is identical).
• The Nav block should have a specific color setting for Link color in order that the CSS ordering will output in the correct order thereby allowing the Nav block specific styles for Links to win out.
• This happens only in the Editor.
• The block is relying on hardcoded, implicit CSS rules to set the color of anchor links.

Solution

• We need to make the Nav block have a Link specific color control to set the a color of the block explicitly (as opposed to relying on hardcoded inheritance).
• Enabling color block supports for Text, Background and Link allows us to do this.
• Setting the Link color successfully overwrites any link color setting that is being "inherited" from parent blocks - this works!
• The problem:
  • Nav block currently uses custom color controls not standard Block Supports.
  • A user's current expectation is that setting Text color on the block will modify the color of the Links in the Navigation block - indeed this does happen on the front end. (Q: why does that work on the front end?)
  • We need to migrate any existing Text color selections to the new Link color setting. We could do this in a deprecation on the block.
  • We should consider hiding the Text color control because it adds confusion. This could be progressively disclosed.

Summary

We believe we've come up with a solution which should solve. It will have some compromises in the short term but these would mostly be manifested in the block controls' presentation.

PR to follow.

[getdave]

I'm going to suggest we close this one out as I believe it was fixed in #44578 (cc @jasmussen).

However the Navigation block still does not use the standardised color controls from Block Supports. It's a custom implementation.

If it's desirable and necessary to do that then we should consider opening a new issue to track that requirement.

For a summary of the original bug here and the status of the problem you can watch my video.