Twenty Twenty-One: Refactor or remove navigation item padding CSS

[Joen]

Twenty Twenty One has a CSS custom property that targets navigation menu items:

.wp-block-navigation .wp-block-navigation-link .wp-block-navigation-link__content {
    padding: var(--primary-nav--padding);
}

There are a few problems with that. First off, it prevents the block from inheriting padding values set by the Global Styles system. See ​https://github.com/WordPress/gutenberg/issues/31784

Secondly, it overrides new default behavior for the navigation block, which is to have zero padding for menu items unless a background color is set. This behavior is created to enable navigation menus that align with headings and site titles that don't have any padding. See ​https://github.com/WordPress/gutenberg/pull/30805.

Finally, the rule targets only custom menu items, not menu items generated by the dynamic Page List block.

Recommended solution: either remove the rule entirely and rely on default values, or leverage the global styles system to apply padding.

For reference, here is the CSS the will be applying padding to the navigation menu item in the future:

.wp-block-navigation .wp-block-pages-list__item a,
.wp-block-navigation .wp-block-navigation-link a {
    padding: 0;
}

[desrosj]

Looks like this line has been included since the theme was first moved to SVN. Updating the version.

This PR aims to fix obsolete navigation block styles in the TwentyTwentyOne theme.

Trac ticket: https://core.trac.wordpress.org/ticket/53220

[Joen]

I took a stab at a patch for this in ​https://github.com/WordPress/wordpress-develop/pull/1723. There are a few issues around the padding, much of it related to how heavily TT1 styles the navigation block, which has then drifted due to how much the block has evolved over the past year. In this gallery there are a few screenshots of how there's a top/bottom margin on every block, there's a separate padding value attached to the wrong element, and incorrect spacing between items due to the change to using gap: ​https://cloudup.com/c5HjvVORdzJ

I haven't been working on TT1 initially, so there's likely a lot of context for the original styles that I'm unaware of. So I would appreciate a good test.

[Joen]

Before the patch.

[Joen]

After the patch in ​https://github.com/WordPress/wordpress-develop/pull/1723

[poena]

For ​Gutenberg 11.5, the CSS class names in the navigation block were updated. The padding in the theme styles is not applied on WordPress version 5.9 or 6.0.

There are a few things I think that we need to consider:

-Twenty Twenty-one is possibly the last classic theme (Twenty Twenty-Three has not been announced yet), so we should try to solve these types of issues.

-WordPress 5.6 (reported version) - 5.8 does not support the navigation block without Gutenberg. And Gutenberg has a minimum required WordPress version which is 5.8. This minimum required version will keep increasing.

-Generally I would prefer to not remove CSS variables as they may be used by child themes.
But because of the above reasons, I think removing them would only have a low negative impact, and a positive impact for users who are on older versions of WordPress and Gutenberg.

We may need to discuss if we should limit the loading of some CSS to older versions of WordPress.

[poena]

@Joen I have tested the navigation block with 6.2 alpha and the appearance tools theme support.
Enabling this theme support as suggested in #56487 enables block gap for the navigation block.
I did not find any apparent spacing issues, I believe it is working well.

With that in mind, how would you prefer to proceed with this CSS changes?

[Joen]

With that in mind, how would you prefer to proceed with this CSS changes?

Seems like we can apply the patch and go from there? Forgive me if I'm misunderstanding the question.

[poena]

Then let's move this into the 6.2 milestone to remove the redundant CSS. From what I can tell from my testing, the PR does not need to be refreshed.

[Joen]

Thank you!

[audrasjb]

Ok the changeset looks good on my side too 👍
Self assigning for commit consideration.

[audrasjb]

In 55088:

Twenty Twenty-One: Fix obsolete navigation block styles for better Global Styles support.

This changeset removes CSS custom properties that targets navigation menu items to avoid preventing the block from inheriting padding values set by the Global Styles system.

Props Joen, desrosj, poena.
Fixes #53220.

[audrasjb]

Reopening as it seems like [55088] modified some version controlled files.
Revert incoming.

[audrasjb]

In 55089:

Twenty Twenty-One: Revert [55088].

[55088] introduced some issues with the "Test Default Themes & Create ZIPs" workflow on GitHub Actions. This changeset reverts it to leave time to figure out what failed and why.

See #53220.

Trac ticket: https://core.trac.wordpress.org/ticket/53220

[SergeyBiryukov]

53220.diff​ or ​PR 3862 should be a clean refresh of the previous PR without unrelated formatting changes.

Note: When rebuilding the theme's CSS files on Windows, I still got unrelated formatting changes similar to the ones in the previous PR, but rebuilding on Ubuntu via WSL did the trick. This could use some further investigation as to why building on Windows gives a different result, but the tests appear to pass with the new PR.

[audrasjb]

I tested the PR and it works fine on my side, thank you for refreshing the PR Sergey!

[audrasjb]

In 55101:

Twenty Twenty-One: Fix obsolete navigation block styles for better Global Styles support.

This changeset removes some CSS custom properties that targets navigation menu items to avoid preventing the block from inheriting padding values set by the Global Styles system.

Props Joen, desrosj, poena, audrasjb, hellofromtonya, SergeyBiryukov.
Fixes #53220.

Committed in https://core.trac.wordpress.org/changeset/55101, thanks again Sergey!