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

An extra container class is getting added to the first element of each Query Loop #41026

Closed
ndiego opened this issue May 12, 2022 · 5 comments · Fixed by #41827
Closed

An extra container class is getting added to the first element of each Query Loop #41026

ndiego opened this issue May 12, 2022 · 5 comments · Fixed by #41827
Assignees
Labels
[Block] Query Loop Affects the Query Loop Block [Type] Bug An existing feature does not function as intended

Comments

@ndiego
Copy link
Member

ndiego commented May 12, 2022

Description

If you create a Query Loop, the first block inside of the Query loop will receive an erroneous wp-container-# class. This causes a major issue when that block is a Group block because the block will then receive two container classes. These classes add competing block gap specifications, especially with Rows and Stacks.

Step-by-step reproduction instructions

  1. Create a Query Loop block
  2. View the Query Loop on the frontend and see the erroneous container class getting added to the first element.

Screenshots, screen recording, code snippet

Group block with multiple container classes when the first child.
image

Group block with only one class when placed second, but note that now the Cover block has the wp-container-8 class, which it shouldn't.
image

Environment info

This issue exists in 6.0 RC2 as well as Gutenberg 13.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

@ndiego ndiego added [Type] Bug An existing feature does not function as intended [Block] Query Loop Affects the Query Loop Block labels May 12, 2022
@carolinan
Copy link
Contributor

carolinan commented Jun 9, 2022

I can confirm the problem on the front, not the editor.

I think it might be an issue with this piece of code in the layout support.

@ntsekouras
Copy link
Contributor

Thanks for reporting this @ndiego! That's an interesting problem indeed. It seems we have layout support in Post Template where we disallow editing in the UI and gets the default layout. The effect of this is getting the block gap styles(if set) for its children blocks and only if we don't have the grid view(we override the margin to zero.

The problem here occurs because Post Template doesn't have a wrapper div in save.js and the wrapper(ul) is added dynamically in the index.php. This results in layout php adding the container class in the first block as we don't have a wrapper, as we have in other blocks.

I'm not sure yet how it would be best to handle this: specifically for Post Template or if it's a case we should try to find some generic solution. Maybe before adding the extra classes there, make sure the first node contains the wp-block-${block-name}? 🤔 This might be related to the limitations we have for blocks with a single wrapper where other blocks like Cover have two?

--cc @andrewserong @ramonjd

@andrewserong
Copy link
Contributor

andrewserong commented Jun 20, 2022

Oh, interesting problem! I wonder if (a) solution might be to decrease (increase the number numerically) the priority on the layout support's render_block filter? Ideally, the layout support would be applied after the block itself has its final markup, I'd think?

Edit: I think the issue was to do with the block adding the wrapper li element after rendering each instance of the loop, rather than injecting it before rendering (#41026 (comment)).

@andrewserong
Copy link
Contributor

andrewserong commented Jun 20, 2022

I've put together a fix for this in #41802:

I believe the issue is that the block was manually creating the wrapper li markup rather than passing it in with the block instance when rendering each inner instance of the loop. By augmenting the block object prior to rendering the instances within the loop, we can ensure that the Post Template layout support is attached to the wrapper li element instead of to the first child of the inner blocks within the template.

This seems to fix it on the site frontend for me, but could use some more manually testing if folks have time, to make sure I haven't missed anything. 🙂

@andrewserong
Copy link
Contributor

Based on feedback, I've opened an alternative PR to explore a potential fix in #41827 🙂

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
5 participants