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: Use default values in connected custom fields in templates #65128

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Block Bindings: Use default values in connected custom fields in templates #65128

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f012a0b
Abstract `getMetadata` and use it in `getValues`
SantosGuillamot Sep 6, 2024
e2fddae
Adapt e2e tests
SantosGuillamot Sep 6, 2024
fac61a1
Update e2e
cbravobernal Sep 10, 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
2 changes: 1 addition & 1 deletion packages/e2e-tests/plugins/block-bindings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function gutenberg_test_block_bindings_registration() {
'show_in_rest' => true,
'type' => 'string',
'single' => true,
'default' => 'Value of the text_custom_field',
'default' => 'Value of the text custom field',
)
);
register_meta(
Expand Down
126 changes: 63 additions & 63 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 @@ -9,21 +9,75 @@ import { store as coreDataStore } from '@wordpress/core-data';
import { store as editorStore } from '../store';
import { unlock } from '../lock-unlock';

function getMetadata( registry, context ) {
let metaFields = {};
const {
type,
is_custom: isCustom,
slug,
} = registry.select( editorStore ).getCurrentPost();
const { getPostTypes, getEditedEntityRecord } =
registry.select( coreDataStore );

const { getRegisteredPostMeta } = unlock(
registry.select( coreDataStore )
);

// Inherit the postType from the slug if it is a template.
if ( ! context?.postType && type === 'wp_template' ) {
// Get the 'kind' from the start of the slug.
// Use 'post' as the default.
let postType = 'post';
const isGlobalTemplate = isCustom || slug === 'index';
if ( ! isGlobalTemplate ) {
const [ kind ] = slug.split( '-' );
if ( kind === 'page' ) {
postType = 'page';
} else if ( kind === 'single' ) {
const postTypes =
getPostTypes( { per_page: -1 } )?.map(
( entity ) => entity.slug
) || [];

// Infer the post type from the slug.
// TODO: Review, as it may not have a post type. http://localhost:8888/wp-admin/site-editor.php?canvas=edit
const match = slug.match(
`^single-(${ postTypes.join( '|' ) })(?:-.+)?$`
);
postType = match ? match[ 1 ] : 'post';
}
}
const fields = getRegisteredPostMeta( postType );

// Populate the `metaFields` object with the default values.
Object.entries( fields || {} ).forEach( ( [ key, props ] ) => {
// If the template is global, skip the fields with a subtype.
if ( isGlobalTemplate && props.object_subtype ) {
return;
}
metaFields[ key ] = props.default;
Copy link
Member

Choose a reason for hiding this comment

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

What will happen when the default is undefined or empty?

Copy link
Contributor

@cbravobernal cbravobernal Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It says:

Apart from that, when no default is provided, it should still show the meta key.

I'll test it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Registering a field without default value:
Screenshot 2024-09-09 at 22 30 37

is returning the meta key:
Screenshot 2024-09-09 at 22 30 25

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that is returning an empty string, as documented here:
https://developer.wordpress.org/reference/functions/get_metadata_default/#description

Screenshot 2024-09-09 at 22 44 25

Copy link
Contributor

Choose a reason for hiding this comment

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

If single is false in register_meta() args. get_meta_data_default() should return an empty array.

Screenshot 2024-09-09 at 22 56 21

When testing, it returned undefined. But still is showing just the key. I should review if we are covering all the cases with tests.

Screenshot 2024-09-09 at 22 55 45

} );
} else {
metaFields = getEditedEntityRecord(
'postType',
context?.postType,
context?.postId
).meta;
}

return metaFields;
}

export default {
name: 'core/post-meta',
getValues( { registry, context, bindings } ) {
const meta = registry
.select( coreDataStore )
.getEditedEntityRecord(
'postType',
context?.postType,
context?.postId
)?.meta;
const metaFields = getMetadata( registry, context );

const newValues = {};
for ( const [ attributeName, source ] of Object.entries( bindings ) ) {
// Use the key if the value is not set.
newValues[ attributeName ] =
meta?.[ source.args.key ] || source.args.key;
metaFields?.[ source.args.key ] || source.args.key;
}
return newValues;
},
Expand Down Expand Up @@ -83,61 +137,7 @@ export default {
return true;
},
getFieldsList( { registry, context } ) {
let metaFields = {};
const {
type,
is_custom: isCustom,
slug,
} = registry.select( editorStore ).getCurrentPost();
const { getPostTypes, getEditedEntityRecord } =
registry.select( coreDataStore );

const { getRegisteredPostMeta } = unlock(
registry.select( coreDataStore )
);

// Inherit the postType from the slug if it is a template.
if ( ! context?.postType && type === 'wp_template' ) {
// Get the 'kind' from the start of the slug.
// Use 'post' as the default.
let postType = 'post';
// A global template can be used with any post type.
const isGlobalTemplate = isCustom || slug === 'index';
if ( ! isGlobalTemplate ) {
const [ kind ] = slug.split( '-' );
if ( kind === 'page' ) {
postType = 'page';
} else if ( kind === 'single' ) {
const postTypes =
getPostTypes( { per_page: -1 } )?.map(
( entity ) => entity.slug
) || [];

// Infer the post type from the slug.
// TODO: Review, as it may not have a post type. http://localhost:8888/wp-admin/site-editor.php?canvas=edit
const match = slug.match(
`^single-(${ postTypes.join( '|' ) })(?:-.+)?$`
);
postType = match ? match[ 1 ] : 'post';
}
}
const fields = getRegisteredPostMeta( postType );

// Populate the `metaFields` object with the default values.
Object.entries( fields || {} ).forEach( ( [ key, props ] ) => {
// If the template is global, skip the fields with a subtype.
if ( isGlobalTemplate && props.object_subtype ) {
return;
}
metaFields[ key ] = props.default;
} );
} else {
metaFields = getEditedEntityRecord(
'postType',
context?.postType,
context?.postId
).meta;
}
const metaFields = getMetadata( registry, context );

if ( ! metaFields || ! Object.keys( metaFields ).length ) {
return null;
Expand Down
40 changes: 20 additions & 20 deletions test/e2e/specs/editor/various/block-bindings.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
name: 'Block: Paragraph',
} );
await expect( paragraphBlock ).toHaveText(
'text_custom_field'
'Value of the text custom field'
);
} );

