Skip to content

Autocomplete: Fix Voiceover not announcing suggestions #54902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexstine merged 19 commits into from
Oct 19, 2023
Merged

Autocomplete: Fix Voiceover not announcing suggestions #54902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
766855b
Add a function to return a text-only label.
alexstine Sep 28, 2023
3c82605
Use already defined isAppleOS function from keycodes. Remove new OS f…
alexstine Sep 28, 2023
700bb1c
Add fallback if textLabel is unavailable.
alexstine Sep 28, 2023
a6b45ab
Try combobox role to get around annoying re-rendering type effect.
alexstine Sep 29, 2023
98bbc1f
Merge branch 'trunk' of github.com:WordPress/gutenberg into fix/autoc…
alexstine Oct 2, 2023
b43d95e
Changelog entry.
alexstine Oct 2, 2023
6372f1c
Code quality and refresh.
alexstine Oct 10, 2023
96464c6
Merge branch 'trunk' of github.com:WordPress/gutenberg into fix/autoc…
alexstine Oct 14, 2023
7ecf361
Merge branch 'trunk' of github.com:WordPress/gutenberg into fix/autoc…
alexstine Oct 18, 2023
7972225
Revert Windows fix, too much scope creep.
alexstine Oct 18, 2023
7681b88
Fix Changelog.
alexstine Oct 18, 2023
1e24a70
Remove diff artifact.
alexstine Oct 18, 2023
facbe98
Fix conflict.
alexstine Oct 19, 2023
8e7ba56
Merge branch 'fix/autocomplete-voiceover' of github.com:WordPress/gut…
alexstine Oct 19, 2023
307b78e
Merge branch 'trunk' into fix/autocomplete-voiceover
alexstine Oct 19, 2023
98a845b
Remove mistaken files.
alexstine Oct 19, 2023
6e91679
Add comment linking to PR.
alexstine Oct 19, 2023
fe36a90
Revert textLabel prop.
alexstine Oct 19, 2023
607c455
Merge branch 'trunk' of github.com:WordPress/gutenberg into fix/autoc…
alexstine Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Bug Fix

- `Autocomplete`: Add `aria-live` announcements for Mac and IOS Voiceover to fix lack of support for `aria-owns` ([#54902](https://github.com/WordPress/gutenberg/pull/54902)).

## 25.10.0 (2023-10-18)

### Enhancements
Expand Down
62 changes: 54 additions & 8 deletions packages/components/src/autocomplete/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
isCollapsed,
getTextContent,
} from '@wordpress/rich-text';
import { speak } from '@wordpress/a11y';
import { isAppleOS } from '@wordpress/keycodes';

/**
* Internal dependencies
Expand All @@ -39,6 +41,35 @@ import type {
WPCompleter,
} from './types';

const getNodeText = ( node: React.ReactNode ): string => {
Copy link
Contributor

@jeryj jeryj Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is only used once, but it would be useful for it to be its own exported function so we can have some unit tests for it. Given the release is coming up so quickly, I think an extra day of manual testing it "in the wild" is more important than delaying this for unit tests.

if ( node === null ) {
return '';
}

switch ( typeof node ) {
case 'string':
case 'number':
return node.toString();
break;
case 'boolean':
return '';
break;
case 'object': {
if ( node instanceof Array ) {
return node.map( getNodeText ).join( '' );
}
if ( 'props' in node ) {
return getNodeText( node.props.children );
}
break;
}
default:
return '';
}

return '';
Comment on lines +49 to +70
Copy link
Contributor

@jeryj jeryj Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the breaks that are preceded by a return since that code won't ever be run. The final return can be removed as well since the default will get hit first, I believe. Thanks for @ajlende for catching that!

We can do that after this is merged though. I'll open an issue about it.

};
Comment on lines +44 to +71
Copy link
Contributor

@ajlende ajlende Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could simplify a bit.

Suggested change
const getNodeText = ( node: React.ReactNode ): string => {
if ( node === null ) {
return '';
}
switch ( typeof node ) {
case 'string':
case 'number':
return node.toString();
break;
case 'boolean':
return '';
break;
case 'object': {
if ( node instanceof Array ) {
return node.map( getNodeText ).join( '' );
}
if ( 'props' in node ) {
return getNodeText( node.props.children );
}
break;
}
default:
return '';
}
return '';
};
const getNodeText = ( node: React.ReactNode ): string => {
if ( node === null ) {
return '';
}
switch ( typeof node ) {
case 'string':
case 'number':
return node.toString();
case 'object': {
if ( node instanceof Array ) {
return node.map( getNodeText ).join( '' );
}
if ( 'props' in node ) {
return getNodeText( node.props.children );
}
return '';
}
default:
return '';
}
};


const EMPTY_FILTERED_OPTIONS: KeyedOption[] = [];

export function useAutocomplete( {
Expand Down Expand Up @@ -163,20 +194,35 @@ export function useAutocomplete( {
) {
return;
}

switch ( event.key ) {
case 'ArrowUp':
setSelectedIndex(
case 'ArrowUp': {
const newIndex =
( selectedIndex === 0
? filteredOptions.length
: selectedIndex ) - 1
);
: selectedIndex ) - 1;
setSelectedIndex( newIndex );
// See the related PR as to why this is necessary: https://github.com/WordPress/gutenberg/pull/54902.
if ( isAppleOS() ) {
speak(
getNodeText( filteredOptions[ newIndex ].label ),
'assertive'
);
}
break;
}

case 'ArrowDown':
setSelectedIndex(
( selectedIndex + 1 ) % filteredOptions.length
);
case 'ArrowDown': {
const newIndex = ( selectedIndex + 1 ) % filteredOptions.length;
setSelectedIndex( newIndex );
if ( isAppleOS() ) {
Copy link
Contributor

@jeryj jeryj Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful to have a link to this PR and a brief comment explaining why we need to use speak on appleOS. Let's do it in a follow-up though, so we don't need to retrigger all the tests since they're currently passing.

speak(
getNodeText( filteredOptions[ newIndex ].label ),
'assertive'
);
}
break;
}

case 'Escape':
setAutocompleter( null );
Expand Down