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

Fix accessibility of the Classic Edit modal dialog #50161

Closed
afercia opened this issue Apr 28, 2023 · 1 comment · Fixed by #50384
Closed

Fix accessibility of the Classic Edit modal dialog #50161

afercia opened this issue Apr 28, 2023 · 1 comment · Fixed by #50384
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block library /packages/block-library [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Apr 28, 2023

Description

Since a few Gutenberg releases, the Classic Edit block opens in a modal dialog. The modal dialog lacks some important accessibility requirements:

1
The modal dialog is unlabeled.
There is an aria-labelledby="components-modal-header-0" attribute but it points to nothing. There have been quite a few unlabeled modal dialogs so far and this is one more instance. This can be fixed on a per-instance basis but, as pointed out in #47885, it would be best to fix the component in the first place to enforce a proper labeling.

2
The 'Classic Edit' heading contains extraneous content.
The 'Cancel' and 'Save' button are placed within the heading. A heading should be a heading and only contain a meaningful title. Placing buttons within a heading isn't that great. The actual heading text becomes:
Classic Edit Cancel Save
which is arguably meaningful, if not confusing. See screenshot.

Screenshot 2023-04-28 at 09 25 40

3
Minor: <div> elements within a heading is invalid HTML.

4
The modal dialog heading should be a H1.
When the modal dialog component opens, we do hide from assistive technology all the rest of the content of the page by the means of aria-hidden="true". Worth noting the aria-modal="true" would be more correct but at the time the component was implemented, aria-modal was buggy. Regardless, the modal dialog content is the only content that is perceived by assistive technology. As such, it's best to start the headings hierarchy withih the dialog from scratch by using a H1.

5
Tabbing is not constrained within the modal.
I'm guessing TinyMCE conflicts in some way with the builti-in focus management of the modal dialog component.

Step-by-step reproduction instructions

  • Make sure your active theme is a block-based theme e.g. Twenty Twenty-Three.
  • Edit a post.
  • Add a Classic block.
  • A modal dialog opens.
  • In the Chrome dev tools:
    • Inspect the source and select the element with role="dialog".
    • On the right of the dev tools, switch to the Accessibility tab.
    • Observe the accessible name of the dialog is an empty string.
    • Observe the value of the aria-labelledby attribute is marked as 'invalid source', meaning there's no element with that ID.
    • Screenshot:

Screenshot 2023-04-28 at 10 45 14

  • In the dev tools inspector, select the modal dialog heading.
  • Observe it contains the two buttons 'Cancel' and 'Save'.
  • In the dev tools Accessibility tab, observe the heading accessible name is Classic Edit Cancel Save.
  • As such, screen readers announce the heading as Classic Edit Cancel Save. Screenshot:

Screenshot 2023-04-28 at 09 25 40

  • Observe the heading is a H2. It should be a H1.
  • With the modal dialog open, use the Tab key to navigate through the focusable elements.
  • Observe that tabbing is not constrained within the modal dialog: when tabbing away from the editable Rich Text area, focus goes to the browser chrome.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Package] Block library /packages/block-library labels Apr 28, 2023
@afercia afercia self-assigned this Apr 28, 2023
@afercia
Copy link
Contributor Author

afercia commented Apr 28, 2023

Tabbing is not constrained within the modal.

The editable Rich Text area is within an iframe. useConstrainedTabbing can't work properly because the node it is set to lives in the main document while the iframe is in another document. Instead of trying to fix this by over-complicating useConstrainedTabbing, I'd propose to just move the Cancel and Save buttons after the Rich Text area, which is also consistent with the placement of similar buttons in other modal dialogs e.g. the Lock block modal dialog.

@afercia afercia changed the title FIx accessibility of the Classic Edit modal dialog Fix accessibility of the Classic Edit modal dialog May 2, 2023
@ciampo ciampo moved this to Todo in Andrew's onboarding May 3, 2023
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 5, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block library /packages/block-library [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants