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.

Sign up for GitHub

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 Loop: Default to querying posts when on singular content #65067

Merged
merged 19 commits into from
Sep 18, 2024
+122 −17
Merged

Query Loop: Default to querying posts when on singular content #65067

Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
88fc422
Fix linter warnings
mikachan Sep 4, 2024
094bddc
Replace the postType if current post type is different
mikachan Sep 4, 2024
49ba8ca
Remove currentPostType logic
mikachan Sep 5, 2024
51f9383
Default to posts if is_singular
mikachan Sep 5, 2024
4053496
Update test_rendering_post_template_with_main_query_loop_already_star…
mikachan Sep 5, 2024
a2c1429
Add test for query loop not inside a singular query
mikachan Sep 5, 2024
13bfbcc
Revert changes to QueryContent
mikachan Sep 9, 2024
cbc42d7
Merge branch 'trunk' into fix/query-loop-posttype
mikachan Sep 13, 2024
58c0956
Show query type control only when on a template
mikachan Sep 13, 2024
e5f8195
Merge branch 'trunk' into fix/query-loop-posttype
mikachan Sep 16, 2024
457f0f3
Move template logic to QueryInspectorControls
mikachan Sep 16, 2024
92d937d
Ensure inherit value is updated when not in a template
mikachan Sep 16, 2024
82a81c5
Update comment
mikachan Sep 16, 2024
ba494de
Rename showDefaultControl to isTemplate
mikachan Sep 18, 2024
40ecda4
Get postType from context
mikachan Sep 18, 2024
0f26b80
Add a check for singular content based on available post type
mikachan Sep 18, 2024
52aaf5a
Move inherit reset to a useEffect
mikachan Sep 18, 2024
47c84a7
Move isTemplate logic to QueryContent
mikachan Sep 18, 2024
7447863
Fix lint warnings
mikachan Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions packages/block-library/src/query/edit/query-content.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { useSelect, useDispatch } from '@wordpress/data';
import { useInstanceId } from '@wordpress/compose';
import { useEffect } from '@wordpress/element';
import { useEffect, useCallback } from '@wordpress/element';
import {
BlockControls,
InspectorControls,
Expand Down Expand Up @@ -46,10 +46,14 @@ export default function QueryContent( {
const innerBlocksProps = useInnerBlocksProps( blockProps, {
template: TEMPLATE,
} );
const { postsPerPage } = useSelect( ( select ) => {
const { postsPerPage, currentPostType } = useSelect( ( select ) => {
const { getSettings } = select( blockEditorStore );
const { getEntityRecord, getEntityRecordEdits, canUser } =
select( coreStore );
// @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' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

image

const settingPerPage = canUser( 'read', {
kind: 'root',
name: 'site',
Expand All @@ -68,6 +72,7 @@ export default function QueryContent( {
editedSettingPerPage ||
settingPerPage ||
DEFAULTS_POSTS_PER_PAGE,
currentPostType: getCurrentPostType(),
};
}, [] );
// There are some effects running where some initialization logic is
Expand All @@ -76,6 +81,11 @@ export default function QueryContent( {
// resetting again, so we need to mark these changes as not persistent
// with `__unstableMarkNextChangeAsNotPersistent`.

const updateQuery = useCallback(
( newQuery ) => setAttributes( { query: { ...query, ...newQuery } } ),
[ query, setAttributes ]
);
Comment on lines +96 to +99
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was from fixing a lint warning.


// Changes in query property (which is an object) need to be in the same callback,
// because updates are batched after the render and changes in different query properties
// would cause to override previous wanted changes.
Expand All @@ -92,17 +102,38 @@ export default function QueryContent( {
__unstableMarkNextChangeAsNotPersistent();
updateQuery( newQuery );
}
}, [ query.perPage, postsPerPage, inherit ] );
// When we inherit from global query we should check if the current post type
// is different from the query post type, excluding templates and patterns.
// We should default to posts if the current post type is not valid.
let newPostType = 'post';
const excludedPostTypes = [ 'wp_template', 'wp_block' ];
if ( ! excludedPostTypes.includes( currentPostType ) ) {
newPostType = currentPostType;
}
if ( inherit && newPostType !== query.postType ) {
query.postType = newPostType;
}
}, [
query,
postsPerPage,
inherit,
__unstableMarkNextChangeAsNotPersistent,
updateQuery,
currentPostType,
] );
// We need this for multi-query block pagination.
// Query parameters for each block are scoped to their ID.
useEffect( () => {
if ( ! Number.isFinite( queryId ) ) {
__unstableMarkNextChangeAsNotPersistent();
setAttributes( { queryId: instanceId } );
}
}, [ queryId, instanceId ] );
const updateQuery = ( newQuery ) =>
setAttributes( { query: { ...query, ...newQuery } } );
}, [
queryId,
instanceId,
__unstableMarkNextChangeAsNotPersistent,
setAttributes,
] );
const updateDisplayLayout = ( newDisplayLayout ) =>
setAttributes( {
displayLayout: { ...displayLayout, ...newDisplayLayout },
Expand Down
Loading