Components have incorrect id attribute

The `id` was not unique since the component might render the same collection of options multiple times within the same page. We have now a new React hook `useId` which fallback to the `React.useId` in the case the component does not get any `id` as a prop. Fix #26
Spaghetti-Dojo · Feb 16, 2024 · b80bb6c · b80bb6c
1 parent 411f2bb
commit b80bb6c
Show file tree

Hide file tree

Showing 12 changed files with 161 additions and 108 deletions.
diff --git a/@types/index.d.ts b/@types/index.d.ts
@@ -159,6 +159,7 @@ declare namespace EntitiesSearch {
 	interface SearchControl
 		extends Readonly<{
 			id?: string;
+			label?: string;
 			onChange(phrase: string | React.ChangeEvent<HTMLInputElement>);
 		}> {}
 

diff --git a/sources/client/src/components/radio-control.tsx b/sources/client/src/components/radio-control.tsx
@@ -2,9 +2,17 @@ import EntitiesSearch from '@types';
 import classnames from 'classnames';
 import React, { JSX } from 'react';
 
+import { useId } from '../hooks/use-id';
+
+interface Option<V> extends EntitiesSearch.ControlOption<V> {
+	readonly selectedValue: EntitiesSearch.SingularControl<V>['value'];
+	readonly onChange: (event: React.ChangeEvent<HTMLInputElement>) => void;
+}
+
 export function RadioControl(
 	props: EntitiesSearch.SingularControl<EntitiesSearch.Value> & {
 		className?: string;
+		id?: string;
 	}
 ): JSX.Element {
 	const className = classnames(props.className, 'wes-radio-control');
@@ -25,24 +33,36 @@ export function RadioControl(
 	return (
 		<div className={className}>
 			{props.options.map((option) => (
-				<div
+				<Option
 					key={option.value}
-					className={`wes-radio-control-item wes-radio-control-item--${option.value}`}
-				>
-					<label
-						htmlFor={`wes-radio-control-item__input-${option.value}`}
-					>
-						<input
-							type="radio"
-							id={`wes-radio-control-item__input-${option.value}`}
-							checked={props.value === option.value}
-							value={option.value}
-							onChange={onChange}
-						/>
-						{option.label}
-					</label>
-				</div>
+					label={option.label}
+					value={option.value}
+					selectedValue={props.value}
+					onChange={onChange}
+				/>
 			))}
 		</div>
 	);
 }
+
+function Option<V>(props: Option<V>): JSX.Element {
+	const id = useId();
+	const value = String(props.value);
+
+	return (
+		<div
+			className={`wes-radio-control-item wes-radio-control-item--${value}`}
+		>
+			<label htmlFor={id}>
+				<input
+					type="radio"
+					id={id}
+					checked={props.selectedValue === props.value}
+					value={value}
+					onChange={props.onChange}
+				/>
+				{props.label}
+			</label>
+		</div>
+	);
+}
diff --git a/sources/client/src/components/search-control.tsx b/sources/client/src/components/search-control.tsx
@@ -1,19 +1,16 @@
 import EntitiesSearch from '@types';
-import React, { JSX, PropsWithChildren, useCallback } from 'react';
+import React, { JSX } from 'react';
 
 import { __ } from '@wordpress/i18n';
 
+import { useId } from '../hooks/use-id';
+
 export function SearchControl(
 	props: EntitiesSearch.SearchControl
 ): JSX.Element {
-	const [searchValue, setSearchValue] = React.useState<string>('');
-
-	const Container = useCallback(
-		(containerProps: PropsWithChildren) => (
-			<div className="wes-search-control">{containerProps.children}</div>
-		),
-		[]
-	);
+	const id = useId(props.id);
+	const label = props.label || __('Search', 'wp-entities-search');
+	const [searchValue, setSearchValue] = React.useState('');
 
 	const onChange = (event: React.ChangeEvent<HTMLInputElement>) => {
 		setSearchValue(event.target.value);
@@ -27,20 +24,12 @@ export function SearchControl(
 		onChange,
 	};
 
-	if (props.id) {
-		return (
-			<Container>
-				<label htmlFor={props.id}>
-					{__('Search', 'wp-entities-search')}
-					<input id={props.id} {...inputProps} />
-				</label>
-			</Container>
-		);
-	}
-
 	return (
-		<Container>
-			<input {...inputProps} />
-		</Container>
+		<div className="wes-search-control">
+			<label htmlFor={id}>
+				<span className="wes-search-control__label">{label}</span>
+				<input id={id} {...inputProps} />
+			</label>
+		</div>
 	);
 }
diff --git a/sources/client/src/components/toggle-control.tsx b/sources/client/src/components/toggle-control.tsx
@@ -2,9 +2,14 @@ import EntitiesSearch from '@types';
 import classnames from 'classnames';
 import React, { JSX } from 'react';
 
-import { slugifyOptionLabel } from '../utils/slugify-option-label';
+import { useId } from '../hooks/use-id';
 import { NoOptionsMessage } from './no-options-message';
 
+interface Option<V> extends EntitiesSearch.ControlOption<V> {
+	readonly selectedValues: EntitiesSearch.BaseControl<V>['value'];
+	readonly onChange: (event: React.ChangeEvent<HTMLInputElement>) => void;
+}
+
 export function ToggleControl(
 	props: EntitiesSearch.BaseControl<EntitiesSearch.Value> & {
 		className?: string;
@@ -36,36 +41,38 @@ export function ToggleControl(
 
 	return (
 		<div className={className}>
-			{props.options.map((option) => {
-				const value = String(option.value);
-				const id = idByControlOption(option);
-				return (
-					<div
-						key={value}
-						className={`wes-toggle-control-item wes-toggle-control-item--${option.value}`}
-					>
-						<label htmlFor={id}>
-							<input
-								type="checkbox"
-								id={id}
-								className={`wes-toggle-control-item__input-${option.value}`}
-								checked={props.value?.has(option.value)}
-								value={value}
-								onChange={onChange}
-							/>
-							{option.label}
-						</label>
-					</div>
-				);
-			})}
+			{props.options.map((option) => (
+				<Option<EntitiesSearch.Value>
+					key={option.value}
+					label={option.label}
+					value={option.value}
+					selectedValues={props.value}
+					onChange={onChange}
+				/>
+			))}
 		</div>
 	);
 }
 
-function idByControlOption<V>(
-	controlOption: EntitiesSearch.ControlOption<V>
-): string {
-	const { value } = controlOption;
-	const label = slugifyOptionLabel(controlOption);
-	return `wes-toggle-control-item__input-${label}-${value}`;
+function Option<V>(props: Option<V>): JSX.Element {
+	const id = useId();
+	const value = String(props.value);
+
+	return (
+		<div
+			className={`wes-toggle-control-item wes-toggle-control-item--${value}`}
+		>
+			<label htmlFor={id}>
+				<input
+					type="checkbox"
+					id={id}
+					className={`wes-toggle-control-item__input-${value}`}
+					checked={props.selectedValues?.has(props.value)}
+					value={value}
+					onChange={props.onChange}
+				/>
+				{props.label}
+			</label>
+		</div>
+	);
 }
diff --git a/sources/client/src/hooks/use-id.ts b/sources/client/src/hooks/use-id.ts
@@ -0,0 +1,6 @@
+import React from 'react';
+
+export function useId(maybeId?: string): string {
+	const fallbackId = React.useId();
+	return maybeId ?? fallbackId;
+}
diff --git a/sources/client/src/hooks/use-search.ts b/sources/client/src/hooks/use-search.ts
@@ -13,11 +13,10 @@ type SearchFunc = (phrase: SearchPhrase) => void;
  * Build a function to search the entities by a phrase
  *
  * @public
- * @param setSearchPhrase A function to set the search phrase
- * @param searchEntities  The function that will search the entities
- * @param kind            The kind of entities to search
- * @param entities        The entities to exclude from the search
- * @param dispatch        The dispatch function to update the state
+ * @param searchEntities The function that will search the entities
+ * @param kind           The kind of entities to search
+ * @param entities       The entities to exclude from the search
+ * @param dispatch       The dispatch function to update the state
  */
 export function useSearch<E, K>(
 	searchEntities: EntitiesSearch.SearchEntitiesFunction<E, K>,

diff --git a/sources/client/src/index.ts b/sources/client/src/index.ts
@@ -15,7 +15,6 @@ export * from './hooks/use-query-viewable-taxonomies';
 export * from './utils/convert-entities-to-control-options';
 export * from './utils/is-control-option';
 export * from './utils/order-selected-options-at-the-top';
-export * from './utils/slugify-option-label';
 export * from './utils/unique-control-options';
 
 export * from './vo/control-option';

diff --git a/sources/client/src/utils/slugify-option-label.ts b/sources/client/src/utils/slugify-option-label.ts
diff --git a/tests/client/unit/components/__snapshots__/radio-control.test.tsx.snap b/tests/client/unit/components/__snapshots__/radio-control.test.tsx.snap
@@ -9,7 +9,7 @@ exports[`KindRadioControl renders the NoOptionsMessage when there are no options
 exports[`KindRadioControl renders the component 1`] = `
 <input
   checked=""
-  id="wes-radio-control-item__input-option-one"
+  id=":r0:"
   type="radio"
   value="option-one"
 />

diff --git a/tests/client/unit/components/__snapshots__/search-control.test.tsx.snap b/tests/client/unit/components/__snapshots__/search-control.test.tsx.snap
@@ -3,6 +3,7 @@
 exports[`SearchControl renders the input outside the label if the id prop is not passed 1`] = `
 <input
   class="wes-search-control__input"
+  id=":r1:"
   type="search"
   value=""
 />

diff --git a/tests/client/unit/components/__snapshots__/toggle-control.test.tsx.snap b/tests/client/unit/components/__snapshots__/toggle-control.test.tsx.snap
@@ -8,12 +8,12 @@ exports[`EntitiesToggleControl renders correctly 1`] = `
     class="wes-toggle-control-item wes-toggle-control-item--1"
   >
     <label
-      for="wes-toggle-control-item__input-option-1-1"
+      for=":r0:"
     >
       <input
         checked=""
         class="wes-toggle-control-item__input-1"
-        id="wes-toggle-control-item__input-option-1-1"
+        id=":r0:"
         type="checkbox"
         value="1"
       />
@@ -24,11 +24,11 @@ exports[`EntitiesToggleControl renders correctly 1`] = `
     class="wes-toggle-control-item wes-toggle-control-item--2"
   >
     <label
-      for="wes-toggle-control-item__input-option-2-2"
+      for=":r1:"
     >
       <input
         class="wes-toggle-control-item__input-2"
-        id="wes-toggle-control-item__input-option-2-2"
+        id=":r1:"
         type="checkbox"
         value="2"
       />
@@ -39,11 +39,11 @@ exports[`EntitiesToggleControl renders correctly 1`] = `
     class="wes-toggle-control-item wes-toggle-control-item--3"
   >
     <label
-      for="wes-toggle-control-item__input-option-3-3"
+      for=":r2:"
     >
       <input
         class="wes-toggle-control-item__input-3"
-        id="wes-toggle-control-item__input-option-3-3"
+        id=":r2:"
         type="checkbox"
         value="3"
       />