Navigation Block: Let flex-grow/flex-basis be applied to the List Items

[yogeshbhutkar]

What, Why and How?

Fixes: #68753

This PR addresses a bug where the flex-grow property does not work as expected for blocks, such as social links, nested within the core/navigation block. The issue arises because these blocks are wrapped in a list item (<li>) element, which prevents the flex-grow style from being applied effectively.

Testing Instructions

1. Add a social links block inside a core/navigation block.
2. Enable the width grow feature for the social links block.
3. Verify that the block now correctly stretches to fill available space using flex-grow.
4. Repeat the above steps with custom blocks that also rely on list item wrappers.

Screencast

Screen.Recording.2025-01-20.at.3.04.53.PM.mov

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @Pixelous.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: Pixelous.

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org>
Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org>
Co-authored-by: Rishit30G <rishit30g@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[yogeshbhutkar]

Requesting for a PR review.

CC: @t-hamano @Mamaduka

[t-hamano]

Sorry for the late reply.

From what I understand, this issue occurs not only for Grow width (flex-grow), but also for Fixed width (flex-basis). Therefore, it is not enough to just check if the value is "fill".

A completely different approach may be to apply display:contents to the list item wrapper, treating it as if it doesn't exist. This should make flex-grow:1 applied to the child Social Links block work.

Example code changes to the trunk branch

diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php
index bde3ee4038..11d04c18bc 100644
--- a/packages/block-library/src/navigation/index.php
+++ b/packages/block-library/src/navigation/index.php
@@ -151,7 +151,7 @@ class WP_Navigation_Block_Renderer {
                $inner_block_content = $inner_block->render();
                if ( ! empty( $inner_block_content ) ) {
                        if ( static::does_block_need_a_list_item_wrapper( $inner_block ) ) {
-                               return '<li class="wp-block-navigation-item">' . $inner_block_content . '</li>';
+                               return '<li class="wp-block-navigation-item wp-block-navigation-item__wrapper">' . $inner_block_content . '</li>';
                        }
                }
 
diff --git a/packages/block-library/src/navigation/style.scss b/packages/block-library/src/navigation/style.scss
index 17ca8de9a7..822af4b711 100644
--- a/packages/block-library/src/navigation/style.scss
+++ b/packages/block-library/src/navigation/style.scss
@@ -39,6 +39,10 @@ $navigation-icon-size: 24px;
                .wp-block-navigation__submenu-container:empty {
                        display: none;
                }
+
+               &.wp-block-navigation-item__wrapper {
+                       display: contents;
+               }
        }
 
        // Menu item link.

@fabiankaegy, @getdave, and @scruffian may have some ideas.

PR Summary: In the Navigation block, blocks defined in $needs_list_item_wrapper are wrapped in a list element. As a result, even if you set Grow, Fixed widths on those blocks, they will not be applied correctly because they are not children of the flex layout. This PR is an attempt to solve the issue by adding inline styles for widths to the list item wrappers.

[yogeshbhutkar]

Thanks for the review, @t-hamano .

A completely different approach may be to apply display:contents to the list item wrapper, treating it as if it doesn't exist.

This approach seems to be the best solution to the issue. I've refreshed the PR to reflect the same. It works well in both scenarios: when the width is set to grow and when it's fixed.

Ref:

pr-demo.mov

Note: The failing tests are not related to the changes introduced in this PR and should pass on re-run.

[t-hamano]

This approach seems to work best for me and won't cause any issues.

However, I'm not 100% confident in it, so I'd appreciate a second feedback from someone more knowledgeable.

[getdave]

@MaggieCabrera and @mikachan have a lot of experience with Themes and may be able to verify this approach.

[MaggieCabrera]

I need to look at this closer but I'm concerned for the extra added class, it's something we need to commit to so we gotta be very sure about it

[MaggieCabrera]

This is not working consistently for me when I add extra elements in the navigation

Site editor	Frontend

I'm using a blank theme here for reference

[MaggieCabrera]

I need to look at this closer but I'm concerned for the extra added class, it's something we need to commit to so we gotta be very sure about it

Thinking about this more and looking closer at the implementation here, the extra class seems to make sense in this context. My main worry is that the CSS doesn't seem to be working consistently on the site editor and frontend like my previous comment shows.

[yogeshbhutkar]

Thanks for testing. I'll have a look and try to figure out the root cause.

[t-hamano]

@MaggieCabrera

This is not working consistently for me when I add extra elements in the navigation

• How did you configure the child blocks of the navigation block?
• Does this issue only occur with this PR?

[MaggieCabrera]

@MaggieCabrera

This is not working consistently for me when I add extra elements in the navigation

* How did you configure the child blocks of the navigation block?

* Does this issue only occur with this PR?

The markup for the nav block:

<!-- wp:navigation {"align":"wide","style":{"spacing":{"margin":{"top":"0"}}},"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right","orientation":"horizontal"}} /-->

This is the markup for the social links inside the nav block

<!-- wp:navigation {"ref":10077,"metadata":{"ignoredHookedBlocks":["woocommerce/customer-account","woocommerce/mini-cart"]},"align":"wide","style":{"spacing":{"margin":{"top":"0"}}},"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right","orientation":"horizontal"}} /-->

And I added 2 buttons after the social icons:

I am seeing the same thing on trunk with this setup too

[yogeshbhutkar]

I was able to reproduce this issue. I can also confirm that this happens on the latest Trunk as well.

I believe it happens because of the way the markup is structured. I found that the button is not a child of navigation container (a ul element), which makes sense, but that would mean we would need to provide explicit flex-grow: 1 to wp-block-navigation__container to let it occupy available width. This issue however is not limited to social blocks and is reproducible with any other block used in conjunction with buttons within navigation.

[t-hamano]

I realized that the approach using the display: content might cause accessibility issues. See:

• Fix Navigation accessibility issues #36292
• Try using flex layout on the Navigation block #36169 (comment)

It seems that there is a possibility to disable semantics in Safari browser. Unfortunately, I can't test it because I'm a Windows user. Can someone test if this browser bug still occurs?

cc @tellthemachines

[tellthemachines]

I realized that the approach using the display: content might cause accessibility issues.

Yes, it's not recommended to use display: contents on elements that have semantic meaning such as list items. Caniuse confirms there are still accessibility bugs in its implementation across major browsers.

I'm not sure what the best solution is. Ideally the child layout styles would be applied to the li itself, but because it's part of the container block that doesn't happen (unlike with navigation links, which come pre-wrapped in an li). Having social icons render conditional markup depending on its parent isn't great, but we do it in page list to render additional classnames.

We have the same bug with site title and site logo btw, for the same reasons.