Expand Down Expand Up @@ -922,7 +922,7 @@
.getByRole( 'tabpanel', { name: 'Settings' } )
.getByLabel( 'Alternative text' )
.inputValue();
expect( altValue ).toBe( 'text_custom_field' );
expect( altValue ).toBe( 'Value of the text custom field' );

// Title input is enabled and with the original value.
await page
Expand Down Expand Up @@ -1064,7 +1064,7 @@
.getByRole( 'tabpanel', { name: 'Settings' } )
.getByLabel( 'Title attribute' )
.inputValue();
expect( titleValue ).toBe( 'text_custom_field' );
expect( titleValue ).toBe( 'Value of the text custom field' );
} );

test( 'should disable title input when title is bound to an undefined source', async ( {
Expand Down Expand Up @@ -1183,7 +1183,7 @@
.getByRole( 'tabpanel', { name: 'Settings' } )
.getByLabel( 'Alternative text' )
.inputValue();
expect( altValue ).toBe( 'text_custom_field' );

Check failure on line 1186 in test/e2e/specs/editor/various/block-bindings.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 3

[chromium] › editor/various/block-bindings.spec.js:1133:4 › Block bindings › Template context › Image › Multiple bindings should lock the appropriate controls 

  1) [chromium] › editor/various/block-bindings.spec.js:1133:4 › Block bindings › Template context › Image › Multiple bindings should lock the appropriate controls 
    Error: expect(received).toBe(expected) // Object.is equality

    Expected: "text_custom_field"
    Received: "Value of the text custom field"

      1184 | 					.getByLabel( 'Alternative text' )
      1185 | 					.inputValue();
    > 1186 | 				expect( altValue ).toBe( 'text_custom_field' );
           | 				                   ^
      1187 |
      1188 | 				// Title input is enabled and with the original value.
      1189 | 				await page

        at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/block-bindings.spec.js:1186:24

Check failure on line 1186 in test/e2e/specs/editor/various/block-bindings.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 3

[chromium] › editor/various/block-bindings.spec.js:1133:4 › Block bindings › Template context › Image › Multiple bindings should lock the appropriate controls 

  1) [chromium] › editor/various/block-bindings.spec.js:1133:4 › Block bindings › Template context › Image › Multiple bindings should lock the appropriate controls 

    Retry #1 ───────────────────────────────────────────────────────────────────────────────────────
    Error: expect(received).toBe(expected) // Object.is equality

    Expected: "text_custom_field"
    Received: "Value of the text custom field"

      1184 | 					.getByLabel( 'Alternative text' )
      1185 | 					.inputValue();
    > 1186 | 				expect( altValue ).toBe( 'text_custom_field' );
           | 				                   ^
      1187 |
      1188 | 				// Title input is enabled and with the original value.
      1189 | 				await page

        at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/block-bindings.spec.js:1186:24

Check failure on line 1186 in test/e2e/specs/editor/various/block-bindings.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 3

[chromium] › editor/various/block-bindings.spec.js:1133:4 › Block bindings › Template context › Image › Multiple bindings should lock the appropriate controls 

  1) [chromium] › editor/various/block-bindings.spec.js:1133:4 › Block bindings › Template context › Image › Multiple bindings should lock the appropriate controls 

    Retry #2 ───────────────────────────────────────────────────────────────────────────────────────
    Error: expect(received).toBe(expected) // Object.is equality

    Expected: "text_custom_field"
    Received: "Value of the text custom field"

      1184 | 					.getByLabel( 'Alternative text' )
      1185 | 					.inputValue();
    > 1186 | 				expect( altValue ).toBe( 'text_custom_field' );
           | 				                   ^
      1187 |
      1188 | 				// Title input is enabled and with the original value.
      1189 | 				await page

        at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/block-bindings.spec.js:1186:24

