Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

navigation block: fix empty site-log li element in the dom #44049

Merged
merged 2 commits into from
Sep 12, 2022

Conversation

amustaque97
Copy link
Member

What?

Add a condition to verify that the site-logo is not null.

Why?

Don't create a li element in the dom when the site-logo is empty. If there is an empty li tag, it will add some extra space on the frontend.

How?

An empty condition is added if core/site-logo is non-empty then add a li element in the dom or else it will continue the rest of the statements.

Testing Instructions

  • Add a navigation block with two items.
  • Place a site logo block between the two items. The site logo block should not have an image assigned.
  • Save and view the front.
  • Confirm that there is no larger space between the two menu items.
  • View the page source to confirm that there is no empty <li></li>

Fixes: #43470

An empty check is added if `core/site-logo` is non-empty then add a
`li` element in the dom or else it will continue rest of the statements.
Fixes: #43470
@amustaque97 amustaque97 force-pushed the fix-43470-site-logo-empty-navigation-block branch from 23383a2 to 28b539a Compare September 10, 2022 11:18
@amustaque97 amustaque97 self-assigned this Sep 10, 2022
@amustaque97 amustaque97 added Needs Testing Needs further testing to be confirmed. [Block] Navigation Affects the Navigation Block labels Sep 10, 2022
Copy link
Contributor

@carolinan carolinan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working well in my test, thank you.

Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @amustaque97! This LGTM.

I had a minor suggestion, but it's not crucial. Feel free to apply or ignore 😊

Co-authored-by: Bernie Reiter <ockham@raz.or.at>
@amustaque97 amustaque97 merged commit 559b191 into trunk Sep 12, 2022
@amustaque97 amustaque97 deleted the fix-43470-site-logo-empty-navigation-block branch September 12, 2022 15:54
@github-actions github-actions bot added this to the Gutenberg 14.1 milestone Sep 12, 2022
@cbravobernal cbravobernal added [Type] Bug An existing feature does not function as intended and removed Needs Testing Needs further testing to be confirmed. labels Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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