Skip to content
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.

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

Playground block: A11Y: Use correct ARIA specification for autocomplete #289

Closed
alexstine opened this issue Jun 4, 2024 · 9 comments · Fixed by #317
Closed

Playground block: A11Y: Use correct ARIA specification for autocomplete #289

alexstine opened this issue Jun 4, 2024 · 9 comments · Fixed by #317
Assignees
Labels

Comments

@alexstine
Copy link

Test page: https://learn.wordpress.org/test/wordpress-playground-block-plugin-test-page/

Currently, the aria-autocomplete="list" value is set for the code editor. According to specification, if this is set, a few different attributes also need to be set.

Docs: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-autocomplete

I find it odd that autocomplete="off" is set as well. Does the editor currently support suggestions? If so, please modify the ARIA attributes to correctly convey to assistive tech what is happening.

@brandonpayton
Copy link
Member

Some notes after investigating:
I don't see any indication that we are actually using any autocomplete features.

It looks like the aria-autocomplete="list" attribute is added by the third-party CodeMirror component.

Based on this comment in the CodeMirror forums, there does not appear to be a way of removing autocomplete as a basic CodeMirror component without forking the project:
https://discuss.codemirror.net/t/removing-the-autocomplete-from-basicsetup-for-library/6523/2

In addition, the codemirror/autocomplete component appears to always set aria-autocomplete=list and sets other ARIA attributes when displaying a completion dialog:
https://github.com/codemirror/autocomplete/blob/099b9fe7a073a6d09a9a46ca7f518ce5c3278a48/src/state.ts#L171-L183

Based on the MDN docs for aria-autocomplete that were mentioned above, it looks like maybe this is OK as long as no autocomplete popups are actually displayed. Is that correct, @alexstine? Is this possibly something we can live with?

Regarding autocomplete="off" being set as well, I haven't been able to find that but did notice there is a similar autocorrect="off" attribute.

@brandonpayton
Copy link
Member

Based on the MDN docs for aria-autocomplete that were mentioned above, it looks like maybe this is OK as long as no autocomplete popups are actually displayed. Is that correct, @alexstine? Is this possibly something we can live with?

I wish I would have asked this differently. Rather than, "Is this possibly something we can live with?", I would like to ask:
What are the impacts for those using assistive technologies if the aria-autocomplete="list" attribute remains without additional annotations?

@brandonpayton brandonpayton added the Bug Something isn't working label Jun 13, 2024
@alexstine
Copy link
Author

@brandonpayton It adds confusion for users because the field is announcing an autocomplete option that doesn't exist. Imagine if the form had a message above it for sighted users that said something like this.

The below editor supports autocomplete on type.

They would be maybe a little surprised if no autocomplete/suggestions list ever appeared.

I do believe this likely fails: https://www.w3.org/WAI/WCAG22/Techniques/html/H88

Reasoning being we're telling users the input does something that it does not.

I would consider this impact enough to wish we caught this before using this 3rd-party editor. Maybe open an issue upstream and see if they'll add an option? Any way to just remove it with additional JS?

Thanks.

@brandonpayton
Copy link
Member

@alexstine thank you for talking this out with me.

I was thinking we probably could remove the attribute manually with additional JS as you suggested, but it turns out that the editor does actually show autocomplete suggestions for some programming languages or file formats. For example, autocomplete suggestions are shown for HTML files. So as the user switches between file tabs, the editor may or may not provide autocomplete depending on file type.

In that case, would it be better for the aria-autocomplete attribute to be dynamically added or removed depending on file type, or would it be better if it just remains in place to acknowledge the ever present possibility that autocomplete may be supported for one of the file types?

Dynamically adding and removing the attribute may be challenging if we cannot directly detect whether autocomplete is truly supported for a file, but I want to at least understand if it is the preferred compromise for this case.

@brandonpayton
Copy link
Member

I was thinking we probably could remove the attribute manually with additional JS as you suggested, but it turns out that the editor does actually show autocomplete suggestions for some programming languages or file formats. For example, autocomplete suggestions are shown for HTML files.

On more thing to add:
When showing the autocomplete popup, the editor does add the following attributes in addition to the aria-autocomplete="list" attribute:

  • aria-controls
  • aria-haspopup
  • aria-activedescendent

For the popup element itself, the editor adds the aria-expanded="true" attribute to an element with role="listbox". This varies slightly from the MDN documentation which mentions using role="combobox".

@alexstine
Copy link
Author

@brandonpayton Yes, it should only be added if the field is actually an autocomplete field and for some files, it is not.

@brandonpayton
Copy link
Member

I asked in the CodeMirror forums about detecting the absence of autocomplete providers and about only adding aria-autocomplete="list" when there are autcompletion providers loaded:
https://discuss.codemirror.net/t/avoiding-aria-autocomplete-list-when-no-autocompletion-providers/8313

@brandonpayton
Copy link
Member

The maintainer of CodeMirror kindly made a fix for this here:
codemirror/autocomplete@3f6e41d

The fix isn't published as part of a new version yet, but it tests well locally. With the fix, aria-autocomplete="list" is only added when there are completion sources registered for the editor. When editing an unknown file type like index.xyz, the attribute is not added, but it is added for things like JS files where autocomplete sources exist.

Let's leave this issue open until we upgrade to a @codemirror/autocomplete version with this fix.

brandonpayton added a commit that referenced this issue Jun 27, 2024
## What?

This PR updates CodeMirror dependencies to include a recent
accessibility-related autocomplete fix:

https://discuss.codemirror.net/t/avoiding-aria-autocomplete-list-when-no-autocompletion-providers/8313/2

## Why?

Fixes #289 - "Playground block: A11Y: Use correct ARIA specification for
autocomplete"

## Implementation

This PR updates npm dependencies for CodeMirror. The Playground block
appears to work well with the update.

The interactive-code-block doesn't render well in the editor either
before or after these updates. (Is it still in use anywhere, and if not,
can we delete it? cc @adamziel)

## Testing Instructions

- Run `npm install` update dependencies
- Run: `npx nx dev wordpress-playground-block` to build the block and
start a dev server
- Open the editor and create a post containing the WordPress Playground
block
- Leave "index.php" in place, and add a file type like "test.xyz" that
is not a file type with autocomplete support
- Publish the post and view on the front end
- Open the browser dev tools
- Select "index.php", run `$$('[aria-autocomplete]')`, and confirm the
query returns an element
- Select "test.xyz", run `$$('[aria-autocomplete]')`, and confirm the
query returns an empty list
@brandonpayton brandonpayton reopened this Jun 27, 2024
@alexstine
Copy link
Author

Confirmed fixed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants