Navigation block: item link color style property missing in 13.6.0

[adamwoodnz]

Description

A bug was found in wordpress.org and make.wordpress.org where the color of links in the navigation block changed. The root cause was found to be a CSS property being dropped in Gutenberg 13.6.0, due to a change to the navigation block in this PR

In 13.5.1 this CSS rule meant that the links would inherit the white color from the parent's style

.wp-block-navigation .wp-block-navigation-item__content {
    color: inherit;
}

In 13.6.0 this was supposed to still exist but be added via block.json. This missing property means that the links are the default blue set for anchors, leading to very poor color contrast with the dark background.

For now we have patched the styles on wordpress.org but we'd like to remove this once this bug is resolved.

It seems the changes in #41898 aren't working as expected so it would be good to find out the root cause, as maybe this is more widespread.

Step-by-step reproduction instructions

1. Go to WordPress.org and inspect one of the navigation links in the header
2. Observe that the style rule from Gutenberg 13.6.0 does not have color: inherit
   
3. Disable the patch property color: inherit we added to wporg-mu-plugins
   
4. Observe that all the links are blue instead of white

Screenshots, screen recording, code snippet

No response

Environment info

WordPress 6.1-alpha-53680
Gutenberg 13.6.0
All browsers on MacOS

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

[draganescu]

Testing this on trunk I see that the navigation links have

.wp-block-navigation a:not(.wp-element-button) {
  color: inherit;
}

and the colors (with empty theme) seem to be ok. Am I missing something or did the problem fix itself :D ?

[adamwoodnz]

Yeah correct that rule is present on trunk, but isn't on WordPress.org. Maybe something theme related there then. Thanks for testing, I should have done this first and will in future.

[adamwoodnz]

Testing this on trunk I see that the navigation links have

.wp-block-navigation a:not(.wp-element-button) {
  color: inherit;
}

and the colors (with empty theme) seem to be ok. Am I missing something or did the problem fix itself :D ?

Just tested at tag v13.6.0 and the rule is missing, but is there on trunk so it seems another PR has fixed this 🎉

I guess we'll wait for the next release and I'll close this if resolved.

[adamwoodnz]

@draganescu I tried running 13.7.0 RC in my wporg sandbox, but wordpress.org still doesn't get this inline style rule like the gutenberg dev environment does when running at that tag:

Do you know how these styles get generated and how we might hook into getting them in the wporg-main theme?

[andrewserong]

Separately to this issue, I've been looking at a bit of global styles loading lately, so thought I'd drop a few links here in case it helps with the debugging. I'll just ping @draganescu @scruffian and @getdave here, since they've been more involved in the __experimentalStyle feature, I believe! 🙂

I'm wondering if part of the issue here could be to do with how loading separate core block assets works.

