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

Fix: extra wp-block-navigation getting on ul tag #62804

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

up1512001
Copy link
Contributor

What?

  • created function similar to get_block_wrapper_attributes which support extra parameter to exclude some class.

Why?

Fixes #62690

How?

  • as get_block_wrapper_attributes enters default block class, which causes issues by having the block class on the inner container if it's used.

Testing Instructions

  • create navigation menu
  • check ul tag
  • now there will not be wp-block-navigation class which should only be present on main navigation wrapper

Screenshots or screencast

Screenshot 2024-06-25 at 00 13 53
Copy link

github-actions bot commented Jun 24, 2024

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.

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

Co-authored-by: up1512001 <up1512001@git.wordpress.org>
Co-authored-by: talldan <talldanwp@git.wordpress.org>
Co-authored-by: scruffian <scruffian@git.wordpress.org>

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

@up1512001
Copy link
Contributor Author

@talldan can you please review this PR?

@talldan
Copy link
Contributor

talldan commented Jul 1, 2024

@up1512001 I'm not really sure I know the PHP code for the navigation block well enough or how the block is supposed to function in terms of that wrapper. I've added a few other reviewers who may be able to help.

My instinct is that such a close copy of get_block_wrapper_attributes wouldn't be needed, but perhaps a simplified one that handles merging the classes and styles and outputting them in a similar format.

@akasunil akasunil added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block labels Jul 5, 2024
@scruffian
Copy link
Contributor

I wonder if a simple regex is better? #63801

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
4 participants