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

Core / ACF -> Blocks essential markup differences for innerBlocks #465

Closed
CreativeDive opened this issue Mar 1, 2021 · 61 comments
Closed

Comments

@CreativeDive
Copy link
Collaborator

Hey @elliotcondon;

while working on optimizing the user experience of ACF blocks, I encountered a markup issue:

The way of WordPress to create innerBlocks for Cover, Group or other block types, is to use an inner parent container like ".wp-block-cover__inner-container" or ".wp-block-group__inner-container" for inner blocks. All children inner blocks follows directly after this parent container.

<div class="wp-block-cover alignfull">
	<div class="wp-block-cover__inner-container">
		<p class="has-text-align-center has-large-font-size">Cover Full</p>
	</div>
</div>

or

<div class="wp-block-group alignfull">
	<div class="wp-block-group__inner-container">
		<p class="has-text-align-center has-large-font-size">Group Full</p>
	</div>
</div>

Therefore we can use very simple css rules to manage inner block alignment like "align-full" or "align-wide" and so much other important things:

[class*="__inner-container"] > .alignwide {
	...
}

or

[class*="__inner-container"] > *:not(.alignleft):not([data-align="left"]):not(.alignright):not([data-align="right"]) {
	width: auto;
	margin-left: auto;
	margin-right: auto;
}

This is working perfectly for frontend and backend (block editor) output, to show the correct sizing of inner aligned blocks.

BUT in the case of ACF blocks, this follows not the WordPress way in the case of the backend (block editor). For the frontend output, the upper CSS rules works fine with ACF, but not inside the block editor.

In the case of the block editor, ACF use own wrapper container for innerBlocks:

<div data-align="full" data-type="acf/my-acf-block">
	<div class="my-acf-block__inner-container">
		<div class="block-editor-inner-blocks">
			<div class="block-editor-block-list__layout">
				<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
			</div>
		</div>
	</div>
</div>

This means that the above CSS rules have no effect on the inner blocks. But it's very important to use CSS selectors like ">" to style only the first level and not all other nested inner block levels.

There are two ACF container which breaks this CSS rules ".block-editor-inner-blocks" and ".block-editor-block-list__layout".

The WordPress core blocks inside the block editor follows the same way like the frontend markup and everything works fine:

<div data-align="full" data-type="core/cover">
	<div class="wp-block-cover__inner-container block-editor-block-list__layout">
		<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
	</div>
</div>

How we can manage this issue, without create a bloated CSS rules especially for ACF blocks?

Are the parent containers ".block-editor-inner-blocks" and ".block-editor-block-list__layout" very essential for correct working of ACF blocks? Yes I suppose :-)

Would be possible to remove this parent containers or rename this? For example ".block-editor-block-list__layout" to ".block-editor-inner-blocks__inner-container".

Or do you could add a filter to the core of ACF which allow us to add additional class names to this container?

This would be VERY VERY helpful.

@elliotcondon
Copy link
Contributor

elliotcondon commented Mar 1, 2021

Hi @CreativeDive

"ACF use own wrapper container for innerBlocks:"

With regards to the above quote, can you please elaborate a little more on what you are stating here? I can confirm that we are not rolling any custom wrapper container for the <InnerBlocks /> component, or any other block component for that matter.

