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

REST API: Update default values for fields in the block type schema #22695

Merged
merged 6 commits into from
Jun 2, 2020

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented May 28, 2020

Description

This change aligns with the state of the corresponding change proposed to WordPress core to WP_Block_Type class: https://core.trac.wordpress.org/ticket/48529.

More details about this optional field:
https://developer.wordpress.org/block-editor/developers/block-api/block-registration/#parent-optional

Related code that uses this field that confirms that using an empty array would lead to unexpected results:

const blockAllowedParentBlocks = blockType.parent;
const parentName = getBlockName( state, rootClientId );
const hasBlockAllowedParent = checkAllowList(
blockAllowedParentBlocks,
parentName
);

const checkAllowList = ( list, item, defaultResult = null ) => {
if ( isBoolean( list ) ) {
return list;
}
if ( isArray( list ) ) {
// TODO: when there is a canonical way to detect that we are editing a post
// the following check should be changed to something like:
// if ( includes( list, 'core/post-content' ) && getEditorMode() === 'post-content' && item === null )
if ( includes( list, 'core/post-content' ) && item === null ) {
return true;
}
return includes( list, item );
}
return defaultResult;
};

@gziolo gziolo added [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks Core REST API Task Task for Core REST API efforts labels May 28, 2020
@gziolo gziolo requested a review from spacedmonkey May 28, 2020 09:44
@gziolo gziolo self-assigned this May 28, 2020
@gziolo gziolo requested a review from aduth May 28, 2020 09:45
@github-actions
Copy link

github-actions bot commented May 28, 2020

Size Change: 0 B

Total Size: 1.12 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.48 kB 0 B
build/block-directory/style-rtl.css 787 B 0 B
build/block-directory/style.css 787 B 0 B
build/block-editor/index.js 106 kB 0 B
build/block-editor/style-rtl.css 11.3 kB 0 B
build/block-editor/style.css 11.3 kB 0 B
build/block-library/editor-rtl.css 7.87 kB 0 B
build/block-library/editor.css 7.88 kB 0 B
build/block-library/index.js 125 kB 0 B
build/block-library/style-rtl.css 7.68 kB 0 B
build/block-library/style.css 7.68 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 190 kB 0 B
build/components/style-rtl.css 19.5 kB 0 B
build/components/style.css 19.5 kB 0 B
build/compose/index.js 9.32 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.11 kB 0 B
build/edit-navigation/index.js 7.88 kB 0 B
build/edit-navigation/style-rtl.css 857 B 0 B
build/edit-navigation/style.css 856 B 0 B
build/edit-post/index.js 302 kB 0 B
build/edit-post/style-rtl.css 5.43 kB 0 B
build/edit-post/style.css 5.43 kB 0 B
build/edit-site/index.js 14.1 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/index.js 8.88 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.6 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@spacedmonkey
Copy link
Member

I will test this later today.

@gziolo
Copy link
Member Author

gziolo commented May 28, 2020

As discussed in private chat, another option is to tweak the actual usage of this field in the inserter to treat an empty array as no parents and bail out from the check.

@aduth
Copy link
Member

aduth commented May 28, 2020

tweak the actual usage of this field in the inserter to treat an empty array as no parents and bail out from the check.

By "no parents", do you mean "no restriction on the types of parents" or "not allowed to be inserted within any nested context"? I think it could be a worthwhile distinction with each having valid use-cases, where it could be represented by the difference between [] and null.

@gziolo
Copy link
Member Author

gziolo commented May 28, 2020

By "no parents", do you mean "no restriction on the types of parents" or "not allowed to be inserted within any nested context"? I think it could be a worthwhile distinction with each having valid use-cases, where it could be represented by the difference between [] and null.

Exactly, this is something I'm hesitant about. There is also an inserter flag in the supports property of the block definition that serves a similar purpose.

@aduth
Copy link
Member

aduth commented May 28, 2020

Related: #7845

Copy link
Member

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

This didn't work in my testing.

@spacedmonkey
Copy link
Member

These lines need to change

} elseif ( isset( $schema['properties'][ $key ]['default'] ) ) {
} elseif ( array_key_exists( 'default', $schema['properties'][ $key ] ) ) {

The null value means you can't use isset.

@gziolo gziolo changed the title REST API: Update default value for parent in the block type schema REST API: Update default values for fields in the block type schema Jun 1, 2020
@gziolo gziolo force-pushed the update/rest-api-block-prent-default branch from 01de805 to 5fcaf3f Compare June 1, 2020 13:29
@gziolo
Copy link
Member Author

gziolo commented Jun 1, 2020

I went through the list of changes applied in WordPress core and tried to align this PR accordingly:
WordPress/wordpress-develop@822ca9e

It turned out example was missing in both RFC and REST API implementation.

Copy link
Member

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

This is line nit pick and this can be merged - https://github.com/WordPress/gutenberg/pull/22695/files#r433239396

@gziolo gziolo force-pushed the update/rest-api-block-prent-default branch from d22ee26 to 0f5ab25 Compare June 2, 2020 06:07
Copy link
Member

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

LGTM

@gziolo gziolo merged commit 2116931 into master Jun 2, 2020
@gziolo gziolo deleted the update/rest-api-block-prent-default branch June 2, 2020 08:12
@github-actions github-actions bot added this to the Gutenberg 8.3 milestone Jun 2, 2020
@ellatrix ellatrix mentioned this pull request Jun 16, 2020
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts [Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants