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]: Add Posts List variation #26990

Merged
merged 6 commits into from
Nov 17, 2020
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { isMatch } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -16,15 +11,10 @@ import { useSelect, useDispatch } from '@wordpress/data';
import { useState, useEffect } from '@wordpress/element';
import { chevronDown } from '@wordpress/icons';

export const getMatchingVariationName = ( blockAttributes, variations ) => {
if ( ! variations || ! blockAttributes ) return;
const matches = variations.filter( ( { attributes } ) => {
if ( ! attributes || ! Object.keys( attributes ).length ) return false;
return isMatch( blockAttributes, attributes );
} );
if ( matches.length !== 1 ) return;
return matches[ 0 ].name;
};
/**
* Internal dependencies
*/
import { __experimentalGetMatchingVariation as getMatchingVariation } from '../../utils';

function __experimentalBlockVariationTransforms( { blockClientId } ) {
const [ selectedValue, setSelectedValue ] = useState();
Expand All @@ -46,7 +36,7 @@ function __experimentalBlockVariationTransforms( { blockClientId } ) {
);
useEffect( () => {
setSelectedValue(
getMatchingVariationName( blockAttributes, variations )
getMatchingVariation( blockAttributes, variations )?.name
);
}, [ blockAttributes, variations ] );
if ( ! variations?.length ) return null;
Expand Down

This file was deleted.

31 changes: 31 additions & 0 deletions packages/block-editor/src/utils/block-variation-transforms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* External dependencies
*/
import { isMatch } from 'lodash';

/** @typedef {import('@wordpress/blocks').WPBlockVariation} WPBlockVariation */

/**
* Matches the provided block variations with a block's attributes. If no match
* or more than one matches are found it returns `undefined`. If a single match is
* found it returns it.
*
* This is a simple implementation for now as it takes into account only the attributes
* of a block variation and not `InnerBlocks`.
*
* @param {Object} blockAttributes - The block attributes to try to find a match.
* @param {WPBlockVariation[]} variations - A list of block variations to test for a match.
* @return {?WPBlockVariation} - If a match is found returns it. If not or more than one matches are found returns `undefined`.
*/
export const __experimentalGetMatchingVariation = (
blockAttributes,
variations
) => {
if ( ! variations || ! blockAttributes ) return;
const matches = variations.filter( ( { attributes } ) => {
if ( ! attributes || ! Object.keys( attributes ).length ) return false;
return isMatch( blockAttributes, attributes );
} );
if ( matches.length !== 1 ) return;
return matches[ 0 ];
};
1 change: 1 addition & 0 deletions packages/block-editor/src/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { default as transformStyles } from './transform-styles';
export * from './theme';
export * from './block-variation-transforms';
70 changes: 70 additions & 0 deletions packages/block-editor/src/utils/test/block-variation-transforms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* Internal dependencies
*/
import { __experimentalGetMatchingVariation as getMatchingVariation } from '../block-variation-transforms';

describe( 'getMatchingVariation', () => {
describe( 'should not find a match', () => {
it( 'when no variations or attributes passed', () => {
expect(
getMatchingVariation( null, { content: 'hi' } )
).toBeUndefined();
expect( getMatchingVariation( {} ) ).toBeUndefined();
} );
it( 'when no variation matched', () => {
const variations = [
{ name: 'one', attributes: { level: 1 } },
{ name: 'two', attributes: { level: 2 } },
];
expect(
getMatchingVariation( { level: 4 }, variations )
).toBeUndefined();
} );
it( 'when more than one match found', () => {
const variations = [
{ name: 'one', attributes: { level: 1 } },
{ name: 'two', attributes: { level: 1, content: 'hi' } },
];
expect(
getMatchingVariation(
{ level: 1, content: 'hi', other: 'prop' },
variations
)
).toBeUndefined();
} );
it( 'when variation is a superset of attributes', () => {
const variations = [
{ name: 'one', attributes: { level: 1, content: 'hi' } },
];
expect(
getMatchingVariation( { level: 1, other: 'prop' }, variations )
).toBeUndefined();
} );
} );
describe( 'should find a match', () => {
it( 'when variation has one attribute', () => {
const variations = [
{ name: 'one', attributes: { level: 1 } },
{ name: 'two', attributes: { level: 2 } },
];
expect(
getMatchingVariation(
{ level: 2, content: 'hi', other: 'prop' },
variations
).name
).toEqual( 'two' );
} );
it( 'when variation has many attributes', () => {
const variations = [
{ name: 'one', attributes: { level: 1, content: 'hi' } },
{ name: 'two', attributes: { level: 2 } },
];
expect(
getMatchingVariation(
{ level: 1, content: 'hi', other: 'prop' },
variations
).name
).toEqual( 'one' );
} );
} );
} );
22 changes: 16 additions & 6 deletions packages/block-library/src/query/edit/query-placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ import { useSelect, useDispatch } from '@wordpress/data';
import {
useBlockProps,
__experimentalBlockVariationPicker,
__experimentalGetMatchingVariation as getMatchingVariation,
} from '@wordpress/block-editor';
import { createBlocksFromInnerBlocksTemplate } from '@wordpress/blocks';

const QueryPlaceholder = ( { clientId, name, setAttributes } ) => {
const { blockType, defaultVariation, variations } = useSelect(
const QueryPlaceholder = ( { clientId, name, attributes, setAttributes } ) => {
const {
blockType,
defaultVariation,
scopeVariations,
allVariations,
} = useSelect(
( select ) => {
const {
getBlockVariations,
Expand All @@ -20,19 +26,23 @@ const QueryPlaceholder = ( { clientId, name, setAttributes } ) => {
return {
blockType: getBlockType( name ),
defaultVariation: getDefaultBlockVariation( name, 'block' ),
variations: getBlockVariations( name, 'block' ),
scopeVariations: getBlockVariations( name, 'block' ),
allVariations: getBlockVariations( name ),
};
},
[ name ]
);
const { replaceInnerBlocks } = useDispatch( 'core/block-editor' );
const blockProps = useBlockProps();
const matchingVariation = getMatchingVariation( attributes, allVariations );
const icon = matchingVariation?.icon || blockType?.icon?.src;
const label = matchingVariation?.title || blockType?.title;
return (
<div { ...blockProps }>
<__experimentalBlockVariationPicker
icon={ blockType?.icon?.src }
label={ blockType?.title }
variations={ variations }
icon={ icon }
label={ label }
variations={ scopeVariations }
onSelect={ ( nextVariation = defaultVariation ) => {
if ( nextVariation.attributes ) {
setAttributes( nextVariation.attributes );
Expand Down
25 changes: 25 additions & 0 deletions packages/block-library/src/query/variations.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { postList } from '@wordpress/icons';

/**
* Internal dependencies
Expand All @@ -14,6 +15,30 @@ import {
} from './icons';

const variations = [
{
name: 'posts-list',
title: __( 'Posts List' ),
description: __(
'Display a list of your most recent posts, excluding sticky posts.'
),
icon: postList,
attributes: {
query: {
perPage: 4,
pages: 1,
offset: 0,
postType: 'post',
categoryIds: [],
Copy link
Member

Choose a reason for hiding this comment

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

It's pretty unfortunate that it needs to be all duplicated here because of how the query object was shaped:

"perPage": null,
"pages": 1,
"offset": 0,
"postType": "post",
"categoryIds": [],
"tagIds": [],
"order": "desc",
"orderBy": "date",
"author": "",
"search": "",
"exclude": [],
"sticky": ""

It raises a question of whether all those settings shouldn't be top-level attributes. It's clearly out of scope for this PR but I wanted to mention it. I understand that it's simpler to pass query through context this way, but at the same time, we lose some benefits that more granular attributes provide, like type validation, the flexibility to change the default value without breaking backward compatibility, less metadata saved in the content when defaults are used. An interesting fact is that the Latest Posts block defines all attributes as top-level fields:

"attributes": {
"categories": {
"type": "array",
"items": {
"type": "object"
}
},
"selectedAuthor": {
"type": "number"
},
"postsToShow": {
"type": "number",
"default": 5
},
"displayPostContent": {
"type": "boolean",
"default": false
},
"displayPostContentRadio": {
"type": "string",
"default": "excerpt"
},
"excerptLength": {
"type": "number",
"default": 55
},
"displayAuthor": {
"type": "boolean",
"default": false
},
"displayPostDate": {
"type": "boolean",
"default": false
},
"postLayout": {
"type": "string",
"default": "list"
},
"columns": {
"type": "number",
"default": 3
},
"order": {
"type": "string",
"default": "desc"
},
"orderBy": {
"type": "string",
"default": "date"
},
"displayFeaturedImage": {
"type": "boolean",
"default": false
},
"featuredImageAlign": {
"type": "string",
"enum": [
"left",
"center",
"right"
]
},
"featuredImageSizeSlug": {
"type": "string",
"default": "thumbnail"
},
"featuredImageSizeWidth": {
"type": "number",
"default": null
},
"featuredImageSizeHeight": {
"type": "number",
"default": null
},
"addLinkToFeaturedImage": {
"type": "boolean",
"default": false
}
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I know but this applies for all blocks with object properties. We had discussed that in my PR for this:#26162, but for back compat it was closed.

It raises a question of whether all those settings shouldn't be top-level attributes.

This is something to consider for sure. With my work on Query, I don't think it will have many other properties, so a rework could be considered.

tagIds: [],
order: 'desc',
orderBy: 'date',
author: '',
search: '',
sticky: 'exclude',
},
},
scope: [ 'inserter' ],
},
{
name: 'title-date',
title: __( 'Title and Date' ),
Expand Down