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

Query block enhanced pagination: false positives on patterns/template-parts (including TT4 default templates) #55706

Closed
luisherranz opened this issue Oct 30, 2023 · 10 comments · Fixed by #55714
Assignees
Labels
[Block] Query Loop Affects the Query Loop Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@luisherranz
Copy link
Member

luisherranz commented Oct 30, 2023

Description

As reported by @afercia in this PR, the enhanced pagination of the Query block cannot be enabled in the Twenty Twenty-Four theme because we were being very cautious in detecting unsupported blocks.

This is the explanation of the issue:

  • The enhanced pagination is the first of a series of user experiences based on client-side navigation. Other ones that we would like to try in the future are infinite scroll, instant search or client-side form submissions.
  • This client-side navigation can only be enabled when all the blocks of that section are compatible with them. In this case, the section is the Query block and its inner blocks.
  • We are still working on this compatibility layer, so in WP 6.4 we decided to restrict compatibility to Core blocks.
  • We are checking if all the inner blocks of the Query block are compatible in the Editor.
  • If there is a block that is not compatible, the enhanced pagination cannot be activated. For example, if you add a block from a plugin, a modal appears telling you that this block is not compatible and that the enhanced pagination has been disabled. If you try to enable it again, you can't because the toggle is disabled until you remove the unsupported block. You also see a notice informing you about this.
  • Even though all Core blocks are compatible, there are blocks that can change outside of the template that contains the Query block: the synced patterns and template parts.
  • For example, imagine that you add a synced pattern to your Query block. It doesn't contain any blocks from plugins, so everything is compatible and you can enable the enhanced pagination. Now, you open another template that also contains the same synced pattern and edit it from there, adding a block from a plugin. As there's no way to change the setting of the previous template, so we cannot guarantee anymore that the enhanced pagination will work correctly.
  • For that reason, we currently consider patterns and template parts as non-supported blocks.
  • The default templates of TT4 contain a "Post Meta" template-part inside the Query block (<!-- wp:template-part {"slug":"post-meta"} /-->), therefore you can't activate the enhanced pagination.
  • But unless you edit that template part and include a block from a plugin, it's really a false positive.

Step-by-step reproduction instructions

  • Install TT4.
  • Go to edit the home page.
  • Try to enable the enhanced pagination of the Query block.
  • The enhanced pagination is disabled and cannot be enabled.

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

@richtabor
Copy link
Member

Would the easiest/quickest solution be to make TT4 not use a template part in the query block?

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Oct 31, 2023
@peterwilsoncc
Copy link
Contributor

Would the easiest/quickest solution be to make TT4 not use a template part in the query block?

It's certainly worth considering a TT4 change for 6.4.0

I think this will be needed eventually as a defensive measure for themes containing the pattern <!-- wp:query {"enhancedPagination":true} --><!-- wp:template-part /--><!-- /wp:query --> in the download package without realizing it has the potential to be problematic.

That could be done in 6.4.1 if TT4 is edited if you wanted a little breathing room to get this PR wrapped up.

@luisherranz
Copy link
Member Author

@carolinan mentioned here that using patterns or template parts is best practice, so I thought it would be controversial to remove it:

Many block themes will want to use template part or patterns inside a query, because this helps developers reduce code duplication, and I may even say it is best practice.

But I agree with you that it will be better to target this change for 6.4.1 (I also proposed it in Slack).

I'll investigate this possibility today.

@afercia
Copy link
Contributor

afercia commented Oct 31, 2023

Thank you for the detailed issue and the video on the PR, that helps a lot clarifying the issue. As I'm not very familiar with the underlying technical problem, it's still not very clear to me why blocks from plugins are considered not compatible.

I haven't seen a clear explanation of that, not even in the other related issues. For future reference and to help plugin authors better understand, could you please clarify exactly what makes plugin blocks not compatible? Thanks.

@luisherranz
Copy link
Member Author

Sure 🙂

After client-side navigation, we need to make sure that the interactive blocks are properly hydrated, but we can't guarantee that the interactive blocks that were not created with the Interactivity API will be hydrated because their hydration mechanism can vary. For example, they may rely on a document.querySelector triggered by a DOMContentLoaded event.

There's a discussion here about how to allow third-party blocks to declare that compatibility.

As that mechanism is not yet in place, for WP 6.4, we are considering any non-core block not compatible.

@MaggieCabrera
Copy link
Contributor

MaggieCabrera commented Oct 31, 2023

It's not just a matter of avoiding code duplication, but template parts ensure that user changes propagate to all instances where the pattern is used. Post meta is all over the theme, if we stop using the template part, the user will have to manually find where it is to make any changes.

@luminuu
Copy link
Member

luminuu commented Oct 31, 2023

Agreeing with Maggie here. Removing the template parts will be quite a PITA given we're just a week away from the 6.4 release.

@richtabor
Copy link
Member

It's not just a matter of avoiding code duplication, but template parts ensure that user changes propagate to all instances where the pattern is used. Post meta is all over the theme, if we stop using the template part, the user will have to manually find where it is to make any changes.

Yea, it's not ideal, but I do think we should ship the theme so that it works with the latest features.

I'd consider this a last-resort fallback, perhaps only abstracting the blocks where we need to, not everywhere.

@luisherranz
Copy link
Member Author

I'd consider this a last-resort fallback, perhaps only abstracting the blocks where we need to, not everywhere.

Yes. It's not all the template parts, only the template parts that are inside of the Query blocks.

But I think the solution we reached in the PR is solid enough to be merged. So, as long as people agree, there's no need to remove any TT4 template parts.

@luisherranz
Copy link
Member Author

luisherranz commented Nov 1, 2023

We finally merged the PR, so now the enhanced pagination setting (now renamed to "force page reload", which is the inverse of "enhanced pagination") should be able to be used in TT4.

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 [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants