Query Loop: Use templateSlug and postType for more context #65820
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikachan
added
[Type] Enhancement
mikachan
requested review from
creativecoder,
jffng,
ntsekouras and
fabiankaegy
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +173 B (+0.01%) Total Size: 1.81 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in a43b176. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11608100329
|
ellatrix
reviewed
Oct 30, 2024
| // Unless the post type is `page`, as it usually doesn't make sense to loop | ||
| // through pages. | ||
| if ( postType && postType !== 'page' && query.postType !== postType ) { | ||
| newQuery.postType = postType; |
There was a problem hiding this comment.
Why not set use this context straight in the post-template block instead of editing the block on mount by setting attributes?
There was a problem hiding this comment.
I like the sound of this, but when I try moving the postType context to the post-template block, the Query Loop attributes aren't updated and so the incorrect post type is used in the inspector controls on mount, even though the correct post type is used in the query preview itself. For example, if I create a template for a custom post type and insert a Query Loop block, I'd expect the custom post type to be used, but instead it's always defaulting to post, even though the Query Loop block itself is displaying posts from the correct post type:
It looks like the attributes need to be updated in order for these controls to stay up to date (or we keep the context near the controls in query-content).
There was a problem hiding this comment.
Yes, could we not check the context in the controls as well? We want to disable the post type control when this context is set, right?
There was a problem hiding this comment.
Yes, checking the context in the controls seems to work! I've tried that out here: aec7cba.
Rather than disabling the control, we want to read the post type from the context so that post is not always used by default when inside a custom post type.
ellatrix
approved these changes
Oct 31, 2024
This was referenced Oct 31, 2024
ntsekouras
reviewed
Nov 1, 2024
| @@ -186,7 +187,10 @@ export default function PostTemplateEdit( { | |||
| } | |||
| // When we preview Query Loop blocks we should prefer the current | |||
| // block's postType, which is passed through block context. | |||
| const usedPostType = previewPostType || postType; | |||
| const usedPostType = | |||
There was a problem hiding this comment.
We need to be really careful with changes in Query Loop as it has many nuances and inherent complexity (besides the complexity we've added :) ).
This change effectively makes Query Loop kind of broken if not used in a page.
To test it insert a Query Loop block in a post and try to change the post type. The preview and the controls are wrong.
The previewPostType is also important for pattern suggestions on replace.
There was a problem hiding this comment.
Thanks for highlighting, @ntsekouras. I've opened a PR that removes this new context for now, which restores the previous behaviour when changing post types: #66681.
I'll work on figuring out the most appropriate post type in a follow-up PR, so the bug can be fixed on trunk as soon as possible.
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
…#65820) * Use templateSlug for additional context in the editor * Rename isTemplate to isSingularTemplate * Move singular logic to getQueryContext utils function * Simplify function by removing postType logic * Get postType from context * Fix for Post Editor --------- Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
The Query Loop block currently does not have much context about where it has been added when viewed in the Editor, and in some cases, it does not always make sense to inherit the global query. In #65067, we discussed that these cases can mostly be split between whether the block has been added to singular content, such as a single post or page; or non-singular content, such as the archive template or category template.
We addressed some of this logic in #65067, however, this only covered cases where the block is added to single posts or pages. As a follow-up to #65067, this PR also considers the 404, blank, and custom templates as singular content while also refactoring this recently added logic.
This PR does not add any other context for when the block is added to non-singular content, but I think we should address these cases in follow-up PRs, including:
Why?
Part of #30369, and a follow-up to #65067.
How?
templateSlugto the context available to the Query Loop block, which allows us to check which template is in use before deciding whether to inherit the query. There is some prior art in Decouple query from edit site #27972.postType, if available, to the current query.Testing Instructions
The functionality isn't too different to trunk, other than the 404, blank and custom templates are now included in the list of "singular content" contexts (i.e.
isSingular). ThepostTypeis also being applied where it's available, so please also test templates and content for custom post types.inherit: falsein singular content.inherit: truein non-singular content.Screenshots or screencast