Skip to content

Commit

Permalink
Improve the placeholder instructions accessibility. (#45801)
Browse files Browse the repository at this point in the history
* Improve the placeholder instructions accessibility.

* Use a speak message for the placeholder instructions.

* Update test.

* Try to only announce message after initial focus.

* Fix unit tests.

* Announce message on render and add test.

* Changelog entry.

* Reviewer feedback.

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>

* Fix unit tests.

* Fix changelog.

* Remove duplicate CHANGELOG entry

---------

Co-authored-by: Alex Stine <alex.stine@yourtechadvisors.com>
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
  • Loading branch information
3 people authored and mikachan committed Sep 22, 2023
1 parent e38d246 commit af78d3f
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 39 deletions.
6 changes: 3 additions & 3 deletions packages/block-library/src/cover/test/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
} );

Expand Down
3 changes: 2 additions & 1 deletion packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

## Unreleased

### Bug fix
### Bug Fix

- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)).
- `DateTimePicker`: fix onChange callback check so that it also works inside iframes ([#54669](https://github.com/WordPress/gutenberg/pull/54669)).

## 25.8.0 (2023-09-20)
Expand Down
23 changes: 15 additions & 8 deletions packages/components/src/placeholder/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import classnames from 'classnames';
*/
import { useResizeObserver } from '@wordpress/compose';
import { SVG, Path } from '@wordpress/primitives';
import { useEffect } from '@wordpress/element';
import { speak } from '@wordpress/a11y';

/**
* Internal dependencies
Expand Down Expand Up @@ -72,10 +74,17 @@ export function Placeholder(
modifierClassNames,
withIllustration ? 'has-illustration' : null
);

const fieldsetClasses = classnames( 'components-placeholder__fieldset', {
'is-column-layout': isColumnLayout,
} );

useEffect( () => {
if ( instructions ) {
speak( instructions );
}
}, [ instructions ] );

return (
<div { ...additionalProps } className={ classes }>
{ withIllustration ? PlaceholderIllustration : null }
Expand All @@ -90,14 +99,12 @@ export function Placeholder(
<Icon icon={ icon } />
{ label }
</div>
<fieldset className={ fieldsetClasses }>
{ !! instructions && (
<legend className="components-placeholder__instructions">
{ instructions }
</legend>
) }
{ children }
</fieldset>
{ !! instructions && (
<div className="components-placeholder__instructions">
{ instructions }
</div>
) }
<div className={ fieldsetClasses }>{ children }</div>
</div>
);
}
Expand Down
12 changes: 0 additions & 12 deletions packages/components/src/placeholder/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,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;
Expand Down
49 changes: 34 additions & 15 deletions packages/components/src/placeholder/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -41,17 +42,21 @@ const Placeholder = (

const getPlaceholder = () => screen.getByTestId( 'placeholder' );

jest.mock( '@wordpress/a11y', () => ( { speak: jest.fn() } ) );
const mockedSpeak = jest.mocked( speak );

describe( 'Placeholder', () => {
beforeEach( () => {
// @ts-ignore
useResizeObserver.mockReturnValue( [
<div key="1" />,
{ width: 320 },
] );
mockedSpeak.mockReset();
} );

describe( 'basic rendering', () => {
it( 'should by default render label section and fieldset.', () => {
it( 'should by default render label section and content section.', () => {
render( <Placeholder /> );
const placeholder = getPlaceholder();

Expand All @@ -74,9 +79,12 @@ 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`
// eslint-disable-next-line testing-library/no-node-access
const placeholderFieldset = placeholder.querySelector(
'.components-placeholder__fieldset'
);
expect( placeholderFieldset ).toBeInTheDocument();
expect( placeholderFieldset ).toBeEmptyDOMElement();
} );
Expand Down Expand Up @@ -104,27 +112,38 @@ 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( <Placeholder>{ content }</Placeholder> );
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(
<Placeholder instructions={ instructions }>
<div>Fieldset</div>
<div>Placeholder content</div>
</Placeholder>
);
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 instructions={ instructions }>
<div>Placeholder content</div>
</Placeholder>
);
const captionedFieldset = screen.getByRole( 'group', {
name: instructions,
} );

expect( captionedFieldset ).toBeInTheDocument();
expect( speak ).toHaveBeenCalledWith( instructions );
} );

it( 'should add an additional className to the top container', () => {
Expand Down

0 comments on commit af78d3f

Please sign in to comment.