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

Block API: Support optional block category #22192

Closed
wants to merge 5 commits into from
Closed

Block API: Support optional block category #22192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e53fc4
Blocks: Refactor to create category type
aduth May 7, 2020
018ffe6
Blocks: Add missing unit tests for getCategories
aduth May 7, 2020
eb34898
Blocks: Add selector getCategory
aduth May 7, 2020
d95fefd
Block API: Add getCategory singular function
aduth May 7, 2020
a28d026
Block API: Support optional block category
aduth May 7, 2020
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
28 changes: 27 additions & 1 deletion docs/designers-developers/developers/data/data-core-blocks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,21 @@ _Parameters_

_Returns_

- `Array`: Categories list.
- `Array<WPBlockTypeCategory>`: Categories array.

<a name="getCategory" href="#getCategory">#</a> **getCategory**

Returns a single category by slug. Canonicalizes category by slug, using
internal mapping of legacy category slugs to their updated normal form.

_Parameters_

- _state_ `Object`: Blocks state.
- _slug_ `string`: Category slug.

_Returns_

- `(WPBlockTypeCategory|undefined)`: Block category, if exists.

<a name="getChildBlockNames" href="#getChildBlockNames">#</a> **getChildBlockNames**

Expand Down Expand Up @@ -139,6 +153,18 @@ _Returns_

- `?WPBlockVariation`: The default block variation.

<a name="getDefaultCategory" href="#getDefaultCategory">#</a> **getDefaultCategory**

Returns the default block category.

_Parameters_

- _state_ `Object`: Blocks state.

_Returns_

- `(WPBlockTypeCategory|undefined)`: Default block category.

<a name="getFreeformFallbackBlockName" href="#getFreeformFallbackBlockName">#</a> **getFreeformFallbackBlockName**

Returns the name of the block for handling non-block content.
Expand Down
22 changes: 22 additions & 0 deletions lib/compat.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,25 @@ function gutenberg_render_block_with_assigned_block_context( $pre_render, $parse
return $block->render();
}
add_filter( 'pre_render_block', 'gutenberg_render_block_with_assigned_block_context', 9, 2 );

/**
* Appends the "Uncategorized" block category to the default set of block
* categories.
*
* This can be removed when plugin support requires WordPress 5.5.0+.
*
* @see Trac Ticket TBD
*
* @param array $categories Block categories.
*
* @return array Block categories with "Uncategorized" block category.
*/
function gutenberg_append_default_block_category( $categories ) {
$categories[] = array(
'slug' => 'uncategorized',
'title' => __( 'Uncategorized' ),
);

return $categories;
Copy link
Contributor

@youknowriad youknowriad May 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

What's the reasoning for actually registering a default category instead of just having a nullable category and just group blocks with a "null" or inexistant category together in the inserter? It might be simpler and avoids having to define a "default category"?

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the reasoning for actually registering a default category instead of just having a nullable category and just group blocks with a "null" or inexistant category together in the inserter? It might be simpler and avoids having to define a "default category"?

This is mentioned in the original comment:

  • "Uncategorized" may not need to be an actual block category, but instead handled in user interfaces where absence of category is handled.
    • This may actually be good! I'm only now considering it as I write the pull request comment. I do foresee a couple minor downsides.
    • Downside: It becomes a risk for duplication and syncing issues if each handling of the "Uncategorized" behavior needs to behave consistently. For example, both the Block Inserter and Block Manager would separately need to implement a (hopefully consistent) "Uncategorized" label. "Hoping" is always a risk, and we could avoid it by treating the default category as a specially-handled case.

It may also require a bit more "explicit" handling in the components where the groupings are arranged, since it wouldn't be able to be treated just the same as any other category.

In the end though, it still may be the simplest option here 🤷 Worth exploring at least.

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the reasoning for actually registering a default category instead of just having a nullable category and just group blocks with a "null" or inexistant category together in the inserter? It might be simpler and avoids having to define a "default category"?

Still in progress, but mostly completed, and seems quite a bit simpler indeed: #22280

}
add_filter( 'block_categories', 'gutenberg_append_default_block_category' );
10 changes: 9 additions & 1 deletion packages/block-editor/src/components/inserter/block-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ function InserterBlockList( {
} ) {
const {
categories,
defaultCategory,
collections,
items,
rootChildBlocks,
Expand All @@ -73,6 +74,7 @@ function InserterBlockList( {
);
const {
getCategories,
getDefaultCategory,
getCollections,
getChildBlockNames,
} = select( 'core/blocks' );
Expand All @@ -81,6 +83,7 @@ function InserterBlockList( {

return {
categories: getCategories(),
defaultCategory: getDefaultCategory(),
collections: getCollections(),
rootChildBlocks: getChildBlockNames( rootBlockName ),
items: getInserterItems( rootClientId ),
Expand Down Expand Up @@ -209,7 +212,12 @@ function InserterBlockList( {

{ ! hasChildItems &&
map( categories, ( category ) => {
const categoryItems = itemsPerCategory[ category.slug ];
const categoryItems =
itemsPerCategory[
category === defaultCategory
? undefined
: category.slug
];
Comment on lines +216 to +220
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 works, but it's a bit strange, because it's assuming there's a string key undefined in itemsPerCategory, i.e.

itemsPerCategory = {
   // ...
   undefined: [ /* ... */ ],
};

if ( ! categoryItems || ! categoryItems.length ) {
return null;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/blocks/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New Features

- Added a new function `getCategory` to retrieve a single category object by slug.

## 6.13.0 (2020-04-01)

### New Feature
Expand Down
18 changes: 15 additions & 3 deletions packages/blocks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,19 @@ Returns all the block categories.

_Returns_

- `Array<Object>`: Block categories.
- `Array<WPBlockTypeCategory>`: Block categories.

<a name="getCategory" href="#getCategory">#</a> **getCategory**
Copy link
Member Author

Choose a reason for hiding this comment

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

Should probably be consistent here with getDefaultCategory, if it's kept in the implementation. There was some prior concern about including this at all, since these singleton methods are discouraged. In any case, they should either both be included, or both be omitted, depending if the current implementation stays.


Returns a single category by slug.

_Parameters_

- _slug_ `string`: Category slug.

_Returns_

- `(WPBlockTypeCategory|undefined)`: Block category, if exists.

<a name="getChildBlockNames" href="#getChildBlockNames">#</a> **getChildBlockNames**

Expand Down Expand Up @@ -687,7 +699,7 @@ Sets the block categories.

_Parameters_

- _categories_ `Array<Object>`: Block categories.
- _categories_ `Array<WPBlockTypeCategory>`: Block categories.

<a name="setDefaultBlockName" href="#setDefaultBlockName">#</a> **setDefaultBlockName**

Expand Down Expand Up @@ -789,7 +801,7 @@ Updates a category.
_Parameters_

- _slug_ `string`: Block category slug.
- _category_ `Object`: Object containing the category properties that should be updated.
- _category_ `WPBlockTypeCategory`: Object containing the category properties that should be updated.

<a name="withBlockContentContext" href="#withBlockContentContext">#</a> **withBlockContentContext**

Expand Down
22 changes: 18 additions & 4 deletions packages/blocks/src/api/categories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,32 @@
*/
import { dispatch, select } from '@wordpress/data';

/** @typedef {import('../store/reducer').WPBlockTypeCategory} WPBlockTypeCategory */

/**
* Returns all the block categories.
*
* @return {Object[]} Block categories.
* @return {WPBlockTypeCategory[]} Block categories.
*/
export function getCategories() {
return select( 'core/blocks' ).getCategories();
}

/**
* Returns a single category by slug.
*
* @param {string} slug Category slug.
*
* @return {WPBlockTypeCategory|undefined} Block category, if exists.
*/
export function getCategory( slug ) {
return select( 'core/blocks' ).getCategory( slug );
}

/**
* Sets the block categories.
*
* @param {Object[]} categories Block categories.
* @param {WPBlockTypeCategory[]} categories Block categories.
*/
export function setCategories( categories ) {
dispatch( 'core/blocks' ).setCategories( categories );
Expand All @@ -24,8 +37,9 @@ export function setCategories( categories ) {
/**
* Updates a category.
*
* @param {string} slug Block category slug.
* @param {Object} category Object containing the category properties that should be updated.
* @param {string} slug Block category slug.
* @param {WPBlockTypeCategory} category Object containing the category
* properties that should be updated.
*/
export function updateCategory( slug, category ) {
dispatch( 'core/blocks' ).updateCategory( slug, category );
Expand Down
7 changes: 6 additions & 1 deletion packages/blocks/src/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ export {
getSaveContent,
} from './serializer';
export { isValidBlockContent } from './validation';
export { getCategories, setCategories, updateCategory } from './categories';
export {
getCategory,
getCategories,
setCategories,
updateCategory,
} from './categories';
export {
registerBlockType,
registerBlockCollection,
Expand Down
6 changes: 1 addition & 5 deletions packages/blocks/src/api/registration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ import { DEPRECATED_ENTRY_KEYS } from './constants';
* @property {string} name Block type's namespaced name.
* @property {string} title Human-readable block type label.
* @property {string} description A detailed block type description.
* @property {string} category Block type category classification,
* @property {string} [category] Block type category classification,
* used in search interfaces to arrange
* block types by category.
* @property {WPBlockTypeIcon} [icon] Block type icon.
Expand Down Expand Up @@ -203,10 +203,6 @@ export function registerBlockType( name, settings ) {
console.error( 'The "edit" property must be a valid function.' );
return;
}
if ( ! ( 'category' in settings ) ) {
console.error( 'The block "' + name + '" must have a category.' );
return;
}
if (
'category' in settings &&
! some( select( 'core/blocks' ).getCategories(), {
Comment on lines 207 to 208
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess to support the use-case described at #19279 (comment), we probably want to make sure that if a block type attempts to register with a category which doesn't actually exist, it should be treated the same as if it were not assigned a category at all. Rather than as it exists here currently, where an error will occur.

Expand Down
16 changes: 0 additions & 16 deletions packages/blocks/src/api/test/registration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,22 +173,6 @@ describe( 'blocks', () => {
expect( block ).toBeUndefined();
} );

it( 'should reject blocks without category', () => {
const blockType = {
settingName: 'settingValue',
save: noop,
title: 'block title',
},
block = registerBlockType(
'my-plugin/fancy-block-8',
blockType
);
expect( console ).toHaveErroredWith(
'The block "my-plugin/fancy-block-8" must have a category.'
);
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with non registered category.', () => {
const blockType = {
save: noop,
Expand Down
40 changes: 36 additions & 4 deletions packages/blocks/src/store/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,26 @@ import { combineReducers } from '@wordpress/data';
import { __ } from '@wordpress/i18n';

/**
* Module Constants
* @typedef WPBlockTypeCategory
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm on the fence about whether to call this WPBlockTypeCategory or WPBlockCategory. I think technically WPBlockTypeCategory is the more accurate name†, but (a) it's verbose and (b) we're already pretty consistent elsewhere in this module with calling things "block something" where we actually mean "block type something"
("block collections", "block names", "block styles", etc).

See also: #19279 (comment)

† Accuracy as in: Consider that we don't call labels of register_post_type as "post labels". It would be strange to think about it this way, as it would be very easy to confuse this as "the labels of a post", which is meaningless. That's why the function is called get_post_type_labels, and not get_post_labels.

*
* @property {string} slug Unique identifier.
* @property {string} title Human-readable label.
*/

/**
* Default category, used in absence of any assigned category for block type.
*
* @type {WPBlockTypeCategory}
*/
export const DEFAULT_CATEGORY = {
slug: 'uncategorized',
title: __( 'Uncategorized' ),
};

/**
* Default set of block type categories.
*
* @type {WPBlockTypeCategory[]}
*/
export const DEFAULT_CATEGORIES = [
{ slug: 'common', title: __( 'Common blocks' ) },
Expand All @@ -29,6 +48,7 @@ export const DEFAULT_CATEGORIES = [
{ slug: 'widgets', title: __( 'Widgets' ) },
{ slug: 'embed', title: __( 'Embeds' ) },
{ slug: 'reusable', title: __( 'Reusable blocks' ) },
DEFAULT_CATEGORY,
];

/**
Expand Down Expand Up @@ -199,10 +219,10 @@ export const groupingBlockName = createBlockNameSetterReducer(
/**
* Reducer managing the categories
*
* @param {Object} state Current state.
* @param {Object} action Dispatched action.
* @param {WPBlockTypeCategory[]} state Current state.
* @param {Object} action Dispatched action.
*
* @return {Object} Updated state.
* @return {WPBlockTypeCategory[]} Updated state.
*/
export function categories( state = DEFAULT_CATEGORIES, action ) {
switch ( action.type ) {
Expand All @@ -229,6 +249,17 @@ export function categories( state = DEFAULT_CATEGORIES, action ) {
return state;
}

/**
* Reducer managing the default category slug.
*
* @param {string} state Current state.
*
* @return {string} Updated state.
*/
export function defaultCategory( state = DEFAULT_CATEGORY.slug ) {
return state;
}

export function collections( state = {}, action ) {
switch ( action.type ) {
case 'ADD_BLOCK_COLLECTION':
Expand All @@ -254,5 +285,6 @@ export default combineReducers( {
unregisteredFallbackBlockName,
groupingBlockName,
categories,
defaultCategory,
collections,
} );
31 changes: 28 additions & 3 deletions packages/blocks/src/store/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ import {
includes,
map,
some,
find,
} from 'lodash';

/** @typedef {import('../api/registration').WPBlockVariation} WPBlockVariation */

/** @typedef {import('../api/registration').WPBlockVariationScope} WPBlockVariationScope */
/** @typedef {import('./reducer').WPBlockTypeCategory} WPBlockTypeCategory */

/**
* Given a block name or block type object, returns the corresponding
Expand Down Expand Up @@ -117,12 +118,36 @@ export function getDefaultBlockVariation( state, blockName, scope ) {
*
* @param {Object} state Data state.
*
* @return {Array} Categories list.
* @return {WPBlockTypeCategory[]} Categories array.
*/
export function getCategories( state ) {
return state.categories;
}

/**
* Returns a single category by slug. Canonicalizes category by slug, using
* internal mapping of legacy category slugs to their updated normal form.
*
* @param {Object} state Blocks state.
* @param {string} slug Category slug.
*
* @return {WPBlockTypeCategory|undefined} Block category, if exists.
*/
export function getCategory( state, slug ) {
return find( getCategories( state ), { slug } );
}

/**
* Returns the default block category.
*
* @param {Object} state Blocks state.
*
* @return {WPBlockTypeCategory|undefined} Default block category.
*/
export function getDefaultCategory( state ) {
return getCategory( state, state.defaultCategory );
}

/**
* Returns all the available collections.
*
Expand Down Expand Up @@ -273,7 +298,7 @@ export function isMatchingSearchTerm( state, nameOrType, searchTerm ) {
return (
isSearchMatch( blockType.title ) ||
some( blockType.keywords, isSearchMatch ) ||
isSearchMatch( blockType.category )
Boolean( blockType.category && isSearchMatch( blockType.category ) )
);
}

Expand Down
Loading