Make WordPress Core

Opened 9 months ago

Closed 6 months ago

Last modified 6 months ago

#63839 closed defect (bug) (fixed)

Layout type classname incorrect for custom blocks

Reported by: peroks's profile peroks Owned by: isabel_brison's profile isabel_brison
Milestone: 6.9 Priority: normal
Severity: normal Version: 6.8.2
Component: General Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

This bug has already been reported in the Gutenberg repository here: https://github.com/WordPress/gutenberg/issues/53295 and even fixed in this PR: https://github.com/WordPress/gutenberg/pull/53404

Unfortunately, the PHP fix was not ported back to WordPress core, so as of WP 6.8.2, the bug is still not fixed.

The bug is found here: https://github.com/WordPress/wordpress-develop/blob/6.8.2/src/wp-includes/block-supports/layout.php#L861

Current/buggy PHP code

<?php
$block_name    = explode( '/', $block['blockName'] );
$class_names[] = 'wp-block-' . end( $block_name ) . '-' . $layout_classname;

Fixed code in the Gutenberg repository

<?php
$split_block_name = explode( '/', $block['blockName'] );
$full_block_name  = 'core' === $split_block_name[0] ? end( $split_block_name ) : implode( '-', $split_block_name );
$class_names[]    = 'wp-block-' . $full_block_name . '-' . $layout_classname;

Change History (17)

#1 @rishabhwp
9 months ago

Thank you for bringing this up. I’ll be happy to work on it and raise a PR.

This ticket was mentioned in PR #9518 on WordPress/wordpress-develop by @rishabhwp.


9 months ago
#2

  • Keywords has-patch added

#3 @isabel_brison
9 months ago

  • Owner set to isabel_brison
  • Resolution set to fixed
  • Status changed from new to closed

In 60651:

Editor: Include namespace in layout classname for non-core blocks.

Adds block namespace in layout classname so global styles can be built correctly.

Props isabel_brison, wildworks, peroks, rishabhwp.
Fixes #63844, #63839.

@isabel_brison commented on PR #9518:


9 months ago
#4

Closing this out as the change has been committed in https://core.trac.wordpress.org/changeset/60651.

@R1shabh-Gupta I see you added me as a reviewer but I didn't get notified because the PR is in draft mode. For next time, feel free to "create pull request" instead of "create draft" so reviewers can be notified! (Unless you're still working on it that is 😄)

Thanks for contributing!

#5 @rishabhwp
9 months ago

Hey @isabel_brison,
The PR was still a work in progress. I was in the middle of adding tests for it.
You can check https://github.com/WordPress/wordpress-develop/blob/trunk/tests/phpunit/tests/block-supports/layout.php, which contains all the tests for block supports. I think adding tests here would be really valuable in this case. The code is nearly ready. Would you prefer that I update the existing PR, or should I open a new one to include the tests?

#6 @isabel_brison
9 months ago

@rishabhwp might be easier to open a new one for the tests!

This ticket was mentioned in PR #9541 on WordPress/wordpress-develop by @rishabhwp.


9 months ago
#7

  • Keywords has-unit-tests added

#8 @rishabhwp
9 months ago

Hi @isabel_brison,
I've added unit tests to ensure the fix works as expected. Could you please review the changes when you have a moment? Thank you!

#9 @rishabhwp
9 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#10 @ocean90
7 months ago

  • Milestone changed from Awaiting Review to 6.9

#11 @wildworks
6 months ago

Hi @isabel_brison, do you have the bandwidth to review PR 9541?

If there is no activity on this ticket by next week's 6.9 RC1 release, I will punt it to 7.0.

@isabel_brison commented on PR #9541:


6 months ago
#12

Not sure why the tests are failing on CI; they pass for me locally.

#13 @isabel_brison
6 months ago

Thanks for the ping @wildworks! I've reviewed the PR. Happy to commit it unless you want to do so!

@wildworks commented on PR #9541:


6 months ago
#14

@tellthemachines, I merged the latest trunk branch into this branch. Hopefully, this will resolve the test failure.

@wildworks commented on PR #9541:


6 months ago
#15

@tellthemachines, it seems the CI failure has been fixed. If you have the bandwidth, please feel free to commit!

#16 @isabel_brison
6 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 61175:

Editor: update layout tests.

Adds a test for name spacing custom blocks in layout classnames.

Props rishabhwp, wildworks, isabel_brison.
Fixes #63839.

@isabel_brison commented on PR #9541:


6 months ago
#17

Done, thanks for the updates @t-hamano !

Note: See TracTickets for help on using tickets.