Query Loop: Default to querying posts when on singular content #65067
Conversation
mikachan
added
[Type] Bug
|
Size Change: +178 B (+0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
| // @wordpress/block-library should not depend on @wordpress/editor. | ||
| // However, we need to get the current post type in order to inherit the correct query. | ||
| // eslint-disable-next-line @wordpress/data-no-store-string-literals | ||
| const { getCurrentPostType } = select( 'core/editor' ); |
There was a problem hiding this comment.
If we need to access the current postType we should be able to get it via the block context by setting usesContext to include postType.
However I'm not sure this approach is solving the problem described 🤔
Instead of showing the posts of the current post type I do think the default state for the inherit option should still be that is always renders posts of the posts posts type.
The issue is that today it doesn't do that when the query loop is inserted inside content rather than inside a template.
Rather than this I think it might be a better route to add a check in PHP to check whether something like whether the inherit option is set to true and the is_singular check resolves to true.
In that case we change the default params to set the post type to post
There was a problem hiding this comment.
Thanks @fabiankaegy!
If we need to access the current postType we should be able to get it via the block context by setting usesContext to include postType.
Ah thank you, that's helpful.
Instead of showing the posts of the current post type I do think the default state for the inherit option should still be that is always renders posts of the posts posts type.
I agree - I was focused on making the Editor and the front end consistent, and assumed the front end was correct. But now I think it makes more sense for the query loop to default to querying posts, as this seems to be the safest assumption for most users. I've tried out is_singular() here: 51f9383
This new behavior also matches the block description:
mikachan
changed the title
|
Flaky tests detected in 7447863. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10929161840
|
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @alaczek, @autumnfjeld. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
mikachan
requested review from
jameskoster,
jasmussen,
richtabor and
fabiankaegy
|
I'm not sure that it always makes sense to default to the If you're on a single page, I can see the logic of doing that. However, if I'm on a single template for a custom post type, defaulting to posts is unexpected--most of the time, I'd expect to see a list of posts of the same post type. What do you think of defaulting the query block to the post type that matches the currently viewed post? |
|
We should check how the "Default" is potentially a bit misleading because it suggests the global default, which isn't how the option works. We can force it to work that way, but won't that involve adding a lot of logic to account for scenarios like the ones I mentioned above? As an alternative, I still think it might be worth exploring the idea of forcing Query Loops inside Content blocks to be custom, and hiding the 'Query Type' control altogether. The values for controls in the custom query set up could inherit from the global query as a default. |
|
Yeah this should never override a global query if one exists. If you are on an archive of a custom post type, or on the search template it should just inherit the global as a default. The crux of the issue is that created is that when there is no global query, like on single templates. It should default to posts. |
@fabiankaegy That makes sense to me for On single templates for custom post types (like a custom book, movie, news, or location post type), would you still expect the query to default to the |
|
@creativecoder from my perspective I would prefer the predictability of it defaulting to posts on all single templates unless you choose custom. We simply don’t have enough insights into what a CPT is used for to determine whether it should query other items. And even today when you insert a query loop on a single page of some CPT it displays the most recent posts in the editor. It just doesn’t on the frontend today. |
mikachan
changed the title
|
What am I doing wrong, I still see the option when editing a page? 🤔 
|
Good spot - I think it's because you're testing a page within the Editor, rather than via wp-admin -> Pages. I've added a check for editing singular content within the Editor which I think should cover this. Thanks @jameskoster! |
|
That fixed it :) Y'all may consider this an edge case not worth catering to, but how do you think we should handle Query Loops that have set query.mp4
I wonder if we should consider disabling the Query Type option in some templates like 404 as well. It doesn't make much sense there given there's no query to inherit. |
I think this is a great follow up 👍 But I don't think it should block this PR
We may be able to solve for this via an effect that runs on the template change which can then use the |
| const updateQuery = useCallback( | ||
| ( newQuery ) => setAttributes( { query: { ...query, ...newQuery } } ), | ||
| [ query, setAttributes ] | ||
| ); |
There was a problem hiding this comment.
This was from fixing a lint warning.
Ah, I think I was checking for I also fixed some lint warnings in
Yeah that sounds like a good plan, although I agree with @fabiankaegy in that this might be better in a follow-up. This is ready for another review. Thanks both 🙏 |
|
About to publish GB 19.3 and I see an approval here so just going to merge 😇 |
|
Please pardon my quick, after-the-fact comment, but I wanted to flag the use of The comment just above that block of code begins:
followed by a But |
|
Thanks for taking a look, @mikachan! The change in #65483 looks OK to me as far as protecting the main query instance, but I'm not certain that it's actually having an effect. By the time the post template block is rendering, that query object has already executed, and changing the query parameters at that point (via Taking a step back: If I'm following correctly, the changes in this PR have made it so that if a query block is inserted into post content, that query will always use |
I think I tested using
Yep, looks like this is the case. The forceful |
| select( coreStore ).__experimentalGetTemplateForLink()?.type; | ||
| const isInTemplate = 'wp_template' === currentTemplate; | ||
| const isInSingularContent = postType !== undefined; | ||
| return isInTemplate && ! isInSingularContent; |
There was a problem hiding this comment.
Why was isInSingularContent not enough? What conditions do I need to test?
| const isTemplate = useSelect( | ||
| ( select ) => { | ||
| const currentTemplate = | ||
| select( coreStore ).__experimentalGetTemplateForLink()?.type; |
There was a problem hiding this comment.
This is not a valid call, it's missing the link argument. Without this argument it's essentially checking wither we are in the site editor or not, because in the site editor this will always load the front page template, and in any other editor this will load nothing.
There was a problem hiding this comment.
Thanks for explaining this. Just closing the loop here that this was fixed in #65820.
What?
An attempt to try and fix the issue described in #64746.
Why?
To align the default/inherit query results between the Editor and the front end, and also hopefully make this block more intuitive and easier to understand.
How?
This adds a check for
is_singular()to check if the block has been added to a single post or page (i.e. within content), and if so, defaults to querying thepostpost type.It also hides the Query Type control when editing a single page/post/CPT, as these blocks will always default to a custom query type that queries posts. The Query Type control is still used when editing a template, where it makes more sense to default to inheriting the global query.
Testing Instructions
Screenshots or screencast
Front-end (posts should now render by default)
Editor (shouldn't be any change)
Query Type control changes (currently, the Query Type control is always shown)