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.

Sign up for GitHub

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

Jump to bottom

Block bindings: allow the field types matching attribute types in bindings. #66174

Merged
merged 11 commits into from
Oct 18, 2024
+176 −23
Merged

Block bindings: allow the field types matching attribute types in bindings. #66174

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3c8f338
Filter fields by type in post meta
SantosGuillamot Oct 16, 2024
c21e55a
Add e2e test
SantosGuillamot Oct 16, 2024
ece9a68
Allow only strings
cbravobernal Oct 16, 2024
02bcdc4
Update test to expect numbers and integers to be hidden
artemiomorales Oct 17, 2024
6f8397c
Update tests to check for label instead of key
artemiomorales Oct 17, 2024
3ef22f9
Adapt object custom field
SantosGuillamot Oct 17, 2024
48485b1
Adapt e2e tests
SantosGuillamot Oct 17, 2024
98e41bd
Add `type` prop to `getFieldsList`
SantosGuillamot Oct 17, 2024
346faed
Adapt testing source
SantosGuillamot Oct 17, 2024
41d6aba
Reduce diff
SantosGuillamot Oct 17, 2024
1512311
Add `attribute` to dependencies
SantosGuillamot Oct 18, 2024
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
63 changes: 40 additions & 23 deletions packages/block-editor/src/hooks/block-bindings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { __ } from '@wordpress/i18n';
import {
getBlockBindingsSource,
getBlockBindingsSources,
getBlockType,
} from '@wordpress/blocks';
import {
__experimentalItemGroup as ItemGroup,
Expand All @@ -29,6 +30,7 @@ import {
import { unlock } from '../lock-unlock';
import InspectorControls from '../components/inspector-controls';
import BlockContext from '../components/block-context';
import { useBlockEditContext } from '../components/block-edit';
import { useBlockBindingsUtils } from '../utils/block-bindings';
import { store as blockEditorStore } from '../store';

Expand All @@ -50,9 +52,20 @@ const useToolsPanelDropdownMenuProps = () => {
};

function BlockBindingsPanelDropdown( { fieldsList, attribute, binding } ) {
const { clientId } = useBlockEditContext();
const registeredSources = getBlockBindingsSources();
const { updateBlockBindings } = useBlockBindingsUtils();
const currentKey = binding?.args?.key;
const attributeType = useSelect(
( select ) => {
const { name: blockName } =
select( blockEditorStore ).getBlock( clientId );
const _attributeType =
getBlockType( blockName ).attributes?.[ attribute ]?.type;
return _attributeType === 'rich-text' ? 'string' : _attributeType;
Copy link
Member

Choose a reason for hiding this comment

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

We will need a more sophisticated utility in the future. See my notes in https://github.com/WordPress/gutenberg/pull/66174/files#r1804482542. The type can list multiple types, too.

},
[ clientId ]
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
);
return (
<>
{ Object.entries( fieldsList ).map( ( [ name, fields ], i ) => (
Expand All @@ -63,29 +76,33 @@ function BlockBindingsPanelDropdown( { fieldsList, attribute, binding } ) {
{ registeredSources[ name ].label }
</DropdownMenuV2.GroupLabel>
) }
{ Object.entries( fields ).map( ( [ key, args ] ) => (
<DropdownMenuV2.RadioItem
key={ key }
onChange={ () =>
updateBlockBindings( {
[ attribute ]: {
source: name,
args: { key },
},
} )
}
name={ attribute + '-binding' }
value={ key }
checked={ key === currentKey }
>
<DropdownMenuV2.ItemLabel>
{ args?.label }
</DropdownMenuV2.ItemLabel>
<DropdownMenuV2.ItemHelpText>
{ args?.value }
</DropdownMenuV2.ItemHelpText>
</DropdownMenuV2.RadioItem>
) ) }
{ Object.entries( fields )
.filter(
( [ , args ] ) => args?.type === attributeType
)
.map( ( [ key, args ] ) => (
<DropdownMenuV2.RadioItem
key={ key }
onChange={ () =>
updateBlockBindings( {
[ attribute ]: {
source: name,
args: { key },
},
} )
}
name={ attribute + '-binding' }
value={ key }
checked={ key === currentKey }
>
<DropdownMenuV2.ItemLabel>
{ args?.label }
</DropdownMenuV2.ItemLabel>
<DropdownMenuV2.ItemHelpText>
{ args?.value }
</DropdownMenuV2.ItemHelpText>
</DropdownMenuV2.RadioItem>
) ) }
</DropdownMenuV2.Group>
{ i !== Object.keys( fieldsList ).length - 1 && (
<DropdownMenuV2.Separator />
Expand Down
86 changes: 86 additions & 0 deletions packages/e2e-tests/plugins/block-bindings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ function gutenberg_test_block_bindings_registration() {
'text_field' => array(
'label' => 'Text Field Label',
'value' => 'Text Field Value',
'type' => 'string',
),
'url_field' => array(
'label' => 'URL Field Label',
'value' => $testing_url,
'type' => 'string',
),
'empty_field' => array(
'label' => 'Empty Field Label',
'value' => '',
'type' => 'string',
),
);

Expand Down Expand Up @@ -106,6 +109,89 @@ function gutenberg_test_block_bindings_registration() {
'type' => 'string',
)
);
// Register different types of custom fields for testing.
register_meta(
'post',
'string_custom_field',
array(
'label' => 'String custom field',
'default' => '',
'show_in_rest' => true,
'single' => true,
'type' => 'string',
)
);
register_meta(
'post',
'object_custom_field',
array(
'label' => 'Object custom field',
'show_in_rest' => array(
'schema' => array(
'type' => 'object',
'properties' => array(
'foo' => array(
'type' => 'string',
),
),
),
),
'single' => true,
'type' => 'object',
)
);
register_meta(
'post',
'array_custom_field',
array(
'label' => 'Array custom field',
'show_in_rest' => array(
'schema' => array(
'type' => 'array',
'items' => array(
'type' => 'string',
),
),
),
'single' => true,
'type' => 'array',
'default' => array(),
)
);
register_meta(
'post',
'number',
array(
'label' => 'Number custom field',
'type' => 'number',
'show_in_rest' => true,
'single' => true,
'default' => 5.5,
)
);
register_meta(
'post',
'integer',
array(
'label' => 'Integer custom field',
'type' => 'integer',
'show_in_rest' => true,
'single' => true,
'default' => 5,
)
);
register_meta(
'post',
'boolean',
array(
'label' => 'Boolean custom field',
'type' => 'boolean',
'show_in_rest' => true,
'single' => true,
'default' => true,
)
);

// Register CPT custom fields.
register_meta(
'post',
Expand Down
9 changes: 7 additions & 2 deletions packages/editor/src/bindings/post-meta.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,20 @@ function getPostMetaFields( select, context ) {
const registeredFields = getRegisteredPostMeta( context?.postType );
const metaFields = {};
Object.entries( registeredFields || {} ).forEach( ( [ key, props ] ) => {
// Don't include footnotes or private fields.
if ( key !== 'footnotes' && key.charAt( 0 ) !== '_' ) {
if (
// Don't include footnotes.
key !== 'footnotes' &&
// Don't include private fields.
key.charAt( 0 ) !== '_'
) {
metaFields[ key ] = {
label: props.title || key,
value:
// When using the entity value, an empty string IS a valid value.
entityMetaValues?.[ key ] ??
// When using the default, an empty string IS NOT a valid value.
( props.default || undefined ),
type: props.type,
Copy link
Member

@gziolo gziolo Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That looks good.

Source: https://developer.wordpress.org/reference/functions/register_meta/#parameters:

Valid values are 'string', 'boolean', 'integer', 'number', 'array', and 'object'.

Source: https://developer.wordpress.org/block-editor/reference-guides/block-api/block-attributes/#type-validation

The type field MUST be one of the following:

  • null
  • boolean
  • object
  • array
  • string
  • integer
  • number (same as integer)

There is also undocumented rich-text (same as string). The type can also be a mix of several types, example:

"type": [ "string", "boolean" ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for the wrap-up 🙂

I think it could make sense to work on unifying the typing of block attributes and register meta somehow. For example, if in the future a URL type is added, it should be added to both. It is outside of the scope of this PR, but something to consider.

Copy link
Member

@gziolo gziolo Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This nicely ties in with the feedback shared by @afercia at #63018 (comment). In this PR, we will see the first step toward filtering of the options presented for the block attribute based on its type that matches with the Post Meta's type. The next step would be offering more granular type system for all meta fields and block attributes. We will also have to enforce defining the type for custom block binding sources in the future.

};
}
} );
Expand Down
49 changes: 49 additions & 0 deletions test/e2e/specs/editor/various/block-bindings/post-meta.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,5 +547,54 @@ test.describe( 'Post Meta source', () => {
.filter( { hasText: 'Movie field label' } );
await expect( movieField ).toBeVisible();
} );
test( 'should not be possible to connect non-supported fields through the attributes panel', async ( {
editor,
page,
} ) => {
await editor.insertBlock( {
name: 'core/paragraph',
} );
await page.getByLabel( 'Attributes options' ).click();
await page
.getByRole( 'menuitemcheckbox', {
name: 'Show content',
} )
.click();
await page
.getByRole( 'button', {
name: 'content',
} )
.click();
await expect(
page.getByRole( 'menuitemradio', {
name: 'String custom field',
} )
).toBeVisible();
await expect(
page.getByRole( 'menuitemradio', {
name: 'Number custom field',
} )
).toBeHidden();
await expect(
page.getByRole( 'menuitemradio', {
name: 'Integer custom field',
} )
).toBeHidden();
await expect(
page.getByRole( 'menuitemradio', {
name: 'Boolean custom field',
} )
).toBeHidden();
await expect(
page.getByRole( 'menuitemradio', {
name: 'Object custom field',
} )
).toBeHidden();
await expect(
page.getByRole( 'menuitemradio', {
name: 'Array custom field',
} )
).toBeHidden();
} );
} );
} );
Loading