From 0aa97821c8a4fd8b34a2c35cb2ab824e090fc66b Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Tue, 15 Nov 2022 16:29:56 +0100 Subject: [PATCH 01/11] Improve the placeholder instructions accessibility. --- packages/components/src/placeholder/index.tsx | 17 ++++++----- .../components/src/placeholder/style.scss | 12 -------- .../components/src/placeholder/test/index.tsx | 30 +++++++++---------- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/packages/components/src/placeholder/index.tsx b/packages/components/src/placeholder/index.tsx index fe22bb5a3df0e5..9897897aa0beaa 100644 --- a/packages/components/src/placeholder/index.tsx +++ b/packages/components/src/placeholder/index.tsx @@ -93,14 +93,15 @@ export function Placeholder< IconProps = unknown >( { label } -
- { !! instructions && ( - - { instructions } - - ) } - { children } -
+ { !! instructions && ( +
+ { instructions } +
+ ) } +
{ children }
); } diff --git a/packages/components/src/placeholder/style.scss b/packages/components/src/placeholder/style.scss index 7fd9977e4645dc..4cc559bbc8d024 100644 --- a/packages/components/src/placeholder/style.scss +++ b/packages/components/src/placeholder/style.scss @@ -73,18 +73,6 @@ } } -// Overrides for browser and editor fieldset styles. -.components-placeholder__fieldset.components-placeholder__fieldset { - border: none; - padding: 0; - - .components-placeholder__instructions { - padding: 0; - font-weight: normal; - font-size: 1em; - } -} - .components-placeholder__fieldset.is-column-layout, .components-placeholder__fieldset.is-column-layout form { flex-direction: column; diff --git a/packages/components/src/placeholder/test/index.tsx b/packages/components/src/placeholder/test/index.tsx index e0ebc8ccae563d..a503c2140b5a5e 100644 --- a/packages/components/src/placeholder/test/index.tsx +++ b/packages/components/src/placeholder/test/index.tsx @@ -51,7 +51,7 @@ describe( 'Placeholder', () => { } ); describe( 'basic rendering', () => { - it( 'should by default render label section and fieldset.', () => { + it( 'should by default render label section and content section.', () => { render( ); const placeholder = getPlaceholder(); @@ -74,9 +74,11 @@ describe( 'Placeholder', () => { ); expect( placeholderInstructions ).not.toBeInTheDocument(); - // Test for empty fieldset. - const placeholderFieldset = - within( placeholder ).getByRole( 'group' ); + // Test for empty content. When the content is empty, + // the only way to query the div is with `querySelector` + const placeholderFieldset = placeholder.querySelector( + '.components-placeholder__fieldset' + ); expect( placeholderFieldset ).toBeInTheDocument(); expect( placeholderFieldset ).toBeEmptyDOMElement(); } ); @@ -104,27 +106,25 @@ describe( 'Placeholder', () => { expect( placeholderLabel ).toBeInTheDocument(); } ); - it( 'should display a fieldset from the children property', () => { - const content = 'Fieldset'; + it( 'should display content from the children property', () => { + const content = 'Placeholder content'; render( { content } ); - const placeholderFieldset = screen.getByRole( 'group' ); + const placeholder = screen.getByText( content ); - expect( placeholderFieldset ).toBeInTheDocument(); - expect( placeholderFieldset ).toHaveTextContent( content ); + expect( placeholder ).toBeInTheDocument(); + expect( placeholder ).toHaveTextContent( content ); } ); - it( 'should display a legend if instructions are passed', () => { + it( 'should display instructions when provided', () => { const instructions = 'Choose an option.'; render( -
Fieldset
+
Placeholder content
 ); - const captionedFieldset = screen.getByRole( 'group', { - name: instructions, - } ); + const instructionsContainer = screen.getByRole( 'status' ); - expect( captionedFieldset ).toBeInTheDocument(); + expect( instructionsContainer ).toBeInTheDocument(); } ); it( 'should add an additional className to the top container', () => { From 9bba5f0b5317c0a6e5b64db473d387a3d724679f Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Thu, 17 Nov 2022 16:39:23 +0100 Subject: [PATCH 02/11] Use a speak message for the placeholder instructions. --- packages/components/src/placeholder/index.tsx | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/components/src/placeholder/index.tsx b/packages/components/src/placeholder/index.tsx index 9897897aa0beaa..811b67db61beb3 100644 --- a/packages/components/src/placeholder/index.tsx +++ b/packages/components/src/placeholder/index.tsx @@ -6,8 +6,10 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useResizeObserver } from '@wordpress/compose'; +import { useResizeObserver, useDebounce } from '@wordpress/compose'; import { SVG, Path } from '@wordpress/primitives'; +import { useEffect } from '@wordpress/element'; +import { speak } from '@wordpress/a11y'; /** * Internal dependencies @@ -76,9 +78,19 @@ export function Placeholder< IconProps = unknown >( modifierClassNames, withIllustration ? 'has-illustration' : null ); + const fieldsetClasses = classnames( 'components-placeholder__fieldset', { 'is-column-layout': isColumnLayout, } ); + + const debouncedSpeak = useDebounce( speak, 100 ); + + useEffect( () => { + if ( instructions ) { + debouncedSpeak( instructions ); + } + }, [ instructions, debouncedSpeak ] ); + return (
{ withIllustration ? PlaceholderIllustration : null } @@ -94,10 +106,7 @@ export function Placeholder< IconProps = unknown >( { label }
{ !! instructions && ( -
+
{ instructions }
) } From ed6999188a9b19a47ad1a6d54f8c6e3d75ca98d9 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Thu, 17 Nov 2022 17:05:00 +0100 Subject: [PATCH 03/11] Update test. --- packages/components/src/placeholder/test/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/placeholder/test/index.tsx b/packages/components/src/placeholder/test/index.tsx index a503c2140b5a5e..0b51e6a37222f6 100644 --- a/packages/components/src/placeholder/test/index.tsx +++ b/packages/components/src/placeholder/test/index.tsx @@ -122,7 +122,7 @@ describe( 'Placeholder', () => {
Placeholder content
); - const instructionsContainer = screen.getByRole( 'status' ); + const instructionsContainer = screen.getByText( instructions ); expect( instructionsContainer ).toBeInTheDocument(); } ); From f818056bd860e2ca766322f581b51850bf98df85 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 6 Jan 2023 22:11:51 -0600 Subject: [PATCH 04/11] Try to only announce message after initial focus. --- packages/components/src/placeholder/index.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/components/src/placeholder/index.tsx b/packages/components/src/placeholder/index.tsx index 811b67db61beb3..10339d35037028 100644 --- a/packages/components/src/placeholder/index.tsx +++ b/packages/components/src/placeholder/index.tsx @@ -8,7 +8,7 @@ import classnames from 'classnames'; */ import { useResizeObserver, useDebounce } from '@wordpress/compose'; import { SVG, Path } from '@wordpress/primitives'; -import { useEffect } from '@wordpress/element'; +import { useEffect, useState } from '@wordpress/element'; import { speak } from '@wordpress/a11y'; /** @@ -84,15 +84,20 @@ export function Placeholder< IconProps = unknown >( } ); const debouncedSpeak = useDebounce( speak, 100 ); + const [ isInitialFocus, setIsInitialFocus ] = useState( false ); useEffect( () => { - if ( instructions ) { + if ( instructions && isInitialFocus ) { debouncedSpeak( instructions ); } - }, [ instructions, debouncedSpeak ] ); + }, [ instructions, debouncedSpeak, isInitialFocus ] ); return ( -
+
setIsInitialFocus( true ) } + > { withIllustration ? PlaceholderIllustration : null } { resizeListener } { notices } From d3faeb92f27f91e73a0e06130071dad22cfd0ce9 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 6 Jan 2023 22:57:11 -0600 Subject: [PATCH 05/11] Fix unit tests. --- packages/components/src/placeholder/test/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/placeholder/test/index.tsx b/packages/components/src/placeholder/test/index.tsx index 0b51e6a37222f6..f80b5c90ee5266 100644 --- a/packages/components/src/placeholder/test/index.tsx +++ b/packages/components/src/placeholder/test/index.tsx @@ -76,6 +76,7 @@ describe( 'Placeholder', () => { // Test for empty content. When the content is empty, // the only way to query the div is with `querySelector` + // eslint-disable-next-line testing-library/no-node-access const placeholderFieldset = placeholder.querySelector( '.components-placeholder__fieldset' ); From 3c8d17b18e3444c6435c200e6fa38cef7ada112e Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 22 Feb 2023 14:40:31 +0100 Subject: [PATCH 06/11] Announce message on render and add test. --- packages/components/src/placeholder/index.tsx | 19 ++++++------------ .../components/src/placeholder/test/index.tsx | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/components/src/placeholder/index.tsx b/packages/components/src/placeholder/index.tsx index 10339d35037028..36563145f7c78e 100644 --- a/packages/components/src/placeholder/index.tsx +++ b/packages/components/src/placeholder/index.tsx @@ -6,9 +6,9 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useResizeObserver, useDebounce } from '@wordpress/compose'; +import { useResizeObserver } from '@wordpress/compose'; import { SVG, Path } from '@wordpress/primitives'; -import { useEffect, useState } from '@wordpress/element'; +import { useEffect } from '@wordpress/element'; import { speak } from '@wordpress/a11y'; /** @@ -83,21 +83,14 @@ export function Placeholder< IconProps = unknown >( 'is-column-layout': isColumnLayout, } ); - const debouncedSpeak = useDebounce( speak, 100 ); - const [ isInitialFocus, setIsInitialFocus ] = useState( false ); - useEffect( () => { - if ( instructions && isInitialFocus ) { - debouncedSpeak( instructions ); + if ( instructions ) { + speak( instructions ); } - }, [ instructions, debouncedSpeak, isInitialFocus ] ); + }, [ instructions ] ); return ( -
setIsInitialFocus( true ) } - > +
{ withIllustration ? PlaceholderIllustration : null } { resizeListener } { notices } diff --git a/packages/components/src/placeholder/test/index.tsx b/packages/components/src/placeholder/test/index.tsx index f80b5c90ee5266..a131989d252385 100644 --- a/packages/components/src/placeholder/test/index.tsx +++ b/packages/components/src/placeholder/test/index.tsx @@ -8,6 +8,7 @@ import { render, screen, within } from '@testing-library/react'; */ import { useResizeObserver } from '@wordpress/compose'; import { SVG, Path } from '@wordpress/primitives'; +import { speak } from '@wordpress/a11y'; /** * Internal dependencies @@ -41,6 +42,9 @@ const Placeholder = ( const getPlaceholder = () => screen.getByTestId( 'placeholder' ); +jest.mock( '@wordpress/a11y', () => ( { speak: jest.fn() } ) ); +const mockedSpeak = jest.mocked( speak ); + describe( 'Placeholder', () => { beforeEach( () => { // @ts-ignore @@ -48,6 +52,7 @@ describe( 'Placeholder', () => {
, { width: 320 }, ] ); + mockedSpeak.mockReset(); } ); describe( 'basic rendering', () => { @@ -123,11 +128,24 @@ describe( 'Placeholder', () => {
Placeholder content
); - const instructionsContainer = screen.getByText( instructions ); + const placeholder = getPlaceholder(); + const instructionsContainer = + within( placeholder ).getByText( instructions ); expect( instructionsContainer ).toBeInTheDocument(); } ); + it( 'should announce instructions to screen readers', () => { + const instructions = 'Awesome block placeholder instructions.'; + render( + +
Placeholder content
+ + ); + + expect( speak ).toHaveBeenCalledWith( instructions ); + } ); + it( 'should add an additional className to the top container', () => { render( ); const placeholder = getPlaceholder(); From 887b619e89039c09332213cc15f380df58baf2bb Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 19 Sep 2023 10:54:08 -0500 Subject: [PATCH 07/11] Changelog entry. --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 93b3f52d6e4915..f0b4ef6d73badf 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -25,6 +25,7 @@ - `InputControl`: Fix focus style size ([#54394](https://github.com/WordPress/gutenberg/pull/54394)). - `BorderControl`: Use standard focus style on BorderControl ([#54429](https://github.com/WordPress/gutenberg/pull/54429)). - `Color values`: Update borderFocus to ADMIN.theme ([#54425](https://github.com/WordPress/gutenberg/pull/54425)). +- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)). ### Internal From 22c9153f40daf104cedaa61289683f7fecbd8fbe Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 19 Sep 2023 13:04:04 -0500 Subject: [PATCH 08/11] Reviewer feedback. Co-authored-by: Marco Ciampini --- packages/components/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 80b221c4d4f3b0..9e1db679bb811a 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -25,7 +25,7 @@ - `InputControl`: Fix focus style size ([#54394](https://github.com/WordPress/gutenberg/pull/54394)). - `BorderControl`: Use standard focus style on BorderControl ([#54429](https://github.com/WordPress/gutenberg/pull/54429)). - `Color values`: Update borderFocus to ADMIN.theme ([#54425](https://github.com/WordPress/gutenberg/pull/54425)). -- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)). +- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)). ### Internal From d4c52659742f85502d9b221f9058f4e3b6c5f96e Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 19 Sep 2023 17:38:39 -0500 Subject: [PATCH 09/11] Fix unit tests. --- packages/block-library/src/cover/test/edit.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/cover/test/edit.js b/packages/block-library/src/cover/test/edit.js index 74db754ff16969..8a391baa5f01d7 100644 --- a/packages/block-library/src/cover/test/edit.js +++ b/packages/block-library/src/cover/test/edit.js @@ -64,9 +64,9 @@ describe( 'Cover block', () => { await setup(); expect( - screen.getByRole( 'group', { - name: 'To edit this block, you need permission to upload media.', - } ) + within( screen.getByLabelText( 'Block: Cover' ) ).getByText( + 'To edit this block, you need permission to upload media.' + ) ).toBeInTheDocument(); } ); From e4a0f6a24c128f95f9b84a55beebaeba5673e59d Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 20 Sep 2023 11:49:35 -0500 Subject: [PATCH 10/11] Fix changelog. --- packages/components/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 8865c19f9209eb..987f9f463b1db4 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Bug Fix + +- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)). + ## 25.8.0 (2023-09-20) ### Enhancements From 87923daaf727ca54b172a834925c8f5f79600c22 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 21 Sep 2023 10:44:32 +0200 Subject: [PATCH 11/11] Remove duplicate CHANGELOG entry --- packages/components/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 922a1f9a3b9561..b0102ecc3ca176 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -35,7 +35,6 @@ - `InputControl`: Fix focus style size ([#54394](https://github.com/WordPress/gutenberg/pull/54394)). - `BorderControl`: Use standard focus style on BorderControl ([#54429](https://github.com/WordPress/gutenberg/pull/54429)). - `Color values`: Update borderFocus to ADMIN.theme ([#54425](https://github.com/WordPress/gutenberg/pull/54425)). -- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)). ### Internal