From 0080898f7391db113595adedc9f1ddd961bfbcb3 Mon Sep 17 00:00:00 2001
From: Luis Felipe Zaguini <26530524+zaguiini@users.noreply.github.com>
Date: Tue, 24 Sep 2024 09:43:28 -0300
Subject: [PATCH] Block Bindings: Prioritize existing placeholder over
 bindingsPlaceholder (#65220)

* Pass data-custom-placeholder prop to all RichText instances that are modifying the placeholder prop

* Use placeholder if data-custom-placeholder is true

* Add test

* Only use placeholder if aria-label is not defined

* Prioritize custom placeholder in aria-label

* Fix Cover block test

* Revert aria-label changes

* Try: Read block attribute in bindings placeholder

* Revert changes to placeholder prop

* Fix after rebase

* Explicitly return `null` for empty values

---------

Co-authored-by: zaguiini <zaguiini@git.wordpress.org>
Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
---
 .../src/components/rich-text/index.js         | 35 ++++++++++++++-----
 .../editor/various/block-bindings.spec.js     | 35 ++++++++++++++++++-
 2 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js
index 387f388b8fdad6..9c67476ed0ea63 100644
--- a/packages/block-editor/src/components/rich-text/index.js
+++ b/packages/block-editor/src/components/rich-text/index.js
@@ -165,7 +165,7 @@ export function RichTextWrapper(
 		isBlockSelected,
 	] );
 
-	const { disableBoundBlock, bindingsPlaceholder } = useSelect(
+	const { disableBoundBlock, bindingsPlaceholder, bindingsLabel } = useSelect(
 		( select ) => {
 			if (
 				! blockBindings?.[ identifier ] ||
@@ -179,10 +179,6 @@ export function RichTextWrapper(
 			const blockBindingsSource = getBlockBindingsSource(
 				relatedBinding.source
 			);
-			const fieldsList = blockBindingsSource?.getFieldsList?.( {
-				registry,
-				context: blockContext,
-			} );
 
 			const _disableBoundBlock =
 				! blockBindingsSource?.canUserEditValue?.( {
@@ -191,6 +187,22 @@ export function RichTextWrapper(
 					args: relatedBinding.args,
 				} );
 
+			// Don't modify placeholders if value is not empty.
+			if ( adjustedValue.length > 0 ) {
+				return {
+					disableBoundBlock: _disableBoundBlock,
+					// Null values will make them fall back to the default behavior.
+					bindingsPlaceholder: null,
+					bindingsLabel: null,
+				};
+			}
+
+			const { getBlockAttributes } = select( blockEditorStore );
+			const blockAttributes = getBlockAttributes( clientId );
+			const fieldsList = blockBindingsSource?.getFieldsList?.( {
+				registry,
+				context: blockContext,
+			} );
 			const bindingKey =
 				fieldsList?.[ relatedBinding?.args?.key ]?.label ??
 				blockBindingsSource?.label;
@@ -202,12 +214,19 @@ export function RichTextWrapper(
 						__( 'Add %s' ),
 						bindingKey
 				  );
+			const _bindingsLabel = _disableBoundBlock
+				? relatedBinding?.args?.key || blockBindingsSource?.label
+				: sprintf(
+						/* translators: %s: source label or key */
+						__( 'Empty %s; start writing to edit its value' ),
+						relatedBinding?.args?.key || blockBindingsSource?.label
+				  );
 
 			return {
 				disableBoundBlock: _disableBoundBlock,
 				bindingsPlaceholder:
-					( ! adjustedValue || adjustedValue.length === 0 ) &&
-					_bindingsPlaceholder,
+					blockAttributes?.placeholder || _bindingsPlaceholder,
+				bindingsLabel: _bindingsLabel,
 			};
 		},
 		[
@@ -421,7 +440,7 @@ export function RichTextWrapper(
 				aria-readonly={ shouldDisableEditing }
 				{ ...props }
 				aria-label={
-					bindingsPlaceholder || props[ 'aria-label' ] || placeholder
+					bindingsLabel || props[ 'aria-label' ] || placeholder
 				}
 				{ ...autocompleteProps }
 				ref={ useMergeRefs( [
diff --git a/test/e2e/specs/editor/various/block-bindings.spec.js b/test/e2e/specs/editor/various/block-bindings.spec.js
index 90a5d2b1da9f10..010d173e760ca9 100644
--- a/test/e2e/specs/editor/various/block-bindings.spec.js
+++ b/test/e2e/specs/editor/various/block-bindings.spec.js
@@ -1254,6 +1254,39 @@ test.describe( 'Block bindings', () => {
 				).toHaveText( 'fallback value' );
 			} );
 
+			test( 'should prioritize the placeholder attribute over the placeholder generated by the bindings API', async ( {
+				editor,
+			} ) => {
+				await editor.insertBlock( {
+					name: 'core/paragraph',
+					attributes: {
+						placeholder: 'My custom placeholder',
+						content: 'paragraph default content',
+						metadata: {
+							bindings: {
+								content: {
+									source: 'core/post-meta',
+									args: { key: 'empty_field' },
+								},
+							},
+						},
+					},
+				} );
+
+				const paragraphBlock = editor.canvas.getByRole( 'document', {
+					// Aria-label is changed for empty paragraphs.
+					name: 'Empty empty_field; start writing to edit its value',
+				} );
+
+				await expect( paragraphBlock ).toBeEmpty();
+
+				const placeholder = paragraphBlock.locator( 'span' );
+				await expect( placeholder ).toHaveAttribute(
+					'data-rich-text-placeholder',
+					'My custom placeholder'
+				);
+			} );
+
 			test( 'should show the prompt placeholder in field with empty value', async ( {
 				editor,
 			} ) => {
@@ -1274,7 +1307,7 @@ test.describe( 'Block bindings', () => {
 
 				const paragraphBlock = editor.canvas.getByRole( 'document', {
 					// Aria-label is changed for empty paragraphs.
-					name: 'Add empty_field',
+					name: 'Empty empty_field; start writing to edit its value',
 				} );
 
 				await expect( paragraphBlock ).toBeEmpty();