I wonder if this issue is related to a recent change in the block API that is affecting how blocks wrapping elements are rendered. There are tickets (WordPress/gutenberg#25088) and (#358) which may be related. These tickets discuss a change in the block API effecting how blocks are wrapped, which effects how the alignment classes are applied. I wonder if these are all related.

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 2, 2021

@elliotcondon I don't know how to explain it any easier? I've already mentioned everything to understand this issue. I'll try again and create a screen so that it's easier to understand. This is an ACF issue because the WordPress blocks work as expected. But the ACF blocks don't do it in the block editor. I will come back soon with the screens.

@elliotcondon your linked Gutenberg issues are not related to my issue.

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 2, 2021

@elliotcondon Now I will try to explain it again.

I'm a theme WordPress developer. I'm trying to get the same result from frontend in the block editor.

The main issue I'm addressing here is the result of ACF nested (inner) blocks in the block editor.

I don't know if you've already dealt with this topic, but to always correctly display nested (inner) blocks with all their setting options is a difficult task. Especially when it comes to inner blocks with a depth of 2, 3 or more levels.

The big mission is to get always the same result on frontend and in the block editor (backend).

All WordPress core blocks using a wrapper parent container for inner blocks.

It's always the same scheme:

<div class="wp-block-cover alignfull">
	<div class="wp-block-cover__inner-container">
		<p class="has-text-align-center has-large-font-size">Cover Full</p>
	</div>
</div>

or

<div class="wp-block-group alignfull">
	<div class="wp-block-group__inner-container">
		<p class="has-text-align-center has-large-font-size">Group Full</p>
	</div>
</div>

Each time, if I use ACF to include inner blocks, I always use the same scheme:

<div class="my-acf-block alignfull">
	<div class="my-acf-block__inner-container">
		<p class="has-text-align-center has-large-font-size">ACF Block Full</p>
	</div>
</div>

This are the frontend basics to deal with inner blocks and to get always the same result for inner blocks.

Imagine you have about 10 blocks supporting the inner blocks. But they all use the same scheme. That's great, because we can control all block types with the same CSS rules, like this:

[class*="__inner-container"] > .alignwide {
	...
}

The selector [class*="__inner-container"] > is magic and saves a lot of extra CSS rules. By the way, the WordPress default themes use the same selectors.

All this things are working correctly for WordPress core and ACF blocks on the frontend case.

The main issue I'm addressing here is the result of ACF nested (inner) blocks in the block editor.

Now let's look at the backend side inside the block editor.

We want to get the same result from the frontend in the backend block editor.

It is therefore important that the markup of the parent blocks and inner blocks is the same or follows the same scheme.

The WordPress way to show inner blocks follows exactly the same scheme like the frontend:

<div data-align="full" data-type="core/cover">
	<div class="wp-block-cover__inner-container block-editor-block-list__layout">
		<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
	</div>
</div>

CSS rules like the following working as expected also for frontend and backend (block editor):

[class*="__inner-container"] > .alignwide {
	...
}

What is not going as expected and causing issues?

Now I am going to describe the problem what it is about. ACF uses two additional HTML parent containers, which are located between the parent container block and the inner blocks.

These two additional parent containers are:

".block-editor-inner-blocks"

and

".block-editor-block-list__layout".

You can see it in the example below:

<div data-align="full" data-type="acf/my-acf-block">
	<div class="my-acf-block__inner-container">
		<div class="block-editor-inner-blocks">
			<div class="block-editor-block-list__layout">
				<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
			</div>
		</div>
	</div>
</div>

For this reason the css rules like the following example do not work for ACF inner blocks in the block editor case:

[class*="__inner-container"] > .alignwide {
	...
}

The result of the frontend and backend output is different. Because nested (inner) blocks are displayed incorrectly in the block editor if you have further settings such as "alignfull", "alignwide", "alignleft" or "alignright".

That's because the CSS rules don't work. Because the markup of ACF inner blocks is different from the frontend.

I hope you can understand it now :-)

Here you can see this issue in pictures:

Frontend Output - (Core Parent Block)

Here are a WordPress core parent block with nested inner blocks. All blocks (parent + inner blocks) are "aligned full":

wp-blocks-frontend

Block Editor Output - (Core Parent Block)

This upper example looks exactly same in the block editor, because all CSS rules matches perfectly:

wp-blocks-backend

Frontend Output - (ACF Parent Block)

At the frontend, it works as expected in the case, a ACF parent block wraps the inner blocks:

acf-blocks-frontend

Block Editor Output - (ACF Parent Block)

--> HERE <-- you can see the differences: In the case of the ACF Parent block, the inner "aligned full" blocks are not displayed correctly. The result of the inner blocks is not aligned, but "aligned full" is active for all inner blocks. This is not working, because the CSS rules are not matching in the case of the block editor. Because ACF uses two more other parent blocks between the main parent block and the inner blocks:

acf-blocks-backend

How we can solve this?

In short, the problem is that the representation of the ACF inner blocks in the block editor is not as expected.

We could solve this, if we could remove this two additional parent blocks:

".block-editor-inner-blocks"

and

".block-editor-block-list__layout".

... in this case, all inner blocks follows directly after the block markup we use for the frontend or we can add an additional class to the last parent container, directly before all inner blocks.

With an additional class this could be like the following:

Change ".block-editor-block-list__layout" to ".block-editor-block-list__layout my-acf-block__inner-container".

A possible solution would be a filter, to allow us to add an additional class name depending on the ACF parent block name.

@CreativeDive
Copy link
Collaborator Author

@elliotcondon Many descriptions will show you this issue. A correct solution, however, would be to save that many additional CSS rules. It is very important to always use a simple method to correctly display nested blocks with two or more levels.

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 2, 2021

@elliotcondon the best practice would be:

Best practice

Changing the markup like the default WordPress way.

From:

<div data-align="full" data-type="acf/my-acf-block">
	<div class="block-editor-inner-blocks">
		<div class="block-editor-block-list__layout">
			<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
		</div>
	</div>
</div>

To:

<div data-align="full" data-type="acf/my-acf-block">
	<div class="my-acf-block__inner-container block-editor-block-list__layout">
		<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
	</div>
