-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Allow contents of Paragraph to be "connected" to a meta custom field #53247
Changes from 36 commits
77047c3
ff12248
514a068
66a9aef
5fbf21c
a493bb1
7575b40
f8ee25e
39cb931
fa042ca
64a5f03
d7fccec
0b52159
2014934
52b4a58
cbec419
c29d885
9f0d851
1c65e74
8638dff
88dd0b5
81f9c67
60fa061
713c6df
e80effe
7640a60
f0bcb0f
6a2785f
4408e69
5cd3d22
6d00a9b
367ee0c
a45e464
84f2a08
49d5cb6
414b322
92c3cd9
ba713a4
a5b9e3b
a0bd2ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -121,3 +121,105 @@ function gutenberg_register_metadata_attribute( $args ) { | |||
return $args; | ||||
} | ||||
add_filter( 'register_block_type_args', 'gutenberg_register_metadata_attribute' ); | ||||
|
||||
|
||||
$gutenberg_experiments = get_option( 'gutenberg-experiments' ); | ||||
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-connections', $gutenberg_experiments ) ) { | ||||
/** | ||||
* Renders the block meta attributes. | ||||
* | ||||
* @param string $block_content Block Content. | ||||
* @param array $block Block attributes. | ||||
* @param WP_Block $block_instance The block instance. | ||||
*/ | ||||
function gutenberg_render_block_connections( $block_content, $block, $block_instance ) { | ||||
$connection_sources = require __DIR__ . '/connection-sources/index.php'; | ||||
$block_type = $block_instance->block_type; | ||||
|
||||
// Allowlist of blocks that support custom connections | ||||
// Currently, we only allow the following blocks and attributes: | ||||
// - Paragraph: content. | ||||
// - Image: url. | ||||
$blocks_attributes_allowlist = array( | ||||
'core/paragraph' => array( 'content' ), | ||||
'core/image' => array( 'url' ), | ||||
); | ||||
|
||||
// Whitelist of the block types that support custom connection sources | ||||
// Currently, we only allow the Paragraph and Image blocks to use custom sources. | ||||
if ( ! in_array( $block['blockName'], array_keys( $blocks_attributes_allowlist ), true ) ) { | ||||
return $block_content; | ||||
} | ||||
|
||||
// If for some reason, the block type is not found, skip it. | ||||
if ( null === $block_type ) { | ||||
return $block_content; | ||||
} | ||||
|
||||
// If the block does not have support for connections, skip it. | ||||
if ( ! block_has_support( $block_type, array( '__experimentalConnections' ), false ) ) { | ||||
return $block_content; | ||||
} | ||||
|
||||
// Get all the attributes that have a connection. | ||||
$connected_attributes = _wp_array_get( $block['attrs'], array( 'connections', 'attributes' ), false ); | ||||
if ( ! $connected_attributes ) { | ||||
return $block_content; | ||||
} | ||||
|
||||
foreach ( $connected_attributes as $attribute_name => $attribute_value ) { | ||||
|
||||
// If the attribute is not in the allowlist, skip it. | ||||
if ( ! in_array( $attribute_name, $blocks_attributes_allowlist[ $block['blockName'] ], true ) ) { | ||||
continue; | ||||
} | ||||
|
||||
// If the source value is not meta, skip it because we only support meta | ||||
// sources for now. | ||||
if ( 'meta_fields' !== $attribute_value['source'] ) { | ||||
continue; | ||||
} | ||||
|
||||
// If the attribute does not have a source, skip it. | ||||
if ( ! isset( $block_type->attributes[ $attribute_name ]['source'] ) ) { | ||||
continue; | ||||
} | ||||
|
||||
// If the attribute does not specify the name of the custom field, skip it. | ||||
if ( ! isset( $attribute_value['value'] ) ) { | ||||
continue; | ||||
} | ||||
|
||||
// Get the content from the connection. | ||||
$custom_value = $connection_sources[ $attribute_value['source'] ]( | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is weird, I'd have preferred a unique name for all connection, like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, maybe the confusion is coming from this line:
I should have removed the So in your PR, there was an assumption that there's one file per connection source ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah ok, I guess depending on the complexity we could split but agree, seems like implementation detail for now but let's aim for the simplest. I do expect us to allow registering sources at some point so I think "one config array per source" is probably better ultimately (as opposed to all configs in a single array). |
||||
$block_instance, | ||||
$attribute_value['value'] | ||||
); | ||||
|
||||
$tags = new WP_HTML_Tag_Processor( $block_content ); | ||||
$found = $tags->next_tag( | ||||
array( | ||||
// TODO: In the future, when blocks other than Paragraph and Image are | ||||
// supported, we should build the full query from CSS selector. | ||||
'tag_name' => $block_type->attributes[ $attribute_name ]['selector'], | ||||
) | ||||
); | ||||
if ( ! $found ) { | ||||
return $block_content; | ||||
}; | ||||
$tag_name = $tags->get_tag(); | ||||
$markup = "<$tag_name>$custom_value</$tag_name>"; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is scary. Any plans for escaping? It's not trivial, since the goal is to be able to inject rich text and not just simple values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for catching this @mcsf. this definitely should be actually @michalczaplinski let's talk about how to sub-class the Tag Processor so this is cleaner all over There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for raising it! We should definitely do that. Keep in mind that this code was considered experimental and we'll need to refactor it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely, but all too often lines of experimental code somehow make their way into production code. :) I'd really patch ASAP, even if just with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching this! I want to take a holistic look at all the code shipped behind the experimental flag. It's very likely most of it will be replaced altogether. The team is on support rotation this week so it'll have to wait till next week at least 🙂 @dmsnell yes, let's talk! |
||||
$updated_tags = new WP_HTML_Tag_Processor( $markup ); | ||||
$updated_tags->next_tag(); | ||||
$names = $tags->get_attribute_names_with_prefix( '' ); | ||||
foreach ( $names as $name ) { | ||||
$updated_tags->set_attribute( $name, $tags->get_attribute( $name ) ); | ||||
} | ||||
|
||||
return $updated_tags->get_updated_html(); | ||||
} | ||||
|
||||
return $block_content; | ||||
} | ||||
add_filter( 'render_block', 'gutenberg_render_block_connections', 10, 3 ); | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
/** | ||
* Custom sources that block attributes can be connected to. | ||
* | ||
* @package gutenberg | ||
*/ | ||
|
||
return array( | ||
'name' => 'meta', | ||
'meta_fields' => function ( $block_instance, $meta_field ) { | ||
// We should probably also check if the meta field exists but for now it's okay because | ||
// if it doesn't, `get_post_meta()` will just return an empty string. | ||
return get_post_meta( $block_instance->context['postId'], $meta_field, true ); | ||
}, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { addFilter } from '@wordpress/hooks'; | ||
import { PanelBody, TextControl } from '@wordpress/components'; | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
import { hasBlockSupport } from '@wordpress/blocks'; | ||
import { createHigherOrderComponent } from '@wordpress/compose'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { InspectorControls } from '../components'; | ||
import { useBlockEditingMode } from '../components/block-editing-mode'; | ||
|
||
/** | ||
* Filters registered block settings, extending attributes to include `connections`. | ||
* | ||
* @param {Object} settings Original block settings. | ||
* | ||
* @return {Object} Filtered block settings. | ||
*/ | ||
function addAttribute( settings ) { | ||
if ( hasBlockSupport( settings, '__experimentalConnections', true ) ) { | ||
// Gracefully handle if settings.attributes.connections is undefined. | ||
settings.attributes = { | ||
...settings.attributes, | ||
connections: { | ||
type: 'object', | ||
}, | ||
}; | ||
} | ||
|
||
return settings; | ||
} | ||
|
||
/** | ||
* Override the default edit UI to include a new block inspector control for | ||
* assigning a connection to blocks that has support for connections. | ||
* Currently, only the `core/paragraph` block is supported and there is only a relation | ||
* between paragraph content and a custom field. | ||
* | ||
* @param {WPComponent} BlockEdit Original component. | ||
* | ||
* @return {WPComponent} Wrapped component. | ||
*/ | ||
const withInspectorControl = createHigherOrderComponent( ( BlockEdit ) => { | ||
return ( props ) => { | ||
const blockEditingMode = useBlockEditingMode(); | ||
const hasCustomFieldsSupport = hasBlockSupport( | ||
props.name, | ||
'__experimentalConnections', | ||
false | ||
); | ||
|
||
// Check if the current block is a paragraph or image block. | ||
// Currently, only these two blocks are supported. | ||
if ( ! [ 'core/paragraph', 'core/image' ].includes( props.name ) ) { | ||
return <BlockEdit { ...props } />; | ||
} | ||
|
||
// If the block is a paragraph or image block, we need to know which | ||
// attribute to use for the connection. Only the `content` attribute | ||
// of the paragraph block and the `url` attribute of the image block are supported. | ||
let attributeName; | ||
if ( props.name === 'core/paragraph' ) attributeName = 'content'; | ||
if ( props.name === 'core/image' ) attributeName = 'url'; | ||
|
||
if ( hasCustomFieldsSupport && props.isSelected ) { | ||
return ( | ||
<> | ||
<BlockEdit { ...props } /> | ||
{ blockEditingMode === 'default' && ( | ||
<InspectorControls> | ||
<PanelBody | ||
title={ __( 'Connections' ) } | ||
initialOpen={ true } | ||
> | ||
<TextControl | ||
__nextHasNoMarginBottom | ||
autoComplete="off" | ||
label={ __( 'Custom field meta_key' ) } | ||
value={ | ||
props.attributes?.connections | ||
?.attributes?.[ attributeName ] | ||
?.value || '' | ||
} | ||
onChange={ ( nextValue ) => { | ||
if ( nextValue === '' ) { | ||
props.setAttributes( { | ||
connections: undefined, | ||
[ attributeName ]: undefined, | ||
placeholder: undefined, | ||
} ); | ||
} else { | ||
props.setAttributes( { | ||
connections: { | ||
attributes: { | ||
// The attributeName will be either `content` or `url`. | ||
[ attributeName ]: { | ||
// Source will be variable, could be post_meta, user_meta, term_meta, etc. | ||
// Could even be a custom source like a social media attribute. | ||
source: 'meta_fields', | ||
value: nextValue, | ||
}, | ||
}, | ||
}, | ||
[ attributeName ]: undefined, | ||
placeholder: sprintf( | ||
'This content will be replaced on the frontend by the value of "%s" custom field.', | ||
nextValue | ||
), | ||
} ); | ||
} | ||
} } | ||
/> | ||
</PanelBody> | ||
</InspectorControls> | ||
) } | ||
</> | ||
); | ||
} | ||
|
||
return <BlockEdit { ...props } />; | ||
}; | ||
}, 'withInspectorControl' ); | ||
|
||
if ( window.__experimentalConnections ) { | ||
addFilter( | ||
'blocks.registerBlockType', | ||
'core/connections/attribute', | ||
addAttribute | ||
); | ||
addFilter( | ||
'editor.BlockEdit', | ||
'core/connections/with-inspector-control', | ||
withInspectorControl | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
"description": "Start with the basic building block of all narrative.", | ||
"keywords": [ "text" ], | ||
"textdomain": "default", | ||
"usesContext": [ "postId" ], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder whose responsibility is it to do that. In other words, the block is not using the "postId" explicitly, so why add the "useContext" here. The "connection framework" should inject/filter this somehow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean that in the context of Core blocks or custom blocks or both? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK! For now, I think that for core blocks this is fine. But you're right that for custom blocks we'll have to come up with a proper mechanism here. Are you all right with that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think it's part of the definition of a "custom source" to say which change it needs to make to block registration. I think we should use it for both core and third-party blocks but yeah it's fine to start without it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, you could update There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even though this is the first implementation and custom blocks are not supported yet, I believe it'd be better to start creating mechanisms that would work for both if possible. Mainly because we already know we will want to support that in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coming in a bit late here. The current work on partial syncing (#54233) is using the store actions/selectors for attributes to inject/update values, but it might be difficult to pull this off if block context is being used for custom fields, as I believe it uses react context in JS. While they're still proofs of concepts, the current work for custom fields/partial syncing does need to remain compatible, so it might be good to explore ways to read/write custom field values in the editor soon though so that we have a clear idea of the problems that need to be solved. |
||
"attributes": { | ||
"align": { | ||
"type": "string" | ||
|
@@ -41,6 +42,7 @@ | |
"text": true | ||
} | ||
}, | ||
"__experimentalConnections": true, | ||
"spacing": { | ||
"margin": true, | ||
"padding": true, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we retrieve this from a block support instead of a hardcoded list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it's the user that should define which attribute to connect, so this is coming from the block attribute value normally no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment in #53241 (comment) :)
Yes, but for now we want to limit this to core blocks and those two attributes. We'll expand it in the future.
So far it's not well-defined how users are going to opt-in to connecting their custom blocks' attributes to custom fields.