From a2b06d02ccd19f5a22e2c5027be0b543de082b6a Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Fri, 13 Jan 2023 16:10:25 +0100 Subject: [PATCH 1/4] Fix UrlInput combobox to use the ARIA 1.0 pattern. --- .../src/components/link-control/test/index.js | 38 +++++++++++++++++++ .../src/components/url-input/index.js | 5 ++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 86eae7289f4f61..012e4aacc8caae 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -140,6 +140,17 @@ describe( 'Basic rendering', () => { expect( searchInput ).toBeInTheDocument(); } ); + it( 'should have aria-owns attribute to follow the ARIA 1.0 pattern', () => { + render( ); + + // Search Input UI. + const searchInput = screen.getByRole( 'combobox', { name: 'URL' } ); + + expect( searchInput ).toBeInTheDocument(); + expect( searchInput ).not.toHaveAttribute( 'aria-controls' ); + expect( searchInput ).toHaveAttribute( 'aria-owns' ); + } ); + it( 'should not render protocol in links', async () => { const user = userEvent.setup(); mockFetchSearchSuggestions.mockImplementation( () => @@ -1375,6 +1386,15 @@ describe( 'Selecting links', () => { firstSearchSuggestion ); + // Check aria-selected attribute is omitted on non-highlighted items. + expect( firstSearchSuggestion ).toHaveAttribute( + 'aria-selected', + 'true' + ); + expect( secondSearchSuggestion ).not.toHaveAttribute( + 'aria-selected' + ); + // Check we can go down again using the down arrow. triggerArrowDown( searchInput ); @@ -1387,6 +1407,15 @@ describe( 'Selecting links', () => { secondSearchSuggestion ); + // Check aria-selected attribute is omitted on non-highlighted items. + expect( firstSearchSuggestion ).not.toHaveAttribute( + 'aria-selected' + ); + expect( secondSearchSuggestion ).toHaveAttribute( + 'aria-selected', + 'true' + ); + // Check we can go back up via up arrow. triggerArrowUp( searchInput ); @@ -1399,6 +1428,15 @@ describe( 'Selecting links', () => { firstSearchSuggestion ); + // Check aria-selected attribute is omitted on non-highlighted items. + expect( firstSearchSuggestion ).toHaveAttribute( + 'aria-selected', + 'true' + ); + expect( secondSearchSuggestion ).not.toHaveAttribute( + 'aria-selected' + ); + expect( mockFetchSearchSuggestions ).toHaveBeenCalledTimes( 1 ); } ); } ); diff --git a/packages/block-editor/src/components/url-input/index.js b/packages/block-editor/src/components/url-input/index.js index 758c3bf51bec3d..cc4af719e63ec3 100644 --- a/packages/block-editor/src/components/url-input/index.js +++ b/packages/block-editor/src/components/url-input/index.js @@ -468,7 +468,7 @@ class URLInput extends Component { 'aria-label': label ? undefined : __( 'URL' ), // Ensure input always has an accessible label 'aria-expanded': showSuggestions, 'aria-autocomplete': 'list', - 'aria-controls': suggestionsListboxId, + 'aria-owns': suggestionsListboxId, 'aria-activedescendant': selectedSuggestion !== null ? `${ suggestionOptionIdPrefix }-${ selectedSuggestion }` @@ -531,7 +531,8 @@ class URLInput extends Component { tabIndex: '-1', id: `${ suggestionOptionIdPrefix }-${ index }`, ref: this.bindSuggestionNode( index ), - 'aria-selected': index === selectedSuggestion, + 'aria-selected': + index === selectedSuggestion ? true : undefined, }; }; From 228b4bf2ddf249b8f43cdb3276d9a80c392446a7 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Fri, 13 Jan 2023 17:21:50 +0100 Subject: [PATCH 2/4] Update changelog. --- changelog.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog.txt b/changelog.txt index 65bb846e47371c..f17871d971ac22 100644 --- a/changelog.txt +++ b/changelog.txt @@ -78,6 +78,8 @@ #### Block Library - Lodash: Remove `_.pickBy()` from latest posts block. ([46974](https://github.com/WordPress/gutenberg/pull/46974)) +### Accessibility +- Block Editor: Revert `aria-controls` to `aria-owns` in `URLInput` to use the more broadly supported ARIA 1.0 combobox pattern. ([47148](https://github.com/WordPress/gutenberg/pull/47148)) ### Experiments From e32866a4cfa8d45701e8070f4db5a36e47e9315b Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Mon, 16 Jan 2023 16:33:57 +0100 Subject: [PATCH 3/4] Improve testing for correct ARIA combobox pattern. --- .../src/components/link-control/test/index.js | 137 ++++++++++++++---- 1 file changed, 106 insertions(+), 31 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 012e4aacc8caae..fda223061a0f1f 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -137,7 +137,7 @@ describe( 'Basic rendering', () => { // Search Input UI. const searchInput = screen.getByRole( 'combobox', { name: 'URL' } ); - expect( searchInput ).toBeInTheDocument(); + expect( searchInput ).toBeVisible(); } ); it( 'should have aria-owns attribute to follow the ARIA 1.0 pattern', () => { @@ -146,11 +146,113 @@ describe( 'Basic rendering', () => { // Search Input UI. const searchInput = screen.getByRole( 'combobox', { name: 'URL' } ); - expect( searchInput ).toBeInTheDocument(); + expect( searchInput ).toBeVisible(); expect( searchInput ).not.toHaveAttribute( 'aria-controls' ); expect( searchInput ).toHaveAttribute( 'aria-owns' ); } ); + it( 'should have aria-selected attribute only on the highlighted item', async () => { + const user = userEvent.setup(); + + let resolver; + mockFetchSearchSuggestions.mockImplementation( + () => + new Promise( ( resolve ) => { + resolver = resolve; + } ) + ); + + render( ); + + // Search Input UI. + const searchInput = screen.getByRole( 'combobox', { name: 'URL' } ); + + // Simulate searching for a term. + await user.type( searchInput, 'Hello' ); + + // Wait for the spinner SVG icon to be rendered. + expect( await screen.findByRole( 'presentation' ) ).toBeVisible(); + // Check the suggestions list is not rendered yet. + expect( screen.queryByRole( 'listbox' ) ).not.toBeInTheDocument(); + + // Make the search suggestions fetch return a response. + resolver( fauxEntitySuggestions ); + + const resultsList = await screen.findByRole( 'listbox', { + name: 'Search results for "Hello"', + } ); + + // Check the suggestions list is rendered. + expect( resultsList ).toBeVisible(); + // Check the spinner SVG icon is not rendered any longer. + expect( screen.queryByRole( 'presentation' ) ).not.toBeInTheDocument(); + + const searchResultElements = + within( resultsList ).getAllByRole( 'option' ); + + expect( searchResultElements ).toHaveLength( + // The fauxEntitySuggestions length plus the 'Press ENTER to add this link' button. + fauxEntitySuggestions.length + 1 + ); + + // Step down into the search results, highlighting the first result item. + triggerArrowDown( searchInput ); + + const firstSearchSuggestion = searchResultElements[ 0 ]; + const secondSearchSuggestion = searchResultElements[ 1 ]; + + let selectedSearchResultElement = screen.getByRole( 'option', { + selected: true, + } ); + + // We should have highlighted the first item using the keyboard. + expect( selectedSearchResultElement ).toEqual( firstSearchSuggestion ); + + // Check the aria-selected attribute is set on the highlighted item. + expect( firstSearchSuggestion ).toHaveAttribute( + 'aria-selected', + 'true' + ); + // Check the aria-selected attribute is omitted on the non-highlighted items. + expect( secondSearchSuggestion ).not.toHaveAttribute( 'aria-selected' ); + + // Step down into the search results, highlighting the second result item. + triggerArrowDown( searchInput ); + + selectedSearchResultElement = screen.getByRole( 'option', { + selected: true, + } ); + + // We should have highlighted the first item using the keyboard. + expect( selectedSearchResultElement ).toEqual( secondSearchSuggestion ); + + // Check the aria-selected attribute is omitted on non-highlighted items. + expect( firstSearchSuggestion ).not.toHaveAttribute( 'aria-selected' ); + // Check the aria-selected attribute is set on the highlighted item. + expect( secondSearchSuggestion ).toHaveAttribute( + 'aria-selected', + 'true' + ); + + // Step up into the search results, highlighting the first result item. + triggerArrowUp( searchInput ); + + selectedSearchResultElement = screen.getByRole( 'option', { + selected: true, + } ); + + // We should be back to highlighting the first search result again. + expect( selectedSearchResultElement ).toEqual( firstSearchSuggestion ); + + // Check the aria-selected attribute is set on the highlighted item. + expect( firstSearchSuggestion ).toHaveAttribute( + 'aria-selected', + 'true' + ); + // Check the aria-selected attribute is omitted on non-highlighted items. + expect( secondSearchSuggestion ).not.toHaveAttribute( 'aria-selected' ); + } ); + it( 'should not render protocol in links', async () => { const user = userEvent.setup(); mockFetchSearchSuggestions.mockImplementation( () => @@ -570,7 +672,7 @@ describe( 'Manual link entry', () => { } ); // Verify the UI hasn't allowed submission. - expect( searchInput ).toBeInTheDocument(); + expect( searchInput ).toBeVisible(); expect( submitButton ).toBeDisabled(); expect( submitButton ).toBeVisible(); } @@ -612,7 +714,7 @@ describe( 'Manual link entry', () => { } ); // Verify the UI hasn't allowed submission. - expect( searchInput ).toBeInTheDocument(); + expect( searchInput ).toBeVisible(); expect( submitButton ).toBeDisabled(); expect( submitButton ).toBeVisible(); } @@ -1386,15 +1488,6 @@ describe( 'Selecting links', () => { firstSearchSuggestion ); - // Check aria-selected attribute is omitted on non-highlighted items. - expect( firstSearchSuggestion ).toHaveAttribute( - 'aria-selected', - 'true' - ); - expect( secondSearchSuggestion ).not.toHaveAttribute( - 'aria-selected' - ); - // Check we can go down again using the down arrow. triggerArrowDown( searchInput ); @@ -1407,15 +1500,6 @@ describe( 'Selecting links', () => { secondSearchSuggestion ); - // Check aria-selected attribute is omitted on non-highlighted items. - expect( firstSearchSuggestion ).not.toHaveAttribute( - 'aria-selected' - ); - expect( secondSearchSuggestion ).toHaveAttribute( - 'aria-selected', - 'true' - ); - // Check we can go back up via up arrow. triggerArrowUp( searchInput ); @@ -1428,15 +1512,6 @@ describe( 'Selecting links', () => { firstSearchSuggestion ); - // Check aria-selected attribute is omitted on non-highlighted items. - expect( firstSearchSuggestion ).toHaveAttribute( - 'aria-selected', - 'true' - ); - expect( secondSearchSuggestion ).not.toHaveAttribute( - 'aria-selected' - ); - expect( mockFetchSearchSuggestions ).toHaveBeenCalledTimes( 1 ); } ); } ); From aaa647dccea9d0752c5f95389c26464ef4175f15 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 1 Feb 2023 14:23:56 +0100 Subject: [PATCH 4/4] Add and clarify test comments. --- .../src/components/link-control/test/index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index fda223061a0f1f..a5c10665186953 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -147,6 +147,8 @@ describe( 'Basic rendering', () => { const searchInput = screen.getByRole( 'combobox', { name: 'URL' } ); expect( searchInput ).toBeVisible(); + // Make sure we use the ARIA 1.0 pattern with aria-owns. + // See https://github.com/WordPress/gutenberg/issues/47147 expect( searchInput ).not.toHaveAttribute( 'aria-controls' ); expect( searchInput ).toHaveAttribute( 'aria-owns' ); } ); @@ -208,7 +210,7 @@ describe( 'Basic rendering', () => { // We should have highlighted the first item using the keyboard. expect( selectedSearchResultElement ).toEqual( firstSearchSuggestion ); - // Check the aria-selected attribute is set on the highlighted item. + // Check the aria-selected attribute is set only on the highlighted item. expect( firstSearchSuggestion ).toHaveAttribute( 'aria-selected', 'true' @@ -228,7 +230,7 @@ describe( 'Basic rendering', () => { // Check the aria-selected attribute is omitted on non-highlighted items. expect( firstSearchSuggestion ).not.toHaveAttribute( 'aria-selected' ); - // Check the aria-selected attribute is set on the highlighted item. + // Check the aria-selected attribute is set only on the highlighted item. expect( secondSearchSuggestion ).toHaveAttribute( 'aria-selected', 'true' @@ -244,7 +246,7 @@ describe( 'Basic rendering', () => { // We should be back to highlighting the first search result again. expect( selectedSearchResultElement ).toEqual( firstSearchSuggestion ); - // Check the aria-selected attribute is set on the highlighted item. + // Check the aria-selected attribute is set only on the highlighted item. expect( firstSearchSuggestion ).toHaveAttribute( 'aria-selected', 'true'