</div>

This follows the same scheme like WordPress core blocks:

<div data-align="full" data-type="core/cover">
	<div class="wp-block-cover__inner-container block-editor-block-list__layout">
		<p role="group" aria-label="Paragraph block" data-type="core/paragraph">Cover Full</p>
	</div>
</div>

How can we solve it?

There is one little difficulty that you need to be aware of.

If we use ACF inner blocks, we creating templates like this:

<div class="my-acf-block <?php echo esc_html( $block ); ?>">
	[maybe other code here]
	<div class="my-acf-block__inner-container"> // <-- This must be always the last parent of inner blocks
		<InnerBlocks />
	</div>
</div>

This upper example is the best practice, because it follows the WordPress default way.

The difficulty now is that the inner container ".my-acf-block__inner-container" is permanently inserted in the template and it would appear a second time in the block editor, if ACF would be insert the "*__inner-container" to the last parent like this: "wp-block-cover__inner-container block-editor-block-list__layout".

The result in the block editor would be:

<div class="my-acf-block">
	<div class="my-acf-block__inner-container">
		<div class="my-acf-block__inner-container block-editor-block-list__layout">
			<p role="group" aria-label="Paragraph block" data-type="core/paragraph">My inner block</p>
		</div>
	</div>
</div>

You can see the ".my-acf-block__inner-container" container would appear two times in this case.

Therefore a better solution would be perfect.

A solution by ACF, which insert the ".my-acf-block__inner-container" container automatically for both cases (frontend + backend) as the "LAST" parent of all other inner blocks.

Please believe me, this way is consistent and very important, to get always the same result. At the end it is so much easier to deal with inner block settings.

It's possible for ACF to create a fix or an optional option, to allow us to going the same way like all other WordPress default block, which can include inner blocks? @elliotcondon what do you think about this issue?

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 2, 2021

@elliotcondon

Maybe a solution like this:

<InnerBlocks parentContainer="true" />

or if we can't pass the name of the parent block to the innerBlocks element:

<InnerBlocks parentContainerClass="my-acf-block__inner-container" />

@elliotcondon
Copy link
Contributor

Hi @CreativeDive. Thanks for the further detail.

I think there may have been some misunderstanding from my previous reply. Please know that I do understand the issue, and see from your comparisons that there is a difference in HTML output between core and ACF blocks. I also understand the importance of consistency for CSS rules, and how this issue presents a major problem for theme developers.

With that said, I want to confirm that the ACF PRO plugin is not intentionally adding these two additional HTML parent containers (".block-editor-inner-blocks" and ".block-editor-block-list__layout"). I have searched our source code and can confirm there is no occurrence of these elements.

When implementing the <InnerBlocks /> component in JS, we simply call that component as per normal JSX - without any additional wrapping elements.

To solve this issue, we need to discover why there is a difference of HTML output between core and ACF block types. I suspect that in a recent version of Gutenberg, the API has changed which is causing 3rd party block type (such as ACF blocks) to use a legacy or "non experimental" render function.

Can you help identify when this issue started? Perhaps the bug was introduced in WP 5.1, 5.2, 5.3, 5.4, 5.5 or 5.6?
Once we identify when the issue began, we can then begin to find relevant changelog notes that can help us identify and solve the issue.

Looking forward to hearing from you.

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 3, 2021

@elliotcondon I can't remember it ever being any different in WordPress / Gutenberg versions in the past. If I remember correctly, the two parent containers were always set. But that doesn't solve the problem.

Now that you mention that this containers are not from ACF, can we analyze it like this:

If we look at the inner block component without adding more template containers with the ACF, we only get this:

I can see the element will wrap from two parent container each time.

<div class="block-editor-inner-blocks">
	<div class="block-editor-block-list__layout">

		<innerBlocks />

	</div>
</div>

And this inner block component content, is wrapped always from the first block parent container:

<div data-type="acf/my-acf-block" data-title="Grid Columns" class="block-editor-block-list__block wp-block is-selected">

	...

</div>

The result is always:

<div data-type="acf/my-acf-block" data-title="My ACF Block" class="block-editor-block-list__block wp-block is-selected">

	<div class="block-editor-inner-blocks">
		<div class="block-editor-block-list__layout">

			<innerBlocks />

		</div>
	</div>

</div>

It looks to me as if it follows exactly the scheme that WordPress uses for the core blocks. With the ecxeption of the ".block-editor-inner-blocks". This container is unnecessary.

If we continue to follow the scheme of the core blocks, the container ".block-editor-block-list__layout" should contain the class my-acf-block__inner-container.

But the container ".block-editor-block-list__layout" around the innerBlocks element, destroyed the default hierarchy.

