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

wp_render_layout_support_flag renders classes on wrong element #57242

Open
coreyworrell opened this issue Dec 20, 2023 · 1 comment
Open

wp_render_layout_support_flag renders classes on wrong element #57242

coreyworrell opened this issue Dec 20, 2023 · 1 comment
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Bug An existing feature does not function as intended

Comments

@coreyworrell
Copy link
Contributor

Description

Like the comment here says (in @todo as well), the wrong element is used when there is no inner block wrapper. This happens for example with a block that uses a <details> element and a <summary class="anything"> inside, and then renders the inner blocks directly after the </summary> like the HTML spec allows/recommends:

<details>
  <summary class="anything">Summary title</summary>
  <p>Inner blocks</p>
  <p>Other blocks</p>
</details>

The core/details block is not affected because it just happens to not apply any classes to the summary element. But for custom blocks that do (ie: supports.color.__experimentalSkipSerialization that apply to summary instead of the root details element), this becomes a problem.

A workaround is to obviously wrap the inner blocks in a div or similar, but this may cause other style issues and is not in line with HTML spec again. And a Make WordPress Core article about inner blocks recommends this approach of adding HTML inline with inner blocks.

Step-by-step reproduction instructions

  1. Create a custom block that supports layout, and renders an element with a non-empty class attribute and then renders inner blocks directly after: <details { ...innerBlocksProps }><summary className="anything">Summary</summary>{ innerBlocksProps.children }</details>
  2. Insert this block in the editor and save post
  3. View post. See how the is-layout-flow (or flex, etc) class is applied to the summary element instead of the details element.

Screenshots, screen recording, code snippet

No response

Environment info

WordPress 6.4.2

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

@coreyworrell coreyworrell added the [Type] Bug An existing feature does not function as intended label Dec 20, 2023
@ramonjd ramonjd added the [Feature] Layout Layout block support, its UI controls, and style output. label Dec 20, 2023
@jordesign jordesign added the Needs Testing Needs further testing to be confirmed. label Dec 20, 2023
@tellthemachines tellthemachines removed the Needs Testing Needs further testing to be confirmed. label Jan 5, 2024
@tellthemachines
Copy link
Contributor

Thanks for reporting this! I can reproduce the issue; it happens due to the classname on the inner element and is an unfortunate side-effect of wp_render_layout_support_flag using the last classname before inner blocks start to attach the layout classes to. The intention is to target the innermost wrapper of inner blocks, which works well for blocks such as Cover, but might not work in cases such as the one you mention. The logic needs improving - there's even a note in the code - though there's no specific timeline for doing so yet.

On the other hand, a known side-effect of adding the layout classes to a container that has both inner blocks and other, non-inner block, markup is that the layout styles will apply to everything in the container. In the case of Details, you can see that changing the block spacing will change the space between summary and inner blocks, as well as the inner blocks themselves. This may or may not be a desirable; if it isn't, then adding a container of some sort around only the inner blocks is probably a better solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Bug An existing feature does not function as intended
4 participants