Currently the logic to determine whether or not to output the styles in __experimentalStyle (as in #41898) is guarded behind a wp_is_block_theme check (here). This ultimately calls is_block_theme, which looks for the presence of /templates/index.html or /block-templates/index.html, before calling wp_add_global_styles_for_blocks() to go and generate the styles from the Theme JSON class (that were resolved to include the __experimentalStyle here).

So, for classic themes, or more specifically themes that don't provide the above two html templates, it sounds like in PRs like #41898, the style is effectively unavailable for those themes, and themers would then need to add those rules into their own themes?

When styles are moved over to __experimentalStyle, what's the expectation for themes that don't render/output global styles?

[adamwoodnz]

I tested a few themes today and I’ve found that the moved styles are available in the block themes Twenty Twenty-Two and Poe, but not available in the classics Twenty Twenty-One and Twenty Twenty, which seems to support this theory.

[getdave]

So, for classic themes, or more specifically themes that don't provide the above two html templates, it sounds like in PRs like #41898, the style is effectively unavailable for those themes, and themers would then need to add those rules into their own themes?

Currently this is the case. But I don't feel particularly comfortable that that's an acceptable approach.

When styles are moved over to __experimentalStyle, what's the expectation for themes that don't render/output global styles?

I think we're going to have to come up with a solution for backwards compatibility. Perhaps for Themes that don't output the global styles stylesheet we're going to need to build a blocks stylessheet regardless and enqueue that. If we don't then it's going to cause regressions for Themes that don't meet the criteria set by is_block_theme.

I'm AFK until Tuesday but I think @scruffian will be available Monday and may be able to look into a fix?

[getdave]

I noticed that...

.wp-block-navigation a:not(.wp-element-button) {
  color: inherit;
}

is not the same as the CSS that was originally part of the block CSS:

.wp-block-navigation .wp-block-navigation-item__content {
    color: inherit;
}

So whilst this might "work" the problem still remains that Themes that are not identified as "block themes" will be missing CSS styles which have been moved to JSON.

[scruffian]

So, for classic themes, or more specifically themes that don't provide the above two html templates, it sounds like in PRs like #41898, the style is effectively unavailable for those themes, and themers would then need to add those rules into their own themes?

While that was the case with #41898 it was addressed in #42005. In my tests the CSS is output in both block and classic themes.

[scruffian]

These are the different scenarios we tested:

Block theme with separate assets enabled
The CSS is loaded in the header in a separate inline stylesheet:

<style id='wp-block-navigation-inline-css'>
 .wp-block-navigation{font-size: 16px;text-transform: uppercase;}
 .wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;}
 </style>

Block theme with separate assets disabled
This case is the same as the previous one - it seems like block themes always load separate assets?

Classic theme with separate assets disabled (twentyten)
This CSS loads inline in the header:

<style id='global-styles-inline-css' type='text/css'>
...
.wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;}
</style>

Classic theme with separate assets enabled (twentyten) with nav block on the page
This inline CSS loads in the footer:

<style id='wp-block-navigation-inline-css' type='text/css'>
.wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;}
</style>

Classic theme with separate assets enabled (twentyten) without a nav block on the page
No CSS is loaded in this scenario.

[scruffian]

@adamwoodnz can you please confirm that this is still an issue on trunk, and if so which of the following cases you are in?

[adamwoodnz]

I've just tested Gutenberg 13.7.3 in my .org sandbox and still the same unfortunately. Specifically we don't have style tags with either global-styles-inline-css or wp-block-navigation-inline-css.

Sorry for my ignorance but could you elaborate on what 'separate assets enabled' means please? How would this setting be configured in a theme?

I'm wondering if something in blocks.php could be disabling the inline CSS and getting the styles some other way. @ryelle any insight here please?

[getdave]

@adamwoodnz That would be this function

https://github.com/WordPress/wordpress-develop/blob/299ac4dbb9351371961155ca30d772ba2cebb549/src/wp-includes/script-loader.php#L2465

You can enable or disable it using the filter

add_filter( 'should_load_separate_core_block_assets', '__return_true' ); // or `__return_false`

You can learn about what this function does here.

[scruffian]

How would this setting be configured in a theme?

I don't think it should be, it's a core performance feature.

[scruffian]

I tested running the wporg site with Gutenberg locally and I see this CSS:

<style id='global-styles-inline-css'>
....
 .wp-block-navigation a:not(.wp-element-button){color: inherit;}

[ryelle]

I'm wondering if something in blocks.php could be disabling the inline CSS and getting the styles some other way.

Yes, on the sites that use classic themes, the global styles are removed to prevent overriding the custom properties we've set. See unregister_classic_global_styles. The global styles are then generated from the News site, but only the variables and presets. For example, global-styles-inline-css is added on News, but the Make sites have our custom global-styles-for-classic-themes (though, that might change soon).

[scruffian]

I don't think this is a Gutenberg bug then...

[adamwoodnz]

👍 Closing this then. Thanks for your assistance @scruffian @getdave @andrewserong, sorry if I was on the wrong track.