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 Loop: Use templateSlug and postType for more context #65820

Merged
merged 7 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/block-library/src/post-template/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"displayLayout",
"templateSlug",
"previewPostType",
"enhancedPagination"
"enhancedPagination",
"postType"
],
"supports": {
"reusable": false,
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/post-template/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export default function PostTemplateEdit( {
} = {},
templateSlug,
previewPostType,
postType: postTypeFromContext,
},
attributes: { layout },
__unstableLayoutClassNames,
Expand Down Expand Up @@ -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 =
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

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 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.

postTypeFromContext !== 'page'
? postTypeFromContext
: previewPostType || postType;
return {
posts: getEntityRecords( 'postType', usedPostType, {
...query,
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/query/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"default": false
}
},
"usesContext": [ "postType" ],
"usesContext": [ "postType", "templateSlug" ],
"providesContext": {
"queryId": "queryId",
"query": "query",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,15 @@ import { useToolsPanelDropdownMenuProps } from '../../../utils/hooks';
const { BlockInfo } = unlock( blockEditorPrivateApis );

export default function QueryInspectorControls( props ) {
const { attributes, setQuery, setDisplayLayout, isTemplate } = props;
const { query, displayLayout } = attributes;
const {
attributes,
setQuery,
setDisplayLayout,
postTypeFromContext,
isSingular,
} = props;
const { query, displayLayout } = attributes;
let {
order,
orderBy,
author: authorIds,
Expand All @@ -61,6 +67,16 @@ export default function QueryInspectorControls( props ) {
parents,
format,
} = query;
// If a post type is set in context, update `postType` to match it,
// unless the post type is `page`, as it usually doesn't make sense to loop
// through pages.
if (
postTypeFromContext &&
postTypeFromContext !== 'page' &&
postTypeFromContext !== postType
) {
postType = postTypeFromContext;
}
const allowedControls = useAllowedControls( attributes );
const showSticky = postType === 'post';
const {
Expand Down Expand Up @@ -118,7 +134,7 @@ export default function QueryInspectorControls( props ) {
}, [ querySearch, onChangeDebounced ] );

const showInheritControl =
isTemplate && isControlAllowed( allowedControls, 'inherit' );
! isSingular && isControlAllowed( allowedControls, 'inherit' );
const showPostTypeControl =
! inherit && isControlAllowed( allowedControls, 'postType' );
const postTypeControlLabel = __( 'Post type' );
Expand Down
27 changes: 10 additions & 17 deletions packages/block-library/src/query/edit/query-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import EnhancedPaginationControl from './inspector-controls/enhanced-pagination-
import QueryToolbar from './query-toolbar';
import QueryInspectorControls from './inspector-controls';
import EnhancedPaginationModal from './enhanced-pagination-modal';
import { getQueryContextFromTemplate } from '../utils';

const DEFAULTS_POSTS_PER_PAGE = 3;

Expand All @@ -42,24 +43,15 @@ export default function QueryContent( {
tagName: TagName = 'div',
query: { inherit } = {},
} = attributes;
const { postType } = context;
const { templateSlug, postType } = context;
const { isSingular } = getQueryContextFromTemplate( templateSlug );
const { __unstableMarkNextChangeAsNotPersistent } =
useDispatch( blockEditorStore );
const instanceId = useInstanceId( QueryContent );
const blockProps = useBlockProps();
const innerBlocksProps = useInnerBlocksProps( blockProps, {
template: TEMPLATE,
} );
const isTemplate = useSelect(
( select ) => {
const currentTemplate =
select( coreStore ).__experimentalGetTemplateForLink()?.type;
const isInTemplate = 'wp_template' === currentTemplate;
const isInSingularContent = postType !== undefined;
return isInTemplate && ! isInSingularContent;
},
[ postType ]
);
const { postsPerPage } = useSelect( ( select ) => {
const { getSettings } = select( blockEditorStore );
const { getEntityRecord, getEntityRecordEdits, canUser } =
Expand Down Expand Up @@ -106,9 +98,9 @@ export default function QueryContent( {
} else if ( ! query.perPage && postsPerPage ) {
newQuery.perPage = postsPerPage;
}
// We need to reset the `inherit` value if not in a template, as queries
// are not inherited when outside a template (e.g. when in singular content).
if ( ! isTemplate && query.inherit ) {
// We need to reset the `inherit` value if in a singular template, as queries
// are not inherited when in singular content (e.g. post, page, 404, blank).
if ( isSingular && query.inherit ) {
newQuery.inherit = false;
}
if ( !! Object.keys( newQuery ).length ) {
Expand All @@ -117,10 +109,10 @@ export default function QueryContent( {
}
}, [
query.perPage,
query.inherit,
postsPerPage,
inherit,
isTemplate,
query.inherit,
isSingular,
__unstableMarkNextChangeAsNotPersistent,
updateQuery,
] );
Expand Down Expand Up @@ -167,7 +159,8 @@ export default function QueryContent( {
setDisplayLayout={ updateDisplayLayout }
setAttributes={ setAttributes }
clientId={ clientId }
isTemplate={ isTemplate }
postTypeFromContext={ postType }
isSingular={ isSingular }
/>
</InspectorControls>
<BlockControls>
Expand Down
60 changes: 59 additions & 1 deletion packages/block-library/src/query/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
* Internal dependencies
*/
import { terms } from './fixtures';
import { getEntitiesInfo, getValueFromObjectPath } from '../utils';
import {
getEntitiesInfo,
getValueFromObjectPath,
getQueryContextFromTemplate,
} from '../utils';

describe( 'Query block utils', () => {
describe( 'getEntitiesInfo', () => {
Expand Down Expand Up @@ -61,4 +65,58 @@ describe( 'Query block utils', () => {
expect( result ).toBe( 'test' );
} );
} );

describe( 'getQueryContextFromTemplate', () => {
it( 'should return the correct query context based on template slug', () => {
expect( getQueryContextFromTemplate() ).toStrictEqual( {
isSingular: true,
} );
expect( getQueryContextFromTemplate( '404' ) ).toStrictEqual( {
isSingular: true,
templateType: '404',
} );
expect( getQueryContextFromTemplate( 'blank' ) ).toStrictEqual( {
isSingular: true,
templateType: 'blank',
} );
expect( getQueryContextFromTemplate( 'single' ) ).toStrictEqual( {
isSingular: true,
templateType: 'single',
} );
expect(
getQueryContextFromTemplate( 'single-film' )
).toStrictEqual( {
isSingular: true,
templateType: 'single',
} );
expect( getQueryContextFromTemplate( 'page' ) ).toStrictEqual( {
isSingular: true,
templateType: 'page',
} );
expect( getQueryContextFromTemplate( 'wp' ) ).toStrictEqual( {
isSingular: true,
templateType: 'custom',
} );
expect( getQueryContextFromTemplate( 'category' ) ).toStrictEqual( {
isSingular: false,
templateType: 'category',
} );
expect(
getQueryContextFromTemplate( 'category-dog' )
).toStrictEqual( {
isSingular: false,
templateType: 'category',
} );
expect( getQueryContextFromTemplate( 'archive' ) ).toStrictEqual( {
isSingular: false,
templateType: 'archive',
} );
expect(
getQueryContextFromTemplate( 'archive-film' )
).toStrictEqual( {
isSingular: false,
templateType: 'archive',
} );
} );
} );
} );
29 changes: 29 additions & 0 deletions packages/block-library/src/query/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -435,3 +435,32 @@ export const useUnsupportedBlocks = ( clientId ) => {
[ clientId ]
);
};

/**
* Helper function that returns the query context from the editor based on the
* available template slug.
*
* @param {string} templateSlug Current template slug based on context.
* @return {Object} An object with isSingular and templateType properties.
*/
export function getQueryContextFromTemplate( templateSlug ) {
// In the Post Editor, the template slug is not available.
if ( ! templateSlug ) {
return { isSingular: true };
}
let isSingular = false;
let templateType = templateSlug === 'wp' ? 'custom' : templateSlug;
const singularTemplates = [ '404', 'blank', 'single', 'page', 'custom' ];
const templateTypeFromSlug = templateSlug.includes( '-' )
? templateSlug.split( '-', 1 )[ 0 ]
: templateSlug;
const queryFromTemplateSlug = templateSlug.includes( '-' )
? templateSlug.split( '-' ).slice( 1 ).join( '-' )
: '';
if ( queryFromTemplateSlug ) {
templateType = templateTypeFromSlug;
}
isSingular = singularTemplates.includes( templateType );

return { isSingular, templateType };
}
Loading