Default CSS rules, like the following, dosen't work now:

[class*="__inner-container"] > .alignwide {
	...
}

The core block scheme of block including inner blocks

However, this hierarchy would be correct:

<div data-type="core/cover" data-title="Cover" class="block-editor-block-list__block wp-block is-selected">

	<div class="wp-block-cover__inner-container block-editor-block-list__layout">

		<innerBlocks />

	</div>

</div>

Now we know that this leads to a display issues in the block editor. But how can you solve this issue?

The easiest fix would be, to add the my-acf-block__inner-container class to the container ".block-editor-block-list__layout".

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 3, 2021

@elliotcondon I have also detect other inconsistencies depending with ACF blocks:

The WordPress core blocks way

If a block alignment is selected for a core block, inside the block editor, a container <div class="wp-block" data-align="full"> with the selected alignment wraps the core block:

<div class="wp-block" data-align="full">

	<h2 role="group" aria-label="Block: Heading" class="block-editor-rich-text__editable block-editor-block-list__block is-selected rich-text" data-type="core/heading" data-title="Heading" style="white-space: pre-wrap;">My Core Heading</h2>

</div>

The ACF blocks way

If a block alignment is selected for an ACF block, inside the block editor, a container <div class="wp-block" data-align="full"> with the selected alignment is positioned INSIDE (not outside like core blocks) of the ACF block:

Additionally the alignment attribute [data-align="full"] was added directly to the block itself. However, this cannot be observed in the case of the core blocks.

<div data-align="full" data-type="acf/my-acf-block" data-title="My ACF Block" class="block-editor-block-list__block is-selected">

	<div class="wp-block" data-align="full">

		<innerBlocks />

	</div>

</div>

This is totally different to the scheme of the core blocks. This is another issue that can cause problems designing the correct view in the block editor.

In the core blocks way, ACF blocks alignment should be like this markup:

<div class="wp-block" data-align="full">

	<div data-type="acf/my-acf-block" data-title="My ACF Block" class="block-editor-block-list__block is-selected">

		<innerBlocks />

	</div>

</div>

@elliotcondon do you have any idea, why this is totally different to the core blocks?

@CreativeDive
Copy link
Collaborator Author

@elliotcondon I can confirm, this is a main issue by the Gutenberg innerBlocks component. I have tested some other plugins which add blocks with inner blocks. Always the same inconsistencies.

This is horror. I wonder why the same markup is not used here? This will cause many design problems in the block editor in the future, because the markup does not correspond to the frontend.

@CreativeDive
Copy link
Collaborator Author

@elliotcondon Now I have created a new issue for the Gutenberg editor.

WordPress/gutenberg#29513

I hope someone of the Gutenberg Team will see this issue. Unfortunately, such tickets usually wait a long time until someone answers them or something happens at all.

But the current state of the different markup is really horror.

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 4, 2021

@elliotcondon I've feedback to my Gutenberg issue from @talldan:

WordPress/gutenberg#29513 (comment)

What do you say about his comment? Could be solve this the markup issues?

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 4, 2021

@elliotcondon here you can find all about, how we can solve this issue: https://make.wordpress.org/core/2020/11/18/block-api-version-2/

Including the possibility to support both, the old and the new API.

This sounds like a solution :-)

I am happy to test the changes. This will avoid a lot of headaches for theme developers in the future.

@CreativeDive
Copy link
Collaborator Author

@elliotcondon Can we hope that this will change in the next version? :-)

@elliotcondon
Copy link
Contributor

Hi @CreativeDive

Thank you for all the additional info and Gutenberg Ticket. It looks like we will need to investigate the Block API version 2, and begin testing the new "props" filters / functions.

One major issue when adopting a new API version is the question of backwards compatibility. Our focus for ACF Blocks is to support as many versions of WordPress as possible, not just the latest one. The Block API version2 is quite new, and some parts are still considered "experimental". Please don't anticipate us resolving this ticket quickly. This change will require time and considerations for all users.

@simonseddon
Copy link

@CreativeDive I was having the same issue and solved it using editor CSS. In my case I was using a display: grid on the parent block, and InnerBlocks were supposed to be in the grid, in a sort of masonry layout – which is of course ruined when the InnerBlocks get nested, as the classes which control their size are buried.

Originally I made a JS fix (on the linked ticket) which got me some way there - but was incredibly unrobust in the editor. It's like the blocks are continually rendering new versions of themselves, so you can't persist DOM changes.

Anyway.... that forced me to look for a CSS way of doing it, which I found using display: contents, documented on my ticket: #471 – appreciate this doesn't fix all scenarios, but did the trick for me.

@CreativeDive
Copy link
Collaborator Author

@simonseddon Thank you for your input, but this really sounds like a dirty fix that doesn't work in every way. In some cases, however, it can work with the completely different frontend and backend outputs.

But in truth, the backend output must be exactly the same as in the frontend. That is the only correct solution.

I hope we don't have to wait very long for changes to the ACF blocks.

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Mar 6, 2021

@elliotcondon thank you for response. I am glad that you recognized the importance.

But it is important for me to know how long we have to wait until ACF supports the block API v2?

I'm here for testing and to speed up development at the end. You can attach me, if you have worked out a new solution. After this I can send you feedback after testing. This can help you to speed up development.

I think this is really a VERY important change for anyone using ACF blocks.

Because the longer these differences exist, the more often people will have difficulties with them and the more often there will be inquiries and tickets.

@simonseddon
Copy link

@CreativeDive Yes as stated my solution doesn't work for all scenarios. Hopefully it'll help some people, though. Switching to the new API would be better but it'll take some time for ACF to roll that out and I have deadlines to consider :/ When that happens my solution won't break anything, as the divs I'm styling won't be there, and the original front-end styles will apply 😀

Best of luck getting your stuff working.

@JiveDig
Copy link

JiveDig commented May 10, 2021

Hit this same issue multiple times. It's very difficult to get around. Some blocks are easier than others, and some are just impossible to make work in the editor because of the different markup. This comment sounds extremely promising though. WordPress/gutenberg#29513 (comment)

@elliotcondon as far as backwards compatibility, I think an "opt in" attribute would make the most sense. Something like @CreativeDive already mentioned:

<InnerBlocks parentContainer="true" />

@mattgrshaw
Copy link
Member

@CreativeDive @JiveDig @simonseddon

Just a heads up that in the upcoming 5.10 release we've transitioned to v2 of the blocks API. That alone doesn't fix this issue, but it does let us start focusing on a proper fix for this.

We may still squeeze that fix into the 5.10 release, but otherwise, it will be one of the first blocks-related items we tackle in 5.11.

@JiveDig
Copy link

JiveDig commented Aug 19, 2021

@mattgrshaw Great news! If the editor markup will change to fix this it will be an amazing change for the better, but one that will require some dev on our end. As long as it's an opt-in thing or some way to not break existing instances of the blocks, can't wait to see this one get fixed. It'd be a great update for us in the editor.

@nick6352683
Copy link

I'm 100% with @mattgrshaw in terms of not breaking existing blocks...

@CreativeDive
Copy link
Collaborator Author

CreativeDive commented Aug 20, 2021

Hey @mattgrshaw,

thanks for the update.

First of all, I would like to emphasize that in my case the new version breaks the ACF block styles in the editor.

I think that an opt-in may be necessary here. On the other hand, it is difficult to use an opt-in, since all new users who are working with ACF for the first time would have to activate the Block API 2 manually.

I've worked out the differences between ACF blocks markup before and after the block API 2 was used.

With the block API 1:

<!-- ACF Block Level 1 -->

<div data-type="acf/my-block-parent" class="block-editor-block-list__block wp-block has-child-selected">
    <div class="acf-block-component acf-block-body">
        <div>
            <div class="acf-block-preview">

                <!-- ACF Block Level (InnerBlocks) -->

                <div class="block-editor-inner-blocks">
                    <div class="block-editor-block-list__layout">

                        <!-- ACF Block Level 2 -->

                        <div data-type="acf/my-block-child" class="block-editor-block-list__block wp-block">
                            <div class="acf-block-component acf-block-body">
                                <div>
                                    <div class="acf-block-preview">

With the block API 2:

<!-- ACF Block Level 1 -->

<div data-type="acf/my-block-parent" class="block-editor-block-list__block wp-block acf-block-component acf-block-body is-selected">
    <div>
        <div class="acf-block-preview">

            <!-- ACF Block Level (InnerBlocks) -->

            <div class="block-editor-inner-blocks">
                <div class="block-editor-block-list__layout">

                    <!-- ACF Block Level 2 -->

                    <div data-type="acf/my-block-child" class="block-editor-block-list__block wp-block acf-block-component acf-block-body">
                        <div>
                            <div class="acf-block-preview">

At the end only one DIV container per ACF block has disappeared in the new version.

The result is not what I expected. A DIV container was simply dispensed with. This change does not lead us to the same markup as the core blocks use.

Here, too, we still have to use workaround CSS for ACF blocks in order to get the same editor result as in the frontend.

Is ACF's PHP block solution limited here? Is there no way to match the ACF block markup with the core block markup?

@JiveDig @nick6352683 @simonseddon What is your experience with the new version?

@JiveDig
Copy link

JiveDig commented Aug 20, 2021

If there are any wrapping elements/levels removed in this update it will certainly break things for us. Due to these extra layers we have some ridiculous but necessary CSS selectors like this:

.mai-columns-wrap > .block-editor-inner-blocks > .block-editor-block-list__layout > .wp-block > .acf-block-component > div > .acf-block-preview > .mai-column {

These blocks can all be nested so it has to be this precise. If the wrapping elements from below could be removed, we'd be super happy :)

Markup 2021-08-20 at 15 59 47

@JiveDig
Copy link

JiveDig commented Aug 20, 2021

On another note, my SVGs that are used as Tab labels are getting stripped out in this version. Did something change there? Where should I report?

In acf_register_block_type the icon parameter is an SVG.

@lgladdy
Copy link
Member

lgladdy commented Aug 23, 2021

Markup 2021-08-20 at 16 28 02

@JiveDig Are you achieving this by using an <svg> directly in the tab title? If so, that will almost certainly be the kses changes, you can use the following to allow svgs in the markup:

Depending on your specific SVG, you might need to add more attributes (or tags) as allowed.

add_filter( 'wp_kses_allowed_html', function( $tags, $context ){
	if( $context === 'acf' ) {
		$tags['svg'] = array(
			'xmlns' => array(),
			'fill' => array(),
			'viewbox' => array(),
			'role' => array(),
			'aria-hidden' => array(),
			'focusable' => array(),
		);
		$tags['path'] = array(
			'd' => array(),
			'fill' => array(),
		);
	}
	return $tags;
}, 10, 2 );

@JiveDig
Copy link

JiveDig commented Aug 23, 2021

One question I have though - could you give us some examples of what you're using the extra div to achieve?

It's the same scenario as the others here, and my example CSS above. When dealing with flexbox or CSS grid the hierarchy/descendants are extremely important.

On front end it won't break, because we have the expected structure:
parent (display: flex;)
| direct child (responds to parent flex container as expected)

On the back end we need to hack this stuff to make it work:
parent (display is irrelevant here)
| direct child (extra div)
|| grandchild (another extra div)
||| great grandchild (the div we actually want to target.

This means we currently have CSS selectors like mentioned earlier:
.mai-columns-wrap > .block-editor-inner-blocks > .block-editor-block-list__layout > .wp-block > .acf-block-component > div > .acf-block-preview > .mai-column {

All of these blocks can be nested inside each other so we have to target with this specificity.

If you remove one of those divs we'll have to update our CSS to do so. That's fine to do, but if the plan is to match the markup directly and remove the extra divs so flexbox/grid can work as expected (plus a ton of other benefits) then it'd be better to match the v1 markup for now until a bigger change later when we all update our CSS to match.

@JiveDig
Copy link

JiveDig commented Aug 23, 2021

Are you achieving this by using an directly in the tab title?

Yes, exactly. Are there other options I'm missing besides hijacking the entire kses filter? I want an icon that isn't a dashicon, I wonder if there's an easier way. Maybe I can run acf/validate_value or acf/prepare_field or similar and add the icons there.

@lgladdy
Copy link
Member

lgladdy commented Aug 23, 2021

Yes, exactly. Are there other options I'm missing besides hijacking the entire kses filter? I want an icon that isn't a dashicon, I wonder if there's an easier way. Maybe I can run acf/validate_value or acf/prepare_field or similar and add the icons there.

@JiveDig The kses escaping happens as the last possible thing before output (as it's designed to, for security!) so I think the kses filter would be the only way forward here. We do apply the ACF context in the kses filter I provided above, so it will only affect any ACF escaped output rather than the whole of WordPress.

That said, your use case here is interesting and something we could probably support better. If you're already loading css into the editor, would it be helpful it we supported adding a custom class to a <a class="acf-tab-button"> in the field group settings to achieve this? Happy to hear about any other ways you think we can support this better for a future release.

@JiveDig
Copy link

JiveDig commented Aug 23, 2021

The SVG stuff probably should have been a separate issue, sorry about this. I can probably do it with all CSS via a background-image or mask-image. I just need to find the time to do it before the release ;P

@mattgrshaw
Copy link
Member

Just a heads up that we've pushed out 5.10.0-RC2 which adds back in the missing div brought up by @CreativeDive.

So just to summarize, with this release we'll be on Blocks API v2, but with the same markup as we had on v1. And we're discussing some ways to move towards the new/proper markup in the next release without breaking any workarounds already in place for the old markup.

@lgladdy
Copy link
Member

lgladdy commented Aug 23, 2021

The SVG stuff probably should have been a separate issue, sorry about this. I can probably do it with all CSS via a background-image or mask-image. I just need to find the time to do it before the release ;P

@JiveDig Yeh - GitHub didn't let me split your comment out to a new issue so I figured I'd just deal with it here! Either way, the kses filter will be our recommended way of solving this in 5.10, we'll make sure we get that snippet up on the acf website for release for other people - so thanks for pointing it out!

@JiveDig
Copy link

JiveDig commented Aug 23, 2021

Either way, the kses filter will be our recommended way of solving this in 5.10, we'll make sure we get that snippet up on the acf website for release for other people - so thanks for pointing it out!

@lgladdy Thanks. This would probably benefit others but I was already able to do it via CSS so I'm ready for an update of our plugin when the ACF update is out.

@CreativeDive
Copy link
Collaborator Author

Hey again,

this issue here is essential ... good thinks take time. But at the long run, to make the ACF block markup more WordPress core friendly will be the best decision here for the future.

@mattgrshaw I can confirm, for step one the best option is to match the old ACF block markup by introduce the block API 2 with the upcoming ACF release. Only matching exactly the old markup will prevent that block styles will break.

@mattgrshaw thanks for the RC2 update. It seems everything is working like before with API 1, but now with the API 2.

@lgladdy it's hard to find universal examples. In my opinion there is no way, to change the ACF block markup (by introduce the block API 2) to be more like the frontend markup, without breaking editor styles.

In the past it was very hard and very frustrating to get the same result for ACF blocks in the editor like the frontend view.

Here is only one example of CSS I use to get ACF blocks working like the frontend:

.acf-block-component,
.acf-block-body {
	width: auto!important;
	max-width: none!important;
}

[data-align="wide"] > .acf-block-body > div > .acf-block-preview > .alignwide {
	width: 100%!important;
}

[class*="__inner-container"] > .block-editor-inner-blocks > .block-editor-block-list__layout > *:not(.items):not([data-align]):not(.alignwide):not(.alignfull) {
	max-width: calc( var(--contentWidth) - 4.5rem )!important;
	width: calc( 100% - 4.5rem )!important;
	margin-left: auto;
	margin-right: auto;
}

Partially there are very very confusing parent / child CSS rules to get it to work. There are a thousand other examples that we don't see right now. @JiveDig mentioned a good example.

But what happens after the next release?

I don't know if the "Opt-In" or the "Opt-Out" is the better way to change the ACF block markup?

Opt-In means, many of ACF users will still work with the old (bad) markup, when the release will be there. I assume that many users will never use this Opt-In. Because they don't know about it or don't know how to deal with it.

On the other hand, maybe the "Opt-Out" is the better decision? All new users automatically use the better markup. Users who are negatively affected by the change will inform themselves and could then Opt-Out.

Ultimately, that's a decision by @deliciousbrains

But the change will be necessary. I know the styles will break in my case, but I'm ready to invest some more days the finalize the ACF block markup in the right direction. Because I know it will be worth it in the end.

@JiveDig
Copy link

JiveDig commented Aug 23, 2021

Worth noting, I'm totally fine with it being opt-out and a breaking change when the time comes. It'd be really easy to remove our complex editor styles and just use what we're already using on the front end.

@roryashfordbentley
Copy link

roryashfordbentley commented Dec 7, 2021

I have recently been dealing with the same issues related to InnerBlock markup in the editor.

Let me preface this by saying I fully appreciate that this issue is a WordPress problem and not directly an ACF problem.

Researching this led me to plenty of similar complaints from developers using the vanilla block API. Things like display:flex on a parent element can break layouts when the child element is a wrapper div in the editor.

One solution I saw in another thread was to apply display: content to these divs which for most markup works reasonably well, the additional divs get ignored but it does require a large css selector with a lot of specificity and no guarantee that WP won't change the markup in the future (as they have before).

The recommended WordPress solution at the moment is to switch using <InnerBlock> and instead use the innerBlocksProps hook.

You can see a great example of this on the core/button block edit logic here: https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-library/src/buttons/edit.js

Im wondering if its possible for us to use that hook somehow within our render_callback with JSX enabled. I had a very quick attempt at it using the following:

'render_callback' => function ($block) {
    $nested_blocks = [
        [
            'acf/expander-label',
            [
                'placeholder' => 'Add a label'
            ]
        ],
        [
            'acf/expander-content',
            [
                'placeholder' => 'Add content'
            ]
        ]
    ];

    $inner_block_props = [
        'template' => $nested_blocks
    ];

    echo esc_attr(wp_json_encode($inner_block_props));

    echo '<><div innerBlockProps="' .esc_attr(wp_json_encode($inner_block_props)) . '" /></>';
}

This didnt work but the above div did get rendered in the backend as so

<>
 <div innerBlockProps=[object, Object]></div>

I'm guessing that this isn't going to be possible right now but if it is possible in the future I think innerBlockProps would prove more useful to ACF blocks that <InnerBlocks>. It is worth noting that innerBlockProps is still tagged as experimental but there is an open PR to change it to stable in one of the next releases.

@CreativeDive
Copy link
Collaborator Author

@lgladdy @mattgrshaw

I am wondering how it is scheduled to solve this issue? I keep having issues with the different markup and would like to have them solved. Is there hope for the next release?

@willrowe
Copy link

innerBlockProps is no longer experimental and was released as stable in WordPress v5.9.

@CreativeDive
Copy link
Collaborator Author

@lgladdy @mattgrshaw any update on this?

This Github issue addresses the similar topic with the different markup. With the 12.7.0 RC1 from Gutenberg something was apparently changed here, but only in a special case it seems. Could this still be helpful for you?

WordPress/gutenberg#38613

12.7.0 RC1

Changelog
Enhancements

Remove data-align divs for themes that support layout. (https://github.com/WordPress/gutenberg/pull/38613)

@lgladdy
Copy link
Member

lgladdy commented Feb 24, 2022

No updated on this yet I'm afraid.

This change will require us to implement some kind of versioning or opt-in system for ACF Blocks features, as we can't change the current markup unilaterally.

So this will likely have to come as part of #569, which was held up by the significant work we've had to do in ACF 5.12 to support WordPress 5.9.

@CreativeDive
Copy link
Collaborator Author

@lgladdy thanks for the feedback. Seeing this in 5.13 would be very helpful. I finally want to get started and continue on a good basis.

Don't forget to reply to #605 please.

I've found a lot of scenarios where copying, pasting, duplicating ACF blocks doesn't work.
I consider it very important to fix these issues. I hope you see it the same way. I explained everything about it.

@mike-sheppard
Copy link

How we can solve this?

In short, the problem is that the representation of the ACF inner blocks in the block editor is not as expected.

We could solve this, if we could remove this two additional parent blocks:

".block-editor-inner-blocks"

and

".block-editor-block-list__layout".

I haven't read through the whole thread but a simple workaround we've been using is adding this to our editor stylesheet, hope it helps!

//  Remove ACF additional preview markup from affecting our styles
.block-editor-inner-blocks,
.block-editor-block-list__layout {
  display: contents
}

@CreativeDive
Copy link
Collaborator Author

@mike-sheppard thanks for sharing. But this is not working in cases like this:

.parent-selector > .child-selector {...}

or in block editor style:

.parent-selector > .block-editor-inner-blocks .block-editor-block-list__layout .child-selector {...}

If you have a nested block and only want to format the parent block but not the child blocks, then this type of selectors is required. And this case is not working here.

@mike-sheppard
Copy link

mike-sheppard commented Mar 31, 2022

Yep, def. not a magic bullet to fix all our innerBlock markup woes, but if you want 90% of grid/flex issues fixed in the editor while we wait for block v2 markup (in v5.13 🙏) that's a nice & simple workaround

@CreativeDive
Copy link
Collaborator Author

@mike-sheppard I hope so too.

@lgladdy
Copy link
Member

lgladdy commented May 12, 2022

Hello everyone,

This issue should be resolved in our ACF Blocks Developer Preview we released earlier today, if you opt-in to our new blockVersion 2.

Please try it out and give us your feedback on that issue!

@JiveDig
Copy link

JiveDig commented Sep 19, 2022

Resurrecting this one to ask about display: contents; on the extra wrapping divs. As others have mentioned, this works enough to get grid/flex to work with the extra elements, but I'm finding that the block toolbar never shows when this is used. Anyone else seeing this?

I have display: contents; on these elements, scoped to my blocks:

.my-block > .acf-innerblocks-container,
.my-block > .acf-innerblocks-container > .block-editor-block-list__layout {
    display: contents;
}

but I lose the toolbar on my inner (ACF) blocks.

@CreativeDive
Copy link
Collaborator Author

@JiveDig yes correct. That's the reason why this fix is no longer working. The (inline) block toolbar no longer works with display: contents.

@JiveDig
Copy link

JiveDig commented Sep 19, 2022

That's the reason why this fix is no longer working. The (inline) block toolbar no longer works with display: contents.

Uggghhhh. I didn't realize. This sets me back days worth of work. If I can't figure out how to get the toolbar to display, I'll have to revert a ton of changes I had planned for ACF 6.0 release. This limitation is quite a big deal IMO.

@CreativeDive
Copy link
Collaborator Author

@JiveDig actually display: contents should work if you apply it to .acf-innerblocks-container only, but not block-editor-block-list__block.

@JiveDig
Copy link

JiveDig commented Sep 19, 2022

Yeah that works but then breaks flexbox, which is the whole point of using display: contents in the first place.

Looking at issues like this too -- WordPress/gutenberg#7694

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants