From 596d726fbe567c525285c496e6205479ad182106 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 20 Aug 2024 11:08:42 +0200 Subject: [PATCH] RadioControl: label radio group using fieldset and legend (#64582) * Swap BaseControl for fieldset + legend * Add help text * Use group help text to describe the fieldset instead of each individual option * CHANGELOG * Re-apply ID and classname * Render visually hidden label as div * Fix spacing in latest post block * Increase margin-top override specificity * Remove redundant styles * Use legend even when the label is visually hidden * Test that group labelling works as expected also when label is visually hidden * typo * Revert to original classname order * Move styles from styles.scss to editor.scss --- Co-authored-by: ciampo Co-authored-by: tyxla Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: afercia --- .../block-library/src/latest-posts/edit.js | 1 + .../src/latest-posts/editor.scss | 11 +++ packages/components/CHANGELOG.md | 1 + .../components/src/radio-control/index.tsx | 48 ++++++----- .../components/src/radio-control/style.scss | 13 ++- .../src/radio-control/test/index.tsx | 83 ++++++++++--------- .../src/components/post-discussion/style.scss | 9 -- .../src/components/post-format/style.scss | 6 -- .../src/components/post-status/style.scss | 19 ----- .../src/components/site-discussion/style.scss | 14 ---- 10 files changed, 96 insertions(+), 109 deletions(-) diff --git a/packages/block-library/src/latest-posts/edit.js b/packages/block-library/src/latest-posts/edit.js index 533e9621fd4f2b..b02747bb1c1d8e 100644 --- a/packages/block-library/src/latest-posts/edit.js +++ b/packages/block-library/src/latest-posts/edit.js @@ -242,6 +242,7 @@ export default function LatestPostsEdit( { attributes, setAttributes } ) { /> { displayPostContent && ( ) => onChange( event.target.value ); - // Use `useBaseControlProps` to get the id of the help text. - const { - controlProps: { 'aria-describedby': helpTextId }, - } = useBaseControlProps( { id, help } ); - if ( ! options?.length ) { return null; } return ( - + { hideLabelFromVision ? ( + { label } + ) : ( + + { label } + + ) } + @@ -142,7 +141,16 @@ export function RadioControl( ) ) } - + { !! help && ( + + { help } + + ) } + ); } diff --git a/packages/components/src/radio-control/style.scss b/packages/components/src/radio-control/style.scss index e9732558f2901e..e3dcb8d1e58270 100644 --- a/packages/components/src/radio-control/style.scss +++ b/packages/components/src/radio-control/style.scss @@ -1,3 +1,12 @@ +.components-radio-control { + border: 0; + margin: 0; + padding: 0; + + font-family: $default-font; + font-size: $default-font-size; +} + .components-radio-control__group-wrapper.has-help { margin-block-end: $grid-unit-15; } @@ -57,5 +66,7 @@ // Override the top margin of the StyledHelp component from BaseControl. // TODO: we should tweak the StyledHelp component to not have a top margin. - margin-top: 0; + &.components-radio-control__option-description { + margin-top: 0; + } } diff --git a/packages/components/src/radio-control/test/index.tsx b/packages/components/src/radio-control/test/index.tsx index 0be166513a9a4d..7e30744d96b826 100644 --- a/packages/components/src/radio-control/test/index.tsx +++ b/packages/components/src/radio-control/test/index.tsx @@ -56,6 +56,47 @@ describe.each( [ const [ , Component ] = modeAndComponent; describe( 'semantics and labelling', () => { + it( 'should group all radios under a fieldset with an accessible label (legend)', () => { + const onChangeSpy = jest.fn(); + render( + + ); + + expect( + screen.getByRole( 'group', { name: defaultProps.label } ) + ).toBeVisible(); + } ); + + it( 'should group all radios under a fieldset with an accessible label even when the label is visually hidden', () => { + const onChangeSpy = jest.fn(); + render( + + ); + + expect( + screen.getByRole( 'group', { name: defaultProps.label } ) + ).toBeVisible(); + } ); + + it( 'should describe the radio group with the help text', () => { + const onChangeSpy = jest.fn(); + render( + + ); + + expect( + screen.getByRole( 'group', { name: defaultProps.label } ) + ).toHaveAccessibleDescription( 'Test help text' ); + } ); + it( 'should render radio inputs with accessible labels', () => { const onChangeSpy = jest.fn(); render( @@ -101,46 +142,7 @@ describe.each( [ ).toHaveAccessibleName( defaultProps.options[ 1 ].label ); } ); - it( 'should use the group help text to describe individual options', () => { - const onChangeSpy = jest.fn(); - render( - - ); - - for ( const option of defaultProps.options ) { - expect( - screen.getByRole( 'radio', { name: option.label } ) - ).toHaveAccessibleDescription( 'Select your favorite animal.' ); - } - } ); - it( 'should use the option description text to describe individual options', () => { - const onChangeSpy = jest.fn(); - render( - - ); - - let index = 1; - for ( const option of defaultProps.options ) { - expect( - screen.getByRole( 'radio', { name: option.label } ) - ).toHaveAccessibleDescription( - `This is the option number ${ index }.` - ); - index += 1; - } - } ); - - it( 'should use both the option description text and the group help text to describe individual options', () => { const onChangeSpy = jest.fn(); render( ); + // Group help text should not be used to describe individual options. let index = 1; for ( const option of defaultProps.options ) { expect( screen.getByRole( 'radio', { name: option.label } ) ).toHaveAccessibleDescription( - `This is the option number ${ index }. Select your favorite animal` + `This is the option number ${ index }.` ); index += 1; } diff --git a/packages/editor/src/components/post-discussion/style.scss b/packages/editor/src/components/post-discussion/style.scss index b2d65c9aa7cf3f..d5320605579e41 100644 --- a/packages/editor/src/components/post-discussion/style.scss +++ b/packages/editor/src/components/post-discussion/style.scss @@ -2,15 +2,6 @@ // sidebar width - popover padding - form margin min-width: $sidebar-width - $grid-unit-20 - $grid-unit-20; margin: $grid-unit-10; - - .components-radio-control__option { - align-items: flex-start; - } - - .components-radio-control__label .components-text { - display: block; - margin-top: $grid-unit-05; - } } .editor-post-discussion__panel-toggle { diff --git a/packages/editor/src/components/post-format/style.scss b/packages/editor/src/components/post-format/style.scss index ec3a8b1626b543..9c893f36bcc16c 100644 --- a/packages/editor/src/components/post-format/style.scss +++ b/packages/editor/src/components/post-format/style.scss @@ -7,9 +7,3 @@ min-width: $sidebar-width - $grid-unit-20 - $grid-unit-20; margin: $grid-unit-10; } - -.editor-post-format__options { - .components-base-control__field > .components-v-stack { - gap: $grid-unit-15; - } -} diff --git a/packages/editor/src/components/post-status/style.scss b/packages/editor/src/components/post-status/style.scss index bfd9de46234da3..c601d887d68c7f 100644 --- a/packages/editor/src/components/post-status/style.scss +++ b/packages/editor/src/components/post-status/style.scss @@ -18,25 +18,6 @@ padding: $grid-unit-20; } - .editor-change-status__options { - .components-base-control__field > .components-v-stack { - gap: $grid-unit-15; - } - - // TODO: it's not great to override component styles.. This might be resolved - // by the new radio control component. - .components-radio-control__option { - align-items: flex-start; - } - - label { - .components-text { - display: block; - margin-top: $grid-unit-05; - } - } - } - .editor-change-status__password-legend { padding: 0; margin-bottom: $grid-unit-10; diff --git a/packages/editor/src/components/site-discussion/style.scss b/packages/editor/src/components/site-discussion/style.scss index 2c54424207ea5c..dc0608f0cf3ab1 100644 --- a/packages/editor/src/components/site-discussion/style.scss +++ b/packages/editor/src/components/site-discussion/style.scss @@ -3,17 +3,3 @@ padding: $grid-unit-20; } -.editor-site-discussion__options { - // TODO: it's not great to override component styles.. This might be resolved - // by the new radio control component. - .components-radio-control__option { - align-items: flex-start; - } - - label { - .components-text { - display: block; - margin-top: $grid-unit-05; - } - } -}