CheckboxList and CheckboxControl: Label htmlFor and Input id should be unique (#50566)

* Extract the checkbox list option logic into its own component
This allows to generate a dynamic id per option so it never repeats in the entire document.

* Add changelog file

* Move CheckboxListOptionControl to its own file

* Add value to the checkbox control to be aligned with the native input type=checkbox element. And also use it to get the correct checkbox by value in unit tests

* Because checkboxes not longer have fixed ids, now we use their value to lookup for them instead

* Updating snapshots since the checkboxes ids are now autogenerated
This commit is contained in:
Maikel Perez 2024-08-12 12:33:17 -04:00 committed by GitHub
parent ddbb24e021
commit 640a6ca439
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 114 additions and 47 deletions

View File

@ -125,7 +125,7 @@ const setup = ( params: SetupParams ) => {
: []; : [];
const checkbox = Array.from( checkboxes ).find( const checkbox = Array.from( checkboxes ).find(
( input ) => input.id === value ( input ) => input.value === value
); );
return checkbox; return checkbox;

View File

@ -13,13 +13,14 @@ exports[`Filter by Stock block renders the stock filter block 1`] = `
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="instock" for="wc-block-checkbox-list-option-0"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="instock" id="wc-block-checkbox-list-option-0"
type="checkbox" type="checkbox"
value="instock"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -44,13 +45,14 @@ exports[`Filter by Stock block renders the stock filter block 1`] = `
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="onbackorder" for="wc-block-checkbox-list-option-1"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="onbackorder" id="wc-block-checkbox-list-option-1"
type="checkbox" type="checkbox"
value="onbackorder"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -75,13 +77,14 @@ exports[`Filter by Stock block renders the stock filter block 1`] = `
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="outofstock" for="wc-block-checkbox-list-option-2"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="outofstock" id="wc-block-checkbox-list-option-2"
type="checkbox" type="checkbox"
value="outofstock"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -122,13 +125,14 @@ exports[`Filter by Stock block renders the stock filter block with the filter bu
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="instock" for="wc-block-checkbox-list-option-3"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="instock" id="wc-block-checkbox-list-option-3"
type="checkbox" type="checkbox"
value="instock"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -153,13 +157,14 @@ exports[`Filter by Stock block renders the stock filter block with the filter bu
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="onbackorder" for="wc-block-checkbox-list-option-4"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="onbackorder" id="wc-block-checkbox-list-option-4"
type="checkbox" type="checkbox"
value="onbackorder"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -184,13 +189,14 @@ exports[`Filter by Stock block renders the stock filter block with the filter bu
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="outofstock" for="wc-block-checkbox-list-option-5"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="outofstock" id="wc-block-checkbox-list-option-5"
type="checkbox" type="checkbox"
value="outofstock"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -247,13 +253,14 @@ exports[`Filter by Stock block renders the stock filter block with the product c
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="instock" for="wc-block-checkbox-list-option-6"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="instock" id="wc-block-checkbox-list-option-6"
type="checkbox" type="checkbox"
value="instock"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -292,13 +299,14 @@ exports[`Filter by Stock block renders the stock filter block with the product c
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="onbackorder" for="wc-block-checkbox-list-option-7"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="onbackorder" id="wc-block-checkbox-list-option-7"
type="checkbox" type="checkbox"
value="onbackorder"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"
@ -337,13 +345,14 @@ exports[`Filter by Stock block renders the stock filter block with the product c
class="wc-block-components-checkbox wc-block-checkbox-list__checkbox" class="wc-block-components-checkbox wc-block-checkbox-list__checkbox"
> >
<label <label
for="outofstock" for="wc-block-checkbox-list-option-8"
> >
<input <input
aria-invalid="false" aria-invalid="false"
class="wc-block-components-checkbox__input" class="wc-block-components-checkbox__input"
id="outofstock" id="wc-block-checkbox-list-option-8"
type="checkbox" type="checkbox"
value="outofstock"
/> />
<svg <svg
aria-hidden="true" aria-hidden="true"

View File

@ -135,7 +135,7 @@ const setup = ( params: SetupParams = {} ) => {
: []; : [];
const checkbox = Array.from( checkboxes ).find( const checkbox = Array.from( checkboxes ).find(
( input ) => input.id === value ( input ) => input.value === value
); );
return checkbox; return checkbox;

View File

@ -0,0 +1,62 @@
/**
* External dependencies
*/
import { useInstanceId } from '@wordpress/compose';
/**
* Internal dependencies
*/
import { CheckboxControl } from '../checkbox-control';
import type { CheckboxListOptions } from './types';
export type CheckboxListOptionControlProps = {
option: CheckboxListOptions;
shouldTruncateOptions: boolean;
showExpanded: boolean;
index: number;
limit: number;
checked: boolean;
disabled: boolean;
renderedShowMore: false | JSX.Element;
onChange: ( value: string ) => void;
};
export function CheckboxListOptionControl( {
option,
shouldTruncateOptions,
showExpanded,
index,
limit,
checked,
disabled,
renderedShowMore,
onChange,
}: CheckboxListOptionControlProps ) {
const checkboxControlInstanceId = useInstanceId(
CheckboxListOptionControl,
'wc-block-checkbox-list-option'
) as string;
return (
<>
<li
{ ...( shouldTruncateOptions &&
! showExpanded &&
index >= limit && { hidden: true } ) }
>
<CheckboxControl
id={ checkboxControlInstanceId }
className="wc-block-checkbox-list__checkbox"
label={ option.label }
checked={ checked }
value={ option.value }
onChange={ () => {
onChange( option.value );
} }
disabled={ disabled }
/>
</li>
{ shouldTruncateOptions && index === limit - 1 && renderedShowMore }
</>
);
}

View File

@ -2,18 +2,15 @@
* External dependencies * External dependencies
*/ */
import { __, _n, sprintf } from '@wordpress/i18n'; import { __, _n, sprintf } from '@wordpress/i18n';
import { Fragment, useMemo, useState } from '@wordpress/element'; import { useMemo, useState } from '@wordpress/element';
import clsx from 'clsx'; import clsx from 'clsx';
/** /**
* Internal dependencies * Internal dependencies
*/ */
import { CheckboxListOptionControl } from './checkbox-list-option-control';
import type { CheckboxListOptions } from './types';
import './style.scss'; import './style.scss';
import { CheckboxControl } from '../checkbox-control';
interface CheckboxListOptions {
label: React.ReactNode;
value: string;
}
export interface CheckboxListProps { export interface CheckboxListProps {
className?: string | undefined; className?: string | undefined;
@ -126,27 +123,18 @@ const CheckboxList = ( {
return ( return (
<> <>
{ options.map( ( option, index ) => ( { options.map( ( option, index ) => (
<Fragment key={ option.value }> <CheckboxListOptionControl
<li key={ option.value }
{ ...( shouldTruncateOptions && option={ option }
! showExpanded && shouldTruncateOptions={ shouldTruncateOptions }
index >= limit && { hidden: true } ) } showExpanded={ showExpanded }
> index={ index }
<CheckboxControl limit={ limit }
id={ option.value } checked={ checked.includes( option.value ) }
className="wc-block-checkbox-list__checkbox" disabled={ isDisabled }
label={ option.label } renderedShowMore={ renderedShowMore }
checked={ checked.includes( option.value ) } onChange={ onChange }
onChange={ () => { />
onChange( option.value );
} }
disabled={ isDisabled }
/>
</li>
{ shouldTruncateOptions &&
index === limit - 1 &&
renderedShowMore }
</Fragment>
) ) } ) ) }
{ shouldTruncateOptions && renderedShowLess } { shouldTruncateOptions && renderedShowLess }
</> </>

View File

@ -0,0 +1,4 @@
export interface CheckboxListOptions {
label: React.ReactNode;
value: string;
}

View File

@ -0,0 +1,4 @@
Significance: minor
Type: tweak
Extract the checkbox list option logic into its own component