Try supporting post meta in block attributes #2740

mcsf · 2017-09-16T17:06:00Z

~~Experimental. Not fit to merge.~~ See updated docs for details.

codecov · 2017-09-17T03:49:30Z

Codecov Report

Merging #2740 into master will increase coverage by <.01%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master    #2740      +/-   ##
==========================================
+ Coverage   33.77%   33.77%   +<.01%     
==========================================
  Files         189      191       +2     
  Lines        5640     5930     +290     
  Branches      982     1053      +71     
==========================================
+ Hits         1905     2003      +98     
- Misses       3163     3319     +156     
- Partials      572      608      +36

Impacted Files	Coverage Δ
blocks/editable/index.js	`10.31% <ø> (ø)`	⬆️
editor/modes/visual-editor/block.js	`0% <0%> (ø)`	⬆️
blocks/api/serializer.js	`97.91% <100%> (ø)`	⬆️
editor/selectors.js	`94.73% <78.57%> (-1.67%)`	⬇️
editor/sidebar/post-schedule/index.js	`63.15% <0%> (-3.51%)`	⬇️
editor/modes/visual-editor/index.js	`0% <0%> (ø)`	⬆️
editor/modes/visual-editor/block-list.js	`0% <0%> (ø)`	⬆️
editor/inserter/index.js	`0% <0%> (ø)`	⬆️
editor/sidebar/table-of-contents/item.js	`0% <0%> (ø)`
editor/sidebar/table-of-contents/index.js	`0% <0%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68a93d4...d6f9296. Read the comment docs.

mtias · 2017-09-20T16:53:12Z

docs/attributes.md

+
+### Considerations
+
+By default, a meta field will be excluded from a post object's meta. This can be circumvented by explicitly making the field visible:


@pento this would be nice to address in general :)

Yeah, that's annoying.

@rmccue: I don't recall the history for why postmeta is only included in a response when it has the show_in_rest flag set. (As opposed to at least returning the visible postmeta when the user is appropriately authenticated.)

Meta fields aren't guaranteed to be "safe" by default. "Safe" is a few things, namely safe for JSON serialisation (which is lossy compared to PHP), and safe for capabilities (not all meta is always available).

The current behaviour exists for those reasons. When we initially didn't require show_in_rest, people noted that this would expose hidden fields created by plugin or users (turns out, a bunch of people use the "Custom Fields" metabox for random internal notes). Solving these problems in a consistent and safe way turned out to be essentially impossible.

show_in_rest is an indicator that plugin developers are accepting the limitations of the API for their meta fields.

These hidden fields would be shown if you do get_post_meta() calls, no? It feels like the API should adhere to the expectations of doing such authenticated calls.

mtias · 2017-09-20T16:54:15Z

editor/selectors.js

+		...block,
+		attributes: {
+			...block.attributes,
+			...reduce( metaAttrs, ( acc, val ) => {


Can we use full word argument names?

mtias · 2017-09-20T16:54:54Z

This is looking good to me and opens up a lot of possibilities for block authors, specially those transitioning from meta-boxes.

rmccue · 2017-09-21T06:30:18Z

docs/attributes.md

+		'single' => true,
+	) );
+}
+add_action( 'init', 'gutenberg_my_block_init' );


It may also be worth mentioning that register_meta treats the type as string by default, which may be surprising behaviour.

Undo hack in `setAtttributes`.

Both as exercise and to avoid need for block tests, which would distract from the object of this PR (meta support).

Fixes tests.

That's besides the concept of meta support

aduth · 2017-09-27T00:25:58Z

We should have added some test cases for the revised getBlock behavior, specifically defining the expected behavior of the return value when the block not present in state, because it would have surfaced that it currently crashes the application.

Select a block
Press trash in secondary actions
Boom

aduth · 2017-09-27T00:35:03Z

editor/selectors.js

+		}, {} );
+
+		// Avoid injecting an empty `attributes: {}`
+		if ( ! block.attributes && ! metaAttributes.length ) {


I'm confused how this is working, since metaAttributes.length will always be undefined, and this will shortcut before merging in attributes. I assume this should have been Object.keys( metaAttributes ).length.

Also, shouldn't we still want to merge meta attributes even if there are no other attributes of the block?

I'm confused how this is working

Ah, this one fell through the cracks. :( At some point there was a size( metaAttributes ).

merge meta attributes even if there are no other attributes of the block?

👍

aduth · 2017-09-27T00:40:05Z

editor/selectors.js

+			return result;
+		}, {} );
+
+		// Avoid injecting an empty `attributes: {}`


Is there a negative impact of injecting an empty attributes object? I think I might rather we normalize on the empty object.

The intent was to preserve the original behavior (returning an untouched item from blocksByUid) unless there was any meta. Due to my lack of knowledge, this seemed preferable, in case any consumer of the selector was comparing object references.

The intent was to preserve the original behavior (returning an untouched item from blocksByUid) unless there was any meta. Due to my lack of knowledge, this seemed preferable, in case any consumer of the selector was comparing object references.

Yeah, the intent became clearer in #2805 putting together the changes there, and I left the existing behavior. 👍

aduth · 2018-01-30T22:05:51Z

How does one use a meta attribute value in a (server-rendered) dynamic block?

mtias · 2018-01-31T13:23:30Z

@aduth wouldn't that be just get_post_meta( $id, $attr_key ) on the server?

aduth · 2018-01-31T13:41:06Z

Wouldn't one expect this to be available in the $attributes passed to render_callback ?

mcsf · 2018-01-31T14:37:14Z

I think there will likely be that expectation, yes. For the first pass, it seemed acceptable to require fetching via get_post_meta.

Mind you, this availability of data in $attributes doesn't happen for other non-comment sources (DOM, mainly); a somewhat similar example, wp:block, also handles post retrieval from ref. I mention this so we can better frame the discussion of which and how attributes should be provided to render_callback.

mtias · 2018-02-12T19:41:04Z

Not sure if expecting from attributes is better than using the standard API for retrieving meta fields — which is more likely to have been used by a theme already.

mcsf added Framework Issues related to broader framework topics, especially as it relates to javascript [Status] In Progress Tracking issues with work in progress labels Sep 16, 2017

mcsf force-pushed the try/post-meta-support branch from dea6042 to 8f9f372 Compare September 17, 2017 03:49

mcsf force-pushed the try/post-meta-support branch from b21264e to 10c412c Compare September 17, 2017 23:05

mcsf added Needs Tests and removed [Status] In Progress Tracking issues with work in progress labels Sep 18, 2017

mtias reviewed Sep 20, 2017

View reviewed changes

mtias approved these changes Sep 20, 2017

View reviewed changes

mtias mentioned this pull request Sep 20, 2017

Block API: Allow blocks to specify attributes that map to meta fields #2754

Closed

rmccue reviewed Sep 21, 2017

View reviewed changes

mcsf force-pushed the try/post-meta-support branch 2 times, most recently from 464bbcc to 27b217b Compare September 21, 2017 15:01

mtias added [Feature] Block API API that allows to express the block paradigm. [Type] Developer Documentation Documentation for developers labels Sep 21, 2017

mcsf force-pushed the try/post-meta-support branch 3 times, most recently from 6544a10 to e3e868e Compare September 22, 2017 13:51

mcsf added 8 commits September 22, 2017 15:07

First stab at meta support in block attributes

dce9ac5

Let block author specify meta shape (single or not)

20aa841

Undo hack in `setAtttributes`.

Defining Book as a third-party block

94d94e5

Both as exercise and to avoid need for block tests, which would distract from the object of this PR (meta support).

Bail meta support if not enough data

30f7dc0

Fixes tests.

Serialization: Ignore meta attributes

e0f3a89

Book block: Don't even render statically

4b2ac5e

That's besides the concept of meta support

Docs: Update & rename Attributes doc

8b10a40

Remove hypothetical Book block

37c7292

mcsf force-pushed the try/post-meta-support branch 2 times, most recently from 29f84f7 to f8c5a92 Compare September 22, 2017 14:13

mcsf force-pushed the try/post-meta-support branch 2 times, most recently from 69e19b8 to 8a3b86c Compare September 25, 2017 14:23

mcsf added 4 commits September 25, 2017 17:28

getBlock, setAttributes: fix & refactor handling of meta

35f5d1c

Docs: Mention register_meta type default

d7ffb68

getBlock: memoize w/ createSelector

402806b

Docs: Meta: Considerations: Type casting

d6f9296

mcsf force-pushed the try/post-meta-support branch from 8a3b86c to d6f9296 Compare September 25, 2017 16:29

mcsf merged commit 11864ff into master Sep 25, 2017

mcsf deleted the try/post-meta-support branch September 25, 2017 17:25

aduth reviewed Sep 27, 2017

View reviewed changes

This was referenced Sep 27, 2017

Selectors: Fix getBlock error for block not in state #2805

Merged

Bump plugin to version 1.2.0 #2810

Merged

aduth mentioned this pull request Nov 3, 2017

State: Refactor post dirty state as higher-order reducer #3298

Merged

aduth mentioned this pull request Mar 28, 2019

Block API: Allow blocks to specify attributes that map to site options #2759

Closed

gziolo mentioned this pull request Aug 11, 2023

Connecting block attributes and custom fields & Block Bindings API for WP 6.5 #53300

Closed

42 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try supporting post meta in block attributes #2740

Try supporting post meta in block attributes #2740

mcsf commented Sep 16, 2017 •

edited

Loading

codecov bot commented Sep 17, 2017 •

edited

Loading

mtias Sep 20, 2017

pento Sep 21, 2017

rmccue Sep 21, 2017

mtias Sep 21, 2017

mtias Sep 20, 2017

mtias commented Sep 20, 2017

rmccue Sep 21, 2017

aduth commented Sep 27, 2017 •

edited

Loading

aduth Sep 27, 2017

aduth Sep 27, 2017

mcsf Sep 28, 2017 •

edited

Loading

aduth Sep 27, 2017

mcsf Sep 28, 2017

aduth Sep 29, 2017

aduth commented Jan 30, 2018

mtias commented Jan 31, 2018

aduth commented Jan 31, 2018

mcsf commented Jan 31, 2018

mtias commented Feb 12, 2018


		### Considerations

		By default, a meta field will be excluded from a post object's meta. This can be circumvented by explicitly making the field visible:

Try supporting post meta in block attributes #2740

Try supporting post meta in block attributes #2740

Conversation

mcsf commented Sep 16, 2017 • edited Loading

codecov bot commented Sep 17, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtias commented Sep 20, 2017

Choose a reason for hiding this comment

aduth commented Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcsf Sep 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Jan 30, 2018

mtias commented Jan 31, 2018

aduth commented Jan 31, 2018

mcsf commented Jan 31, 2018

mtias commented Feb 12, 2018

mcsf commented Sep 16, 2017 •

edited

Loading

codecov bot commented Sep 17, 2017 •

edited

Loading

aduth commented Sep 27, 2017 •

edited

Loading

mcsf Sep 28, 2017 •

edited

Loading