Empty Site logo block in navigation block results in empty li item on front

[carolinan]

Description

When the site logo block is added to the navigation block, but no image is selected, there is a n empty <li> item in the navigation.

Step-by-step reproduction instructions

1. Add a navigation block with two items.
2. Place a site logo block between the two items. The site logo block should not have an image assigned.
3. Save and view the front.
4. Confirm that there is a larger space between the two menu items.
5. View the page source to confirm that there is an empty <li></li> where the logo would have been placed if an image had been selected.

Screenshots, screen recording, code snippet

No response

Environment info

WordPress 6.0.1
Gutenberg trunk

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

[amustaque97]

👋🏻 , I had a chance to look into the issue.
As per my observation, it is a bit tricky problem to solve. I have no idea how to fix this. However, I will share my findings.

• In the navigation block we're adding core/site-logo or any other block using useInnerBlocksProps to build nested blocks in the file
• When we add an empty core/site-logo it enable nested block content with the site-logo value as null
• Before saving the nested content either we filter out and remove the site-logo content if there is no image
  OR
• While rendering the navigation block we should add a check if the inner-block is core/site-logo and inner-block->render() is empty then skip adding <li> element to the dom. These changes should be done here.

  gutenberg/packages/block-library/src/navigation/index.php

  Lines 561 to 563 in 2bc40bb

  	if ( 'core/site-title' === $inner_block->name || 'core/site-logo' === $inner_block->name ) {
  	$inner_blocks_html .= '<li class="wp-block-navigation-item">' . $inner_block->render() . '</li>';
  	} else {

Another thought was what if we can do some checks inside the core/site-logo block but I'm not sure how will it affect the userInnerBlockProps.content value.

Let me know what do you guys think?

[getdave]

@amustaque97 Thank you for taking the time to provide this feedback. It is much appreciated.

We'd need to ensure as much consistency between what the block displays in the editor and what it shows on the front of the site.

Therefore we could consider:

• does the "empty" site logo block need to have a more appropriate default?
• how can we convey "empty block" within the editor whilst also not rendering anything on the front of the site.

We had a similar problem with rendering the default fallbacks for the Nav block whereby we needed to render a list of pages by default if the block wasn't populated.

@amustaque97 I'd be happy to help review a PR if you have time/inclination. Much appreciated.

[ockham]

FWIW, I think this is an example of a situation we might see more frequently in FSE. We had a similar case in the Comments block where we needed have all child blocks in the editor so they could be re-arranged and customized; OTOH, we had to deal with the situation where a post didn't have any comments (and didn't want to render any spurious empty elements).

The solution we decided on (#43383) is similar to the second one suggested by @amustaque97, if I'm not mistaken.

[amustaque97]

Thank you @ockham, I will draft a PR with the second approach where in I will be adding an empty check and will share the PR link for feedback.