Block Bindings: Remove getPlaceholder API and rely on key argumen…

…t or source label (WordPress#64910) * Remove `getPlaceholder` API * Skip unregistered sources * Add e2e tests * Accept undefined as a valid value * Properly check if value is a URL Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org>
bph · Aug 31, 2024 · 6bdb9b4 · 6bdb9b4
1 parent 84baf0e
commit 6bdb9b4
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 57 deletions.
diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js
@@ -10,6 +10,7 @@ import { addFilter } from '@wordpress/hooks';
 /**
  * Internal dependencies
  */
+import isURLLike from '../components/link-control/is-url-like';
 import { unlock } from '../lock-unlock';
 import BlockContext from '../components/block-context';
 
@@ -134,10 +135,7 @@ export const withBlockBindingSupport = createHigherOrderComponent(
 			) ) {
 				const { source: sourceName, args: sourceArgs } = binding;
 				const source = sources[ sourceName ];
-				if (
-					! source?.getValues ||
-					! canBindAttribute( name, attributeName )
-				) {
+				if ( ! source || ! canBindAttribute( name, attributeName ) ) {
 					continue;
 				}
 
@@ -161,29 +159,30 @@ export const withBlockBindingSupport = createHigherOrderComponent(
 					}
 
 					// Get values in batch if the source supports it.
-					const values = source.getValues( {
-						registry,
-						context,
-						clientId,
-						bindings,
-					} );
+					let values = {};
+					if ( ! source.getValues ) {
+						Object.keys( bindings ).forEach( ( attr ) => {
+							// Default to the `key` or the source label when `getValues` doesn't exist
+							values[ attr ] =
+								bindings[ attr ].args?.key || source.label;
+						} );
+					} else {
+						values = source.getValues( {
+							registry,
+							context,
+							clientId,
+							bindings,
+						} );
+					}
 					for ( const [ attributeName, value ] of Object.entries(
 						values
 					) ) {
-						// Use placeholder when value is undefined.
-						if ( value === undefined ) {
-							if ( attributeName === 'url' ) {
-								attributes[ attributeName ] = null;
-							} else {
-								attributes[ attributeName ] =
-									source.getPlaceholder?.( {
-										registry,
-										context,
-										clientId,
-										attributeName,
-										args: bindings[ attributeName ].args,
-									} );
-							}
+						if (
+							attributeName === 'url' &&
+							( ! value || ! isURLLike( value ) )
+						) {
+							// Return null if value is not a valid URL.
+							attributes[ attributeName ] = null;
 						} else {
 							attributes[ attributeName ] = value;
 						}

diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js
@@ -773,7 +773,6 @@ export const unregisterBlockVariation = ( blockName, variationName ) => {
  * @param {Array}    [source.usesContext]      Array of context needed by the source only in the editor.
  * @param {Function} [source.getValues]        Function to get the values from the source.
  * @param {Function} [source.setValues]        Function to update multiple values connected to the source.
- * @param {Function} [source.getPlaceholder]   Function to get the placeholder when the value is undefined.
  * @param {Function} [source.canUserEditValue] Function to determine if the user can edit the value.
  * @param {Function} [source.getFieldsList]    Function to get the lists of fields to expose in the connections panel.
  *
@@ -787,7 +786,6 @@ export const unregisterBlockVariation = ( blockName, variationName ) => {
  *     label: _x( 'My Custom Source', 'block bindings source' ),
  *     getValues: () => getSourceValues(),
  *     setValues: () => updateMyCustomValuesInBatch(),
- *     getPlaceholder: () => 'Placeholder text when the value is undefined',
  *     canUserEditValue: () => true,
  * } );
  * ```
@@ -799,7 +797,6 @@ export const registerBlockBindingsSource = ( source ) => {
 		usesContext,
 		getValues,
 		setValues,
-		getPlaceholder,
 		canUserEditValue,
 		getFieldsList,
 	} = source;
@@ -889,13 +886,7 @@ export const registerBlockBindingsSource = ( source ) => {
 		return;
 	}
 
-	// Check the `getPlaceholder` property is correct.
-	if ( getPlaceholder && typeof getPlaceholder !== 'function' ) {
-		warning( 'Block bindings source getPlaceholder must be a function.' );
-		return;
-	}
-
-	// Check the `getPlaceholder` property is correct.
+	// Check the `canUserEditValue` property is correct.
 	if ( canUserEditValue && typeof canUserEditValue !== 'function' ) {
 		warning( 'Block bindings source canUserEditValue must be a function.' );
 		return;

diff --git a/packages/blocks/src/api/test/registration.js b/packages/blocks/src/api/test/registration.js
@@ -1630,19 +1630,6 @@ describe( 'blocks', () => {
 			expect( getBlockBindingsSource( 'core/testing' ) ).toBeUndefined();
 		} );
 
-		// Check the `getPlaceholder` callback is correct.
-		it( 'should reject invalid getPlaceholder callback', () => {
-			registerBlockBindingsSource( {
-				name: 'core/testing',
-				label: 'testing',
-				getPlaceholder: 'should be a function',
-			} );
-			expect( console ).toHaveWarnedWith(
-				'Block bindings source getPlaceholder must be a function.'
-			);
-			expect( getBlockBindingsSource( 'core/testing' ) ).toBeUndefined();
-		} );
-
 		// Check the `canUserEditValue` callback is correct.
 		it( 'should reject invalid canUserEditValue callback', () => {
 			registerBlockBindingsSource( {
@@ -1676,7 +1663,6 @@ describe( 'blocks', () => {
 				usesContext: [ 'postId' ],
 				getValues: () => 'value',
 				setValues: () => 'new values',
-				getPlaceholder: () => 'placeholder',
 				canUserEditValue: () => true,
 				getFieldsList: () => {
 					return { field: 'value' };
@@ -1701,7 +1687,6 @@ describe( 'blocks', () => {
 			expect( source.usesContext ).toBeUndefined();
 			expect( source.getValues ).toBeUndefined();
 			expect( source.setValues ).toBeUndefined();
-			expect( source.getPlaceholder ).toBeUndefined();
 			expect( source.canUserEditValue ).toBeUndefined();
 			expect( source.getFieldsList ).toBeUndefined();
 			unregisterBlockBindingsSource( 'core/valid-source' );
@@ -1726,7 +1711,6 @@ describe( 'blocks', () => {
 			const clientOnlyProperties = {
 				getValues: () => 'values',
 				setValues: () => 'new values',
-				getPlaceholder: () => 'placeholder',
 				canUserEditValue: () => true,
 			};
 			registerBlockBindingsSource( {

diff --git a/packages/blocks/src/store/private-actions.js b/packages/blocks/src/store/private-actions.js
@@ -54,7 +54,6 @@ export function addBlockBindingsSource( source ) {
 		usesContext: source.usesContext,
 		getValues: source.getValues,
 		setValues: source.setValues,
-		getPlaceholder: source.getPlaceholder,
 		canUserEditValue: source.canUserEditValue,
 		getFieldsList: source.getFieldsList,
 	};

diff --git a/packages/blocks/src/store/reducer.js b/packages/blocks/src/store/reducer.js
@@ -404,7 +404,6 @@ export function blockBindingsSources( state = {}, action ) {
 					),
 					getValues: action.getValues,
 					setValues: action.setValues,
-					getPlaceholder: action.getPlaceholder,
 					canUserEditValue: action.canUserEditValue,
 					getFieldsList: action.getFieldsList,
 				},

diff --git a/packages/editor/src/bindings/post-meta.js b/packages/editor/src/bindings/post-meta.js
@@ -10,9 +10,6 @@ import { store as editorStore } from '../store';
 
 export default {
 	name: 'core/post-meta',
-	getPlaceholder( { args } ) {
-		return args.key;
-	},
 	getValues( { registry, context, bindings } ) {
 		const meta = registry
 			.select( coreDataStore )
@@ -23,7 +20,9 @@ export default {
 			)?.meta;
 		const newValues = {};
 		for ( const [ attributeName, source ] of Object.entries( bindings ) ) {
-			newValues[ attributeName ] = meta?.[ source.args.key ];
+			// Use the key if the value is not set.
+			newValues[ attributeName ] =
+				meta?.[ source.args.key ] || source.args.key;
 		}
 		return newValues;
 	},

diff --git a/test/e2e/specs/editor/various/block-bindings.spec.js b/test/e2e/specs/editor/various/block-bindings.spec.js
@@ -41,7 +41,7 @@ test.describe( 'Block bindings', () => {
 		} );
 
 		test.describe( 'Paragraph', () => {
-			test( 'should show the value of the custom field', async ( {
+			test( 'should show the key of the custom field in post meta', async ( {
 				editor,
 			} ) => {
 				await editor.insertBlock( {
@@ -66,6 +66,53 @@ test.describe( 'Block bindings', () => {
 				);
 			} );
 
+			test( 'should show the key of the custom field in server sources with key', async ( {
+				editor,
+			} ) => {
+				await editor.insertBlock( {
+					name: 'core/paragraph',
+					attributes: {
+						content: 'paragraph default content',
+						metadata: {
+							bindings: {
+								content: {
+									source: 'core/server-source',
+									args: { key: 'text_custom_field' },
+								},
+							},
+						},
+					},
+				} );
+				const paragraphBlock = editor.canvas.getByRole( 'document', {
+					name: 'Block: Paragraph',
+				} );
+				await expect( paragraphBlock ).toHaveText(
+					'text_custom_field'
+				);
+			} );
+
+			test( 'should show the source label in server sources without key', async ( {
+				editor,
+			} ) => {
+				await editor.insertBlock( {
+					name: 'core/paragraph',
+					attributes: {
+						content: 'paragraph default content',
+						metadata: {
+							bindings: {
+								content: {
+									source: 'core/server-source',
+								},
+							},
+						},
+					},
+				} );
+				const paragraphBlock = editor.canvas.getByRole( 'document', {
+					name: 'Block: Paragraph',
+				} );
+				await expect( paragraphBlock ).toHaveText( 'Server Source' );
+			} );
+
 			test( 'should lock the appropriate controls with a registered source', async ( {
 				editor,
 				page,