// Title input is enabled and with the original value.
await page
Expand Down Expand Up @@ -1231,14 +1231,14 @@
name: 'Block: Paragraph',
} );
await expect( paragraphBlock ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);

// Check the frontend shows the value of the custom field.
const previewPage = await editor.openPreviewPage();
await expect(
previewPage.locator( '#paragraph-binding' )
).toHaveText( 'Value of the text_custom_field' );
).toHaveText( 'Value of the text custom field' );
} );

test( "should show the value of the key when custom field doesn't exist", async ( {
Expand Down Expand Up @@ -1368,7 +1368,7 @@
.locator( '[data-type="core/paragraph"]' )
.all();
await expect( initialParagraph ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);
await expect( newEmptyParagraph ).toHaveText( '' );
await expect( newEmptyParagraph ).toBeEditable();
Expand Down Expand Up @@ -1478,7 +1478,7 @@
name: 'Block: Paragraph',
} );
await expect( paragraphBlock ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);
} );
} );
Expand Down Expand Up @@ -1506,14 +1506,14 @@
name: 'Block: Heading',
} );
await expect( headingBlock ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);

// Check the frontend shows the value of the custom field.
const previewPage = await editor.openPreviewPage();
await expect(
previewPage.locator( '#heading-binding' )
).toHaveText( 'Value of the text_custom_field' );
).toHaveText( 'Value of the text custom field' );
} );

test( 'should add empty paragraph block when pressing enter', async ( {
Expand Down Expand Up @@ -1552,7 +1552,7 @@
'core/heading'
);
await expect( initialHeading ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);
// Second block should be an empty paragraph block.
await expect( newEmptyParagraph ).toHaveAttribute(
Expand Down Expand Up @@ -1610,7 +1610,7 @@
.getByRole( 'textbox' );
await buttonBlock.click();
await expect( buttonBlock ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);

// Check the frontend shows the value of the custom field.
Expand All @@ -1619,7 +1619,7 @@
'#button-text-binding a'
);
await expect( buttonDom ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);
await expect( buttonDom ).toHaveAttribute(
'href',
Expand Down Expand Up @@ -1699,7 +1699,7 @@
'#button-multiple-bindings a'
);
await expect( buttonDom ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);
await expect( buttonDom ).toHaveAttribute(
'href',
Expand Down Expand Up @@ -1746,7 +1746,7 @@
.all();
// First block should be the original block.
await expect( initialButton ).toHaveText(
'Value of the text_custom_field'
'Value of the text custom field'
);
// Second block should be an empty paragraph block.
await expect( newEmptyButton ).toHaveText( '' );
Expand Down Expand Up @@ -1911,7 +1911,7 @@
.getByRole( 'tabpanel', { name: 'Settings' } )
.getByLabel( 'Alternative text' )
.inputValue();
expect( altValue ).toBe( 'Value of the text_custom_field' );
expect( altValue ).toBe( 'Value of the text custom field' );

// Check the frontend uses the value of the custom field.
const previewPage = await editor.openPreviewPage();
Expand All @@ -1924,7 +1924,7 @@
);
await expect( imageDom ).toHaveAttribute(
'alt',
'Value of the text_custom_field'
'Value of the text custom field'
);
await expect( imageDom ).toHaveAttribute(
'title',
Expand Down Expand Up @@ -1981,7 +1981,7 @@
.getByRole( 'tabpanel', { name: 'Settings' } )
.getByLabel( 'Title attribute' )
.inputValue();
expect( titleValue ).toBe( 'Value of the text_custom_field' );
expect( titleValue ).toBe( 'Value of the text custom field' );

// Check the frontend uses the value of the custom field.
const previewPage = await editor.openPreviewPage();
Expand All @@ -1998,7 +1998,7 @@
);
await expect( imageDom ).toHaveAttribute(
'title',
'Value of the text_custom_field'
'Value of the text custom field'
);
} );

Expand Down Expand Up @@ -2045,7 +2045,7 @@
.getByRole( 'tabpanel', { name: 'Settings' } )
.getByLabel( 'Alternative text' )
.inputValue();
expect( altValue ).toBe( 'Value of the text_custom_field' );
expect( altValue ).toBe( 'Value of the text custom field' );

// Title input should have the original value.
const advancedButton = page
Expand Down Expand Up @@ -2075,7 +2075,7 @@
);
await expect( imageDom ).toHaveAttribute(
'alt',
'Value of the text_custom_field'
'Value of the text custom field'
);
await expect( imageDom ).toHaveAttribute(
'title',
Expand Down
Loading