Allow define RadioControl label with ReactNode component

[retrofox]

What?

This PR allows using a ReactNode element for label in the RadioControl component

Why?

Because sometimes it's desired to render not only text for the radio control label

How?

• Allow to define ReactNode for radio label

Testing Instructions

1. Run storybook

npm run storybook:dev

2. Go to the new With Component Labels story
3. Confirm the story works as expected

Testing Instructions for Keyboard

Screenshots or screencast

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: retrofox <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: ciampo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[mirka]

Sorry for the delay!

Like I noted in the original issue, I think a requirement for this change is that we stop using a <label> internally and associate the input with a aria-labelledby instead. The usage example in this PR already renders invalid HTML (<div> inside a <label>), so we can assume other consumers would make the same mistake 😄

[tyxla]

ReactNode is a bit broad. Do we really want label to possibly be boolean or null or even number? I wonder if we meant ReactElement instead.

[retrofox]

Mmm, I don't think so. Maybe a number, but I believe string covers that.
Replacing...

[retrofox]

c6b9c9e

[mirka]

Do we really want label to possibly be boolean or null

This is a reasonable point, but I think we should also consider how many of our components that accept non-string labels right now already accept ReactNode. Introducing a subtle inconsistency like this could cause complications when composing components/types. Would it be ok to consider a package-wide switch from ReactNode to ReactElement as a separate issue?

[tyxla]

I'm happy to consider this a separate issue 👍 Let's document it in an issue though.

[retrofox]

Sorry for the delay!

Like I noted in the original issue, I think a requirement for this change is that we stop using a <label> internally and associate the input with a aria-labelledby instead. The usage example in this PR already renders invalid HTML (<div> inside a <label>), so we can assume other consumers would make the same mistake 😄

Never stop learning. Thanks a bunch for your :-) feedback

[github-actions]

Flaky tests detected in c6b9c9e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12055287484
📝 Reported issues:

• [Flaky Test] should handle the inserter outer section #38757 in /test/e2e/specs/widgets/customizing-widgets.spec.js

[ciampo]

Looks like all feedback so far has been addressed — @mirka and @tyxla, do you have any more feedback to share?

[tyxla]

Nothing from my end 👍

[mirka]

Looks like all feedback so far has been addressed

The internals need to be altered to stop using <label>.

[retrofox]

Sorry for the delay!

Like I noted in the original issue, I think a requirement for this change is that we stop using a <label> internally and associate the input with a aria-labelledby instead.

Does it mean we need to handle the onClick event when the user clicks on the not-label element (span, div, ...)?

[mirka]

Does it mean we need to handle the onClick event when the user clicks on the not-label element (span, div, ...)?

Good point, I wasn't thinking about that. That handling would be necessary, and the upside doesn't seem worth it, at least for now.

Let's keep the label implementation as is, and start with just guidance in documentation. Something like:

diff --git a/packages/components/src/radio-control/stories/index.story.tsx b/packages/components/src/radio-control/stories/index.story.tsx
index 73123aa956..ddcf596d1a 100644
--- a/packages/components/src/radio-control/stories/index.story.tsx
+++ b/packages/components/src/radio-control/stories/index.story.tsx
@@ -95,8 +95,9 @@ WithOptionDescriptions.args = {
 };
 
 /**
- * When the label is not a string,
- * make sure that the element is accessibly labeled.
+ * When the label is not a string, make sure that the element is accessibly labeled.
+ * To ensure valid HTML and proper accessibility, only include [phrasing content](https://developer.mozilla.org/en-US/docs/Web/HTML/Content_categories#phrasing_content),
+ * but no descendant `label` elements. No [labelable](https://developer.mozilla.org/en-US/docs/Web/HTML/Content_categories#labelable) elements are allowed.
  */
 export const WithComponentLabels: StoryFn< typeof RadioControl > =
 	Template.bind( {} );
diff --git a/packages/components/src/radio-control/types.ts b/packages/components/src/radio-control/types.ts
index be42504d91..eb10112fda 100644
--- a/packages/components/src/radio-control/types.ts
+++ b/packages/components/src/radio-control/types.ts
@@ -20,6 +20,8 @@ export type RadioControlProps = Pick<
 		 * The label to be shown to the user.
 		 *
 		 * When the label is not a string, make sure that the element is accessibly labeled.
+		 * To ensure valid HTML and proper accessibility, only include [phrasing content](https://developer.mozilla.org/en-US/docs/Web/HTML/Content_categories#phrasing_content),
+		 * but no descendant `label` elements. No [labelable](https://developer.mozilla.org/en-US/docs/Web/HTML/Content_categories#labelable) elements are allowed.
 		 */
 		label: string | React.ReactElement;
 		/**

We'll keep an eye out for people ignoring it, and we can add more guardrails if it becomes problem.