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

Allow contents of Paragraph to be "connected" to a meta custom field #53247

Merged
merged 40 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
77047c3
Add custom fields experimental setting
cbravobernal Aug 1, 2023
ff12248
Add connections experimental setting, block inspector control and blo…
cbravobernal Aug 1, 2023
514a068
Add "connections" block attribute and get the meta field
michalczaplinski Aug 1, 2023
66a9aef
Merge branch 'experiment/add-custom-connectinons-inspector-control' i…
michalczaplinski Aug 1, 2023
5fbf21c
Get rid of a phpcs warning
michalczaplinski Aug 1, 2023
a493bb1
Update attribute acces and source definition
michalczaplinski Aug 2, 2023
7575b40
Updated the 'connections' property to include 'attributes'
michalczaplinski Aug 2, 2023
f8ee25e
Merge branch 'experiment/add-custom-connectinons-inspector-control' i…
michalczaplinski Aug 2, 2023
39cb931
Add `attributes` to TextControl value
michalczaplinski Aug 2, 2023
fa042ca
Add `attributes` to TextControl value
michalczaplinski Aug 2, 2023
64a5f03
Wrap the PHP code in experimental flag
michalczaplinski Aug 2, 2023
d7fccec
Merge branch 'experiment/add-custom-connectinons-inspector-control' i…
michalczaplinski Aug 2, 2023
0b52159
Update parens
michalczaplinski Aug 2, 2023
2014934
Return early if there is no support for connections
michalczaplinski Aug 2, 2023
52b4a58
Updated block type checks
michalczaplinski Aug 2, 2023
cbec419
Updated experimental blocks logic
michalczaplinski Aug 2, 2023
c29d885
Add a comment in meta.php
michalczaplinski Aug 2, 2023
9f0d851
Update comment
cbravobernal Aug 2, 2023
1c65e74
Clear the paragraph if meta value is cleared
cbravobernal Aug 2, 2023
8638dff
Use placeholders instead of content
cbravobernal Aug 2, 2023
88dd0b5
Update names of HTML processor instances in meta.php
michalczaplinski Aug 3, 2023
81f9c67
Add extra comments
michalczaplinski Aug 3, 2023
60fa061
Merge branch 'experiment/add-custom-connectinons-inspector-control' i…
michalczaplinski Aug 3, 2023
713c6df
Rephrase the placeholder comment
michalczaplinski Aug 3, 2023
e80effe
Check if current block is a paragraph or image
michalczaplinski Aug 3, 2023
7640a60
Abstract the attribute name to use for the connection
michalczaplinski Aug 3, 2023
f0bcb0f
rename `connections` to `__experimentalConnections`
michalczaplinski Aug 7, 2023
6a2785f
Do not export `addAttribute()` or `withInspectorControl()`
michalczaplinski Aug 7, 2023
4408e69
Merge branch 'experiment/add-custom-connectinons-inspector-control' i…
michalczaplinski Aug 7, 2023
5cd3d22
Renamed '__experimentalConnections' to 'connections' in block settings
michalczaplinski Aug 7, 2023
6d00a9b
Renamed '__experimentalConnections' to 'connections' in block settings
michalczaplinski Aug 7, 2023
367ee0c
Remove `connections` attribute from block.json
michalczaplinski Aug 7, 2023
a45e464
Renamed '__experimentalConnections' to 'connections' in block settings
michalczaplinski Aug 7, 2023
84f2a08
Remove `get_updated_html() . ''` in meta.php
michalczaplinski Aug 7, 2023
49d5cb6
Add an allowlist of blocks/attributes
michalczaplinski Aug 7, 2023
414b322
Refactor blocks.php & custom sources
michalczaplinski Aug 7, 2023
92c3cd9
Remove trailing comma?
michalczaplinski Aug 8, 2023
ba713a4
Update naming:
michalczaplinski Aug 9, 2023
a5b9e3b
Merge branch 'trunk' into experiment/add-custom-connectinons-inspecto…
michalczaplinski Aug 9, 2023
a0bd2ca
Merge branch 'experiment/add-custom-connectinons-inspector-control' i…
michalczaplinski Aug 9, 2023
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
4 changes: 2 additions & 2 deletions docs/reference-guides/core-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,8 @@ Start with the basic building block of all narrative. ([Source](https://github.c

- **Name:** core/paragraph
- **Category:** text
- **Supports:** __unstablePasteTextInline, anchor, color (background, gradients, link, text), spacing (margin, padding), typography (fontSize, lineHeight), ~~className~~
- **Attributes:** align, content, direction, dropCap, placeholder
- **Supports:** __unstablePasteTextInline, anchor, color (background, gradients, link, text), connections, spacing (margin, padding), typography (fontSize, lineHeight), ~~className~~
- **Attributes:** align, connections, content, direction, dropCap, placeholder

## Pattern placeholder

Expand Down
64 changes: 64 additions & 0 deletions lib/experimental/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,67 @@ 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_custom_sources( $block_content, $block, $block_instance ) {
$meta_custom_source = require __DIR__ . '/custom-sources/meta.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused about the "custom sources" naming. Is this what we are doing? Are we rendering custom sources? If I understood it correctly, we are:

  • Processing the connections block attribute.
  • Getting the value from the connections source. Meta in this case using get_post_meta.
  • Replacing the HTML according to the block attributes source. html in the case of the paragraph.

In my opinion, this last step should be independent of the connections source. If I get the value from a different source, I would still have to replace the same HTML, but with a different value. Is this right?

Just to have more context, I believe in the future this distinction will be more relevant:

  • Connections sources: From where we get the value. Right now it is only meta, but it could be drupal, parent-block, my-custom-database...
  • Block attributes source: Dictates which part of the HTML needs to be replaced. Thanks to the block attributes source, it's possible to know the HTML we have to replace. If it is html we will replace the inner content. If it is attribute we will replace an HTML attribute.

That's why it makes sense to separate the code as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Processing the connections block attribute.
  • Getting the value from the connections source. Meta in this case using get_post_meta.
  • Replacing the HTML according to the block attributes source. html in the case of the paragraph.

You're right, that's the right abstraction of the 3 steps that we perform. As it stands right now, the 2nd & 3rd step are not separated. It makes sense that they should be.

Also, some of the naming is a direct copy from Riad's PR (#51375) which is not 100% accurate anymore.

I ll change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code should be better abstracted now to reflect those 3 separate steps. 1 & 3 are both inside of the same function but we can abstract it further when we add support for the Image block & other attributes. Then we can abstract the part that replaces the different attribute sources (html, attribute, etc.).

We should discuss consistent naming though of Step 2. Should it be:

  • custom sources
  • custom connections
  • connection sources
  • custom connection sources
  • ...something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I believe the whole naming should be discussed. Another option I was thinking of was using "Dynamic data" instead of "Connections". So it could be "Dynamic data sources". But I didn't think enough about it. What doesn't quite fit me is the use of "custom". What does "custom" mean in this context? Which sources are not custom?

Do we need it for this initial experiment? Is it too difficult to change it afterwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it too difficult to change it afterwards?
Nope, it won't be difficult at all, I just wanted to start this conversation and have consistent naming even if it gets changed later 🙂 .

I guess "custom" means something like "different than whatever WP uses to get the content normally" but I agree that it's a bit meaningless.

I like "Connections" a little bit better than "Dynamic Data" because the latter can mean many things in many different contexts. Most data is dynamic rather than static in programming 🤓. I think "Connection Sources" sounds best so far. "Connections" alone is also a bit vague. "Connection Sources" implies "data sources" which is roughly what we are referring to here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe "block connections" is also a good name? We're connecting stuff to blocks after all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I was thinking about names using this sentence: we are connecting block attributes to values that might change depending on the context (post_id, parent block…). But not sure yet which is the best abstraction 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I would just go with "block connections" as a name for the whole thing (like I use it in https://github.com/WordPress/gutenberg/pull/53247/files#diff-be8a354ce9c01e85c0fdf04c75d76880cdc6c81992db7a86a7a0515dc11a9833R135) and "connection sources" for the thing that does the connecting to meta fields/parent blocks/other DBs, etc.

We can always change it in the future.

If you don't object, then I ll go ahead and change it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure! We can always change the naming later if we agree on a different one.

$block_type = $block_instance->block_type;

// Whitelist of the block types that support custom sources
// Currently, we only allow the Paragraph and Image blocks to use custom sources.
if ( ! in_array( $block['blockName'], array( 'core/paragraph', 'core/image' ), true ) ) {
return $block_content;
}
michalczaplinski marked this conversation as resolved.
Show resolved Hide resolved

if ( null === $block_type ) {
return $block_content;
}

if ( ! block_has_support( $block_type, array( 'connections' ), 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 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;
}

$block_content = $meta_custom_source['apply_source'](
$block_content,
$block_instance,
$attribute_value['value'],
$block_type->attributes[ $attribute_name ]
);
}

return $block_content;
}
add_filter( 'render_block', 'gutenberg_render_custom_sources', 10, 3 );
}
36 changes: 36 additions & 0 deletions lib/experimental/custom-sources/meta.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
/**
* Meta Custom Source
*
* @package gutenberg
*/

return array(
'name' => 'meta',
'apply_source' => function ( $block_content, $block_instance, $meta_field, $attribute_config ) {
// 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.
$meta_value = get_post_meta( $block_instance->context['postId'], $meta_field, true );
$p = new WP_HTML_Tag_Processor( $block_content );
michalczaplinski marked this conversation as resolved.
Show resolved Hide resolved
$found = $p->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' => $attribute_config['selector'],
)
);
if ( ! $found ) {
return $block_content;
}
$tag_name = $p->get_tag();
$markup = "<$tag_name>$meta_value</$tag_name>";
$p2 = new WP_HTML_Tag_Processor( $markup );
$p2->next_tag();
$names = $p->get_attribute_names_with_prefix( '' );
foreach ( $names as $name ) {
$p2->set_attribute( $name, $p->get_attribute( $name ) );
}

return $p2 . '';
michalczaplinski marked this conversation as resolved.
Show resolved Hide resolved
},
);
4 changes: 4 additions & 0 deletions lib/experimental/editor-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ function gutenberg_enable_experiments() {
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalEnableGroupGridVariation = true', 'before' );
}

if ( $gutenberg_experiments && array_key_exists( 'gutenberg-connections', $gutenberg_experiments ) ) {
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalConnections = true', 'before' );
}

if ( gutenberg_is_experiment_enabled( 'gutenberg-no-tinymce' ) ) {
wp_add_inline_script( 'wp-block-library', 'window.__experimentalDisableTinymce = true', 'before' );
}
Expand Down
12 changes: 12 additions & 0 deletions lib/experiments-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ function gutenberg_initialize_experiments_settings() {
)
);

add_settings_field(
'gutenberg-custom-fields',
__( 'Connections', 'gutenberg' ),
'gutenberg_display_experiment_field',
'gutenberg-experiments',
'gutenberg_experiments_section',
array(
'label' => __( 'Test Connections', 'gutenberg' ),
'id' => 'gutenberg-connections',
)
);

register_setting(
'gutenberg-experiments',
'gutenberg-experiments'
Expand Down
136 changes: 136 additions & 0 deletions packages/block-editor/src/hooks/custom-fields.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/**
* 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';
import { useRef } from '@wordpress/element';

/**
* 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.
*/
export function addAttribute( settings ) {
if ( hasBlockSupport( settings, 'connections', true ) ) {
// Gracefully handle if settings.attributes 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.
*/
export const withInspectorControl = createHigherOrderComponent(
( BlockEdit ) => {
return ( props ) => {
const blockEditingMode = useBlockEditingMode();
const hasCustomFieldsSupport = hasBlockSupport(
props.name,
'connections',
false
);
// We prevent that the content is lost when the user removes the custom field.
// Editing the content in the paragraph block with a placeholder is not the best solution.
const prevContent = useRef( props.attributes?.content );
if ( ! prevContent.current ) {
prevContent.current = '';
}
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?.content?.value ||
''
}
onChange={ ( nextValue ) => {
if ( nextValue === '' ) {
props.setAttributes( {
connections: undefined,
content:
prevContent.current !==
''
? prevContent.current
: undefined,
} );
} else {
props.setAttributes( {
connections: {
attributes: {
// Content will be variable, could be content, href, src, etc.
content: {
// 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,
},
},
},
content: sprintf(
'This content will be replaced in the frontend by the custom field "%s" value.',
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
);
}
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import './content-lock-ui';
import './metadata';
import './metadata-name';
import './behaviors';
import './custom-fields';

export { useCustomSides } from './dimensions';
export { useLayoutClasses, useLayoutStyles } from './layout';
Expand Down
10 changes: 10 additions & 0 deletions packages/block-library/src/paragraph/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"description": "Start with the basic building block of all narrative.",
"keywords": [ "text" ],
"textdomain": "default",
"usesContext": [ "postId" ],
Copy link
Contributor

@youknowriad youknowriad Aug 10, 2023

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

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

both

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member

@gziolo gziolo Aug 16, 2023

Choose a reason for hiding this comment

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

For now, you could update usesContext and pass postId in PHP when processing gutenberg_render_block_connections. This way, it won't get unnecessarily wired for all Paragraph block instances through block.json definition.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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"
Expand All @@ -28,6 +29,14 @@
"direction": {
"type": "string",
"enum": [ "ltr", "rtl" ]
},
"connections": {
"type": "object",
"attributes": {
"content": {
"source": "meta_fields"
}
}
michalczaplinski marked this conversation as resolved.
Show resolved Hide resolved
}
},
"supports": {
Expand All @@ -41,6 +50,7 @@
"text": true
}
},
"connections": true,
"spacing": {
"margin": true,
"padding": true,
Expand Down