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

Pagination blocks require being deleted twice to be removed #41178

Open
annezazu opened this issue May 20, 2022 · 9 comments
Open

Pagination blocks require being deleted twice to be removed #41178

annezazu opened this issue May 20, 2022 · 9 comments
Labels
[Block] Query Pagination Affects the Query Pagination Block - used for pagination within the Query Loop Block [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

Description

This occurs both with 13.2.2 and, more concerning, 6.0 RC 3. Essentially in order to properly remove pagination blocks, you have to delete them twice. Nothing this for a minor release since we're tight on time but cc @ndiego and @gziolo for potentially more thoughts in case there's an easy fix ahead of RC4 tomorrow.

Step-by-step reproduction instructions

  1. Open Appearance > Editor
  2. Find pagination blocks within the Query Loop.
  3. Try to delete them.
  4. Notice they come back after you delete all three (next page, last page, the numbers).
  5. Delete all three again and notice they then delete.

Screenshots, screen recording, code snippet

double.delete.mov

Environment info

  • WordPress 6.0 RC 3 with and without GB 13.2.2
  • TT2

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

@annezazu annezazu added [Type] Bug An existing feature does not function as intended Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release [Block] Query Pagination Affects the Query Pagination Block - used for pagination within the Query Loop Block labels May 20, 2022
@carolinan
Copy link
Contributor

I can reproduce this, but I also think it is working as intended?
For example, if you remove all inner blocks in the columns, the selector where you choose the number of columns shows again.
If you remove all inner blocks in the social icons, three placeholder icons are shown, if you remove the content of the cover block, the placeholder title shows.

@gziolo gziolo added [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels May 20, 2022
@gziolo
Copy link
Member

gziolo commented May 20, 2022

When you remove all 3 blocks then probably it restores the default template for the Pagination block defined here:

const innerBlocksProps = useInnerBlocksProps( blockProps, {
template: TEMPLATE,
allowedBlocks: [
'core/query-pagination-previous',
'core/query-pagination-numbers',
'core/query-pagination-next',
],
__experimentalLayout: usedLayout,
} );

I wouldn't call it an obvious bug because it is expected behavior when you have inner blocks with some predefined initial state. For example, for the Query Loop block, we prevent the same behavior by introducing the setup step:

const Component = hasInnerBlocks ? QueryContent : QueryPatternSetup;

Let me summarize the behavior:

  • if there are no inner blocks then the block editor will try to use the default template provided
  • other blocks prevent that by rendering a temporary placeholder state where you can insert a block of a collection of blocks instead of the default template

Although, now I find it surprising that when you remove all the blocks for the second time it no longer injects inner blocks. I will test it, as my expectation is that once you save the content in the current form and refresh the page, then the editor should bring back all the default blocks

Edit: I can confirm that inner blocks were restored for the Pagination block after reloading the editor. In my case, I also didn't have to remove inner blocks twice. So the current handling is buggy but overall it works the way it should 😅

@gziolo gziolo added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels May 20, 2022
@annezazu
Copy link
Contributor Author

Ahh interesting! I was wondering if it was some sort of container issue. Thanks for changing to enhancement as it's a confusing UX to say the least and we can likely improve that initial setup state to make clearer what's going on.

@gziolo
Copy link
Member

gziolo commented May 21, 2022

Thanks for changing to enhancement

I actually changed back to a bug, but it's both to be honest 😅

@ntsekouras
Copy link
Contributor

as it's a confusing UX to say the least and we can likely improve that initial setup state to make clearer what's going on.

What do you think would be the desired result here?

In general the first time you delete the innerBlocks, they're shown again, because of a check we have about template in useInnerBlockTemplateSync. I think this is connected with controlled blocks(like Template Parts) and some technical nuances about what to check, but even if this a bug in this place(and we fix it), it would result to showing the template again, which in this case would be the three blocks.

@annezazu
Copy link
Contributor Author

I'm not quite sure! I'd be keen to hear some design thought. Right now though, there's nothing to indicate why a first pass deletion didn't work and why the placeholders returned. To me, it inherently felt broken and was frustrating to delete three things only to have them all pop back up seemingly spontaneously.

@talldan talldan removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Oct 17, 2022
@talldan
Copy link
Contributor

talldan commented Oct 17, 2022

I'm sure I remember seeing this reported for the buttons block too, but I can't find it now.

@annezazu
Copy link
Contributor Author

Guessing it's this @talldan :) #28217

@talldan
Copy link
Contributor

talldan commented Oct 18, 2022

That's the one 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Pagination Affects the Query Pagination Block - used for pagination within the Query Loop Block [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

No branches or pull requests

5 participants