Add back providesContext block attribute #23212

noahtallen · 2020-06-16T17:06:24Z

Description

Adds back providesContext mapping (see Add context property mapping to block registration #23180)

Note: converting context to uses_context and providesContext to provides_context will happen at the API level in #22686.

How has this been tested?

Locally in edit site. Made sure block context still works for dynamic blocks.

Screenshots

Types of changes

Bug fix

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

noahtallen · 2020-06-16T17:07:27Z

lib/class-wp-block.php

@@ -107,8 +107,8 @@ public function __construct( $block, $available_context = array(), $registry = n

 		$this->available_context = $available_context;

-		if ( ! empty( $this->block_type->context ) ) {


does anyone know is this is used anywhere else? I unfortunately am not familiar with block context code.

It should be rather usesContext, it doesn't error when this context is not provided.

We would need to update all occurrences of context in block.json files of core blocks.

Why? Isn't this just an internal switch which parses context from block.json, turns it into usesContext, and then the WPBlock class takes usesContext to set the context property on itself?

i.e. doesn't the external API stay the same?

github-actions · 2020-06-16T17:13:08Z

Size Change: 0 B

Total Size: 1.13 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`	7.26 kB	0 B
`build/block-directory/style-rtl.css`	955 B	0 B
`build/block-directory/style.css`	955 B	0 B
`build/block-editor/index.js`	106 kB	0 B
`build/block-editor/style-rtl.css`	10.7 kB	0 B
`build/block-editor/style.css`	10.7 kB	0 B
`build/block-library/editor-rtl.css`	7.85 kB	0 B
`build/block-library/editor.css`	7.86 kB	0 B
`build/block-library/index.js`	129 kB	0 B
`build/block-library/style-rtl.css`	8.02 kB	0 B
`build/block-library/style.css`	8.02 kB	0 B
`build/block-library/theme-rtl.css`	749 B	0 B
`build/block-library/theme.css`	751 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`	196 kB	0 B
`build/components/style-rtl.css`	15.9 kB	0 B
`build/components/style.css`	15.9 kB	0 B
`build/compose/index.js`	9.6 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.44 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`	569 B	0 B
`build/dom/index.js`	3.17 kB	0 B
`build/edit-navigation/index.js`	8.26 kB	0 B
`build/edit-navigation/style-rtl.css`	1.04 kB	0 B
`build/edit-navigation/style.css`	1.04 kB	0 B
`build/edit-post/index.js`	303 kB	0 B
`build/edit-post/style-rtl.css`	5.6 kB	0 B
`build/edit-post/style.css`	5.6 kB	0 B
`build/edit-site/index.js`	16.6 kB	0 B
`build/edit-site/style-rtl.css`	3.13 kB	0 B
`build/edit-site/style.css`	3.13 kB	0 B
`build/edit-widgets/index.js`	9.34 kB	0 B
`build/edit-widgets/style-rtl.css`	2.54 kB	0 B
`build/edit-widgets/style.css`	2.54 kB	0 B
`build/editor/editor-styles-rtl.css`	486 B	0 B
`build/editor/editor-styles.css`	487 B	0 B
`build/editor/index.js`	44.8 kB	0 B
`build/editor/style-rtl.css`	3.82 kB	0 B
`build/editor/style.css`	3.82 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.72 kB	0 B
`build/format-library/style-rtl.css`	561 B	0 B
`build/format-library/style.css`	562 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`	537 B	0 B
`build/list-reusable-blocks/style.css`	537 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`	681 B	0 B
`build/nux/style.css`	676 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`	788 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	14 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.06 kB	0 B
`build/viewport/index.js`	1.85 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

gziolo · 2020-06-16T17:27:25Z

Related PR #22686. Depending on what we agree upon here, it should be updated within the linked PR.

@aduth, @epiqueras or @mcsf might have the best knowledge on what's the best name for this property and what parts of the codebase might need to be updated. I assume it's only used in PHP codebase. I'm curious if it was already backported to WordPress core.

epiqueras · 2020-06-16T18:10:15Z

usesContext is fine, but if we're going to rename it, we should rename it everywhere, including JSON and JS files. Otherwise, it will be confusing for people to deal with.

Ideally, /wp/v2/block-type/?context=edit wouldn't be the way of querying based on a block type's values. We shouldn't limit the potential names of properties of REST resources based on the API's global query parameters. Perhaps the query should be something like block-type[context]=... or query[context]=....

noahtallen · 2020-06-16T18:19:40Z

sigh. I feel like this is turning into a rabbit hole which I'm not super interested in going down. :p The initial work to do this conversion left out block context, which broke dynamic blocks which used it (and providesContext is still broken). From my POV, I just want to make sure block context continues working (otherwise FSE breaks entirely on the front end). If deeper changes need to be made to get block context to work with the block API, I'm not the best person to continue working on them since I have some other things to finish this week before my vacation. ;) <3

Otherwise, it will be confusing for people to deal with.

I hope no one started using context outside of Gutenberg. :p I do think that usesContext is a clearer name for the property anyways.

We shouldn't limit the potential names of properties of REST resources based on the API's global query parameters

I completely agree. Is this normal in WordPress?

TimothyBJacobs · 2020-06-16T18:50:07Z

We shouldn't limit the potential names of properties of REST resources based on the API's global query parameters.

Hard disagree on shadowing the context request parameter with a typed value. It is incredibly confusing. Ideally context might've been something like _context, but that ship has long since sailed.

Besides being confusing, the request object doesn't differentiate between query args and body params. So this would give incredibly odd behavior if we ever were to provide the context values, for instance in the renderer.

epiqueras · 2020-06-16T20:54:09Z

Sounds like we need to rename it to usesContext everywhere then.

epiqueras · 2020-06-16T20:56:42Z

If #23212 (comment) is true, then let's merge this for now as a fix at least.

cc @gziolo

noahtallen · 2020-06-16T21:06:19Z

that sounds good to me.

spacedmonkey · 2020-06-16T21:16:03Z

This PR is a duplicate of #22686 isn't it?

lib/compat.php

lib/class-wp-block.php

epiqueras · 2020-06-16T21:35:16Z

Unfortunately, renaming this would make backward compatibility really hard.

Can we keep context in the block type and just map it to use_context in the REST requests and responses?

TimothyBJacobs · 2020-06-16T21:43:14Z

Unfortunately, renaming this would make backward compatibility really hard.

Can we keep context in the block type and just map it to use_context in the REST requests and responses?

I have no issue with that from the REST API perspective.

noahtallen · 2020-06-16T21:45:11Z

Can we keep context in the block type

I'm not 100% sure, but is this what's happening right now? You still specify context in block.json, and you still access context via $block->context.

This just changes the registration property, which is then parsed into context in the WP_Block constructor.

At least that's my reading. If that's correct, I think these changes can be pretty minimal.

This PR is a duplicate of #22686 isn't it?

🤷 I think they accomplish two different things. This makes sure that context and providesContext are set in the first place via block registration. I don't know anything about the API work.

epiqueras · 2020-06-16T21:54:26Z

That PR is handling the block type properties for the REST endpoint.

I still think we should keep $this->block_type->context here. The mapping to use_context can happen at the API level.

noahtallen · 2020-06-16T22:07:24Z

I see what you mean. That makes sense to me. I'll just update this to only add back the providesContext property.

mcsf · 2020-06-17T10:40:18Z

Unfortunately, renaming this would make backward compatibility really hard.

@epiqueras, how hard is really hard? Right now, context is still a pretty new concept, and only lives in the Gutenberg plugin. If there is a time to rename it so as to keep JS-PHP consistency, it's likely now. We could make it usesContext.

Edit: for reference, Trac ticket 49927 is still open.

gziolo · 2020-06-17T10:49:16Z

Given that PHP linter warns about camelCase used in providesContext and REST API endpoint for block types uses snake_case for everything else, we probably should do the mappings regardless: providesContext and usesContext i JS and JSON files, provides_context and uses_context for PHP.

One of the tests fails on Travis and it might be an actual issue because it's related to template parts:
https://travis-ci.com/github/WordPress/gutenberg/jobs/350115876#L856

 Template Part › Template part block › Should load customizations when in a template even if only the slug and theme attributes are set.
857
858    TypeError: Cannot read property 'click' of undefined
859
860      49 | 				'//button[contains(text(), "header")]'
861      50 | 			);
862    > 51 | 			await switchToHeaderTemplatePartButton.click();
863         | 			                                       ^
864      52 | 
865      53 | 			// Edit it.
866      54 | 			await insertBlock( 'Paragraph' );
867
868      at Object.<anonymous> (specs/experiments/template-part.test.js:51:43)
869          at runMicrotasks (<anonymous>)

Let's merge this PR once this test passes.

epiqueras · 2020-06-17T15:25:36Z

So deprecate context in the client? I guess the server/client consistency is worth the trouble.

gziolo · 2020-06-17T16:17:59Z

So deprecate context in the client? I guess the server/client consistency is worth the trouble.

Yes, it seems like usesContext/uses_context would be the preferred way, knowing REST API limitations. I wish we could use context but 🤷‍♂️

noahtallen · 2020-06-17T18:13:49Z

Should I still merge this PR as is for now?

noahtallen · 2020-06-17T22:54:12Z

Just waiting for Travis to be happy.

spacedmonkey

Please change providesContext to provides_context.
In PHP, snake case is not allowed.

spacedmonkey · 2020-06-18T08:24:59Z

lib/compat.php

+			'title'           => 'title',
+			'category'        => 'category',
+			'context'         => 'context',
+			'providesContext' => 'providesContext',


Suggested change

'providesContext' => 'providesContext',

'providesContext' => 'provides_context',

gziolo · 2020-06-18T11:14:53Z

I think I sorted out everything in #22686 to have provides_context and uses_context in PHP. Let's close this one to fix all issues at once.

gziolo · 2020-06-18T11:15:36Z

@noahtallen, thank you for all work on this issue. It's a very difficult issue to solve :(

noahtallen added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. labels Jun 16, 2020

noahtallen requested review from spacedmonkey and TimothyBJacobs as code owners June 16, 2020 17:06

noahtallen self-assigned this Jun 16, 2020

noahtallen mentioned this pull request Jun 16, 2020

Add context property mapping to block registration #23180

Merged

6 tasks

noahtallen commented Jun 16, 2020

View reviewed changes

noahtallen requested review from gziolo, vindl, ockham and Addison-Stavlo June 16, 2020 17:08

gziolo requested review from epiqueras and mcsf June 16, 2020 17:27

noahtallen mentioned this pull request Jun 16, 2020

Site Title: Add support settings for colors, fonts, and line height #23007

Merged

12 tasks

spacedmonkey reviewed Jun 16, 2020

View reviewed changes

lib/compat.php Outdated Show resolved Hide resolved

spacedmonkey reviewed Jun 16, 2020

View reviewed changes

lib/class-wp-block.php Outdated Show resolved Hide resolved

spacedmonkey reviewed Jun 16, 2020

View reviewed changes

lib/class-wp-block.php Outdated Show resolved Hide resolved

noahtallen force-pushed the fix/context-property-name branch from c416efa to 94c9b2a Compare June 16, 2020 22:09

noahtallen changed the title ~~Use better name for context in block registration~~ Add back providesContext block attribute Jun 16, 2020

epiqueras approved these changes Jun 16, 2020

View reviewed changes

noahtallen added 3 commits June 17, 2020 11:14

Use better name for context in block registration

b1e27ae

Rename requiresContex to usesContext

091b02c

Revert to context

1912fe4

noahtallen force-pushed the fix/context-property-name branch from 94c9b2a to 1912fe4 Compare June 17, 2020 18:16

spacedmonkey requested changes Jun 18, 2020

View reviewed changes

spacedmonkey reviewed Jun 18, 2020

View reviewed changes

gziolo closed this Jun 18, 2020

gziolo deleted the fix/context-property-name branch June 18, 2020 11:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add back providesContext block attribute #23212

Add back providesContext block attribute #23212

noahtallen commented Jun 16, 2020 •

edited

Loading

noahtallen Jun 16, 2020

gziolo Jun 16, 2020

noahtallen Jun 16, 2020

noahtallen Jun 16, 2020 •

edited

Loading

github-actions bot commented Jun 16, 2020 •

edited

Loading

gziolo commented Jun 16, 2020

epiqueras commented Jun 16, 2020

noahtallen commented Jun 16, 2020

TimothyBJacobs commented Jun 16, 2020

epiqueras commented Jun 16, 2020

epiqueras commented Jun 16, 2020

noahtallen commented Jun 16, 2020

spacedmonkey commented Jun 16, 2020

epiqueras commented Jun 16, 2020

TimothyBJacobs commented Jun 16, 2020

noahtallen commented Jun 16, 2020

epiqueras commented Jun 16, 2020

noahtallen commented Jun 16, 2020 •

edited

Loading

mcsf commented Jun 17, 2020 •

edited

Loading

gziolo commented Jun 17, 2020 •

edited

Loading

epiqueras commented Jun 17, 2020

gziolo commented Jun 17, 2020

noahtallen commented Jun 17, 2020

noahtallen commented Jun 17, 2020

spacedmonkey left a comment

spacedmonkey Jun 18, 2020

gziolo commented Jun 18, 2020

gziolo commented Jun 18, 2020

		@@ -107,8 +107,8 @@ public function __construct( $block, $available_context = array(), $registry = n

		$this->available_context = $available_context;

		if ( ! empty( $this->block_type->context ) ) {

	'providesContext' => 'providesContext',
	'providesContext' => 'provides_context',

Add back providesContext block attribute #23212

Add back providesContext block attribute #23212

Conversation

noahtallen commented Jun 16, 2020 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

noahtallen Jun 16, 2020

Choose a reason for hiding this comment

gziolo Jun 16, 2020

Choose a reason for hiding this comment

noahtallen Jun 16, 2020

Choose a reason for hiding this comment

noahtallen Jun 16, 2020 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jun 16, 2020 • edited Loading

gziolo commented Jun 16, 2020

epiqueras commented Jun 16, 2020

noahtallen commented Jun 16, 2020

TimothyBJacobs commented Jun 16, 2020

epiqueras commented Jun 16, 2020

epiqueras commented Jun 16, 2020

noahtallen commented Jun 16, 2020

spacedmonkey commented Jun 16, 2020

epiqueras commented Jun 16, 2020

TimothyBJacobs commented Jun 16, 2020

noahtallen commented Jun 16, 2020

epiqueras commented Jun 16, 2020

noahtallen commented Jun 16, 2020 • edited Loading

mcsf commented Jun 17, 2020 • edited Loading

gziolo commented Jun 17, 2020 • edited Loading

epiqueras commented Jun 17, 2020

gziolo commented Jun 17, 2020

noahtallen commented Jun 17, 2020

noahtallen commented Jun 17, 2020

spacedmonkey left a comment

Choose a reason for hiding this comment

spacedmonkey Jun 18, 2020

Choose a reason for hiding this comment

gziolo commented Jun 18, 2020

gziolo commented Jun 18, 2020

noahtallen commented Jun 16, 2020 •

edited

Loading

noahtallen Jun 16, 2020 •

edited

Loading

github-actions bot commented Jun 16, 2020 •

edited

Loading

noahtallen commented Jun 16, 2020 •

edited

Loading

mcsf commented Jun 17, 2020 •

edited

Loading

gziolo commented Jun 17, 2020 •

edited

Loading