WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 5 days ago

Last modified 5 days ago

#47145 closed defect (bug) (fixed)

Feature Image dialog does not follow the dialog pattern

Reported by: anevins Owned by: afercia
Milestone: 5.2.3 Priority: normal
Severity: major Version:
Component: Administration Keywords: has-screenshots wpcampus-report has-patch needs-testing fixed-major
Focuses: ui, accessibility Cc:

Description

Moved from the WPCampus accessibility report issues on GitHub, see https://github.com/WordPress/gutenberg/issues/15295

  • Severity:
    • High
  • Platform(s):
    • Windows - Screen Reader
    • Windows - ZoomText
    • Mac - VoiceOver
    • Android - TalkBack
    • iOS - VoiceOver
  • Components affected:
    • Media Dialog

Issue description
Users opening the "Feature Image" dialog are not told they are
entering a dialog. Keyboard focus is trapped, however screen reader
users are able to go beyond the last items of the dialog without
realising they have left it.

Keyboard users who find their focus cycling around the dialog get a Tab
stop with no visible focus. This is the dialog itself, which can receive
keyboard focus while having no visible focus state.

The triggering "Set Featured Image" button does not express an
expanded or collapsed state, which would not matter if users could not
reach the button when the modal was open; however, they can.

Lack of an explicit role for modal dialogs may be confusing for
assistive technology users, since they may not realise they're inside a
dialog, and that consequently the keyboard interactions may be different
from the rest of the page. Lack of an explicit label for dialogs may be
confusing for assistive technology users, since they won't have an
immediate sense of what the dialog is for.

Issue Code

    <button type="button" class="components-button editor-post-featured-image__toggle">Set featured image</button>


    ...


    <div tabindex="0" class="media-modal wp-core-ui">


        <button type="button" class="...">...Close media panel...</button>


        <div class="media-modal-content">


            ...


            <div class="media-frame-title">


                <h1>Featured Image<span class="dashicons dashicons-arrow-down"></span></h1>


            </div>


            ...


        </div>


    </div>

Remediation Guidance
Give the modal a role of dialog and set "aria-modal=true".
Additionally, set aria-hidden="true" on the element which wraps
the rest of the page, to prevent older browsers or assistive
technologies that don't support aria-modal from accessing the rest
of the page while the modal is open.

Use aria-labelledby and an id on the modal heading to name the
modal.

Change the tabindex to -1 so that it can be focused via
JavaScript without being in the Tab order. The exception to this advice
is if having the modal in the Tab order is necessary so that keyboard
users can scroll content inside it; however in existing cases,
scrollable areas are separate parts of the modal. But if a container
must receive focus in order to allow keyboard users to scroll the
content, then the container must visibly show focus.

Recommended Code

    <div id="wpwrap" aria-hidden="true">...</div>





    <div role="dialog" aria-modal="true" aria-labelledby="media-modal-title" tabindex="-1" class="media-modal wp-core-ui">


        <button type="button" class="...">...Close media panel...</button>


        <div class="media-modal-content">


            ...


            <div class="media-frame-title">


                <h1 id="media-modal-title">Featured Image<span class="dashicons dashicons-arrow-down"></span></h1>


            </div>


            ...


        </div>


    </div>

Relevant standards

  • 1.3.1 Info and Relationships (Level A)
  • 4.1.2 Name, Role, Value (Level A)

Note: This issue may be a duplicate with other existing accessibility-related bugs in this project. This issue comes from the Gutenberg accessibility audit, performed by Tenon and funded by WP Campus. This issue is GUT-42 in Tenon's report

Attachments (4)

57186885-b0666e80-6e9b-11e9-8d39-929e679a2cb0.png (97.9 KB) - added by anevins 4 months ago.
47145.diff (5.2 KB) - added by afercia 2 months ago.
media modal menu alignment.png (123.2 KB) - added by afercia 2 months ago.
Media modal left menu new vertical alignment
47145.2.diff (7.2 KB) - added by afercia 2 months ago.

Download all attachments as: .zip

Change History (24)

#1 @afercia
4 months ago

  • Component changed from Media to Administration

This applies to several modals in the admin. The media views use various modals. However, there are several other modals in WordPress that would need a solid accessibility treatment. Hopefully, also a standardized way to generate the modals.

See also #46985.

#2 @afercia
4 months ago

  • Milestone changed from Awaiting Review to 5.3

@afercia
2 months ago

#3 @afercia
2 months ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to afercia
  • Status changed from new to assigned

47145.diff is a first pass at implementing the ARIA dialog pattern.

It's important to note the media modal is used in different places in the admin and its content displays various "views", not only the one for the featured image. The media modal can also be used in the front end by themes and plugins authors.

Within the modal views there's some text content that is not associated to form controls. It's just text, often used as explanatory text. Using the role=dialog makes screen readers switch to "forms mode" and this textual content may be completely ignored. For this reason, the patch switches back the modal content to role=document, to ensure all content is available to assistive technologies.

The patch also moves the h1 heading before the menu in the source order. As the modal is now an ARIA dialog, the best practice is to have the h1 title be the first piece of content within the dialog.

To better communicate also visually that the h1 title is the first content in the modal, I'd like to propose to slightly move down the menu in the left sidebar. See attached screenshot.

The patch also makes sure to hide the page content, adding an aria-hidden attribute to the relevant body children. This part of the code is basically ported from the modal aria-helper implemented in Gutenberg.

Todo:

  • try to move this code to wp.media.view.FocusManager: it makes sense to have DOM related methods separated from the views code and FocusManager seems the most appropriate place
  • ensure the modal dialog has a default h1 title: will split this to a separate ticket

Some testing would be nice :)

@afercia
2 months ago

Media modal left menu new vertical alignment

#4 @afercia
2 months ago

ensure the modal dialog has a default h1 title: will split this to a separate ticket

See #47612.

@afercia
2 months ago

#5 @afercia
2 months ago

  • Keywords commit added

47145.2.diff moves the new functions to the media view FocusManager, cleans up thing, and improves documentation. Looks good to go to me.

Pending design feedback on the left menu vertical alignment (see previous screenshot).

This ticket was mentioned in Slack in #design by afercia. View the logs.


2 months ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


2 months ago

#8 @karmatosed
2 months ago

  • Keywords needs-design-feedback added

#9 @kjellr
2 months ago

  • Keywords needs-design-feedback removed

Hi @afercia! We discussed this issue in today's Design meeting:
https://wordpress.slack.com/archives/C02S78ZAL/p1561572554396000

It sounds like the reasoning for moving the left sidebar down is to:

To better communicate also visually that the h1 title is the first content in the modal, I’d like to propose to slightly move down the menu in the left sidebar.

While this does improve upon the problem, I think the root of the visual hierarchy issue is that the "Add Media" text in the left column reads more as a heading than as an active menu item. We already have styles elsewhere in WP-Admin for this sort of active menu item + heading relationship:

https://cldup.com/icwZ_7z7lG-3000x3000.png

To keep things consistent, I wonder if we might adopt something like that here? Here's a mockup for example:

https://cldup.com/thdmSzQ9Ro-3000x3000.png

This follows our existing patterns and also helps clarify which is the heading.

#10 @afercia
2 months ago

@kjellr and all the team, thanks for your feedback. I also read the log on Slack. Worth noting the left "menu" is going to be changed to ARIA tabs in #47149 and I'd tend to think that's the best ticket to discuss major visual changes to the "menu".

Also, in #47610 I'm proposing to add more headings within the modal. That's also a ticket open to broader design considerations.

I'm not opposed to major styling changes, which are out of the scope of this ticket though. For now, I'd just like to move down the "menu' because:

  • the main H1 heading is now the first thing in the source order
  • as such, the visual order needs to match the source order and keeping the "menu" at the top wouldn't meet this requirement

References:
1.3.2 Meaningful Sequence (Level A) https://www.w3.org/TR/WCAG21/#meaningful-sequence
2.4.3 Focus Order (Level A) https://www.w3.org/TR/WCAG21/#focus-order

#11 @kjellr
2 months ago

Thanks for the background, @afercia! Moving the left menu down like you have it here helps a bit for now, and we can address broader changes in #47610.

#12 @afercia
2 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 45572:

Accessibility: Make the Media modal an ARIA modal dialog.

For a number of years, the Media modal missed an explicit ARIA role and the required attributes for modal dialogs.

This was confusing for assistive technology users, since they may not realize they're inside a dialog, and that consequently the keyboard interactions may be different from the rest of the page. Lack of an explicit label for the dialog was confusing as well, since assistive technology users didn't have an immediate sense of what the dialog is for.

This change makes the Media modal meet the ARIA Authoring Practices recommendations, helping users better understand the purpose and interactions with the modal. Also, it makes sure to hide the rest of the page content from assistive technologies, until support for aria-modal="true" improves.

Additionally:

  • moves the modal H1 heading to the beginning of the modal content
  • changes the modal left menu position to make visual and DOM order match
  • improves the wp.media.view.FocusManager documentation

Fixes #47145.

#13 @afercia
2 months ago

  • Keywords commit removed

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


13 days ago

#15 @JeffPaul
9 days ago

  • Keywords needs-testing fixed-major added
  • Milestone changed from 5.3 to 5.2.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this so it can be back-ported to the 5.2 branch, also needs testing to validate if this ticket is good to land in 5.2.3.

This ticket was mentioned in Slack in #core by marybaum. View the logs.


5 days ago

#17 @SergeyBiryukov
5 days ago

[45572] doesn't cleanly apply to the 5.2 branch due to some earlier changes in [45524], but the merge conflicts seem trivial to resolve.

#18 @SergeyBiryukov
5 days ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 45866:

Accessibility: Make the Media modal an ARIA modal dialog.

For a number of years, the Media modal missed an explicit ARIA role and the required attributes for modal dialogs.

This was confusing for assistive technology users, since they may not realize they're inside a dialog, and that consequently the keyboard interactions may be different from the rest of the page. Lack of an explicit label for the dialog was confusing as well, since assistive technology users didn't have an immediate sense of what the dialog is for.

This change makes the Media modal meet the ARIA Authoring Practices recommendations, helping users better understand the purpose and interactions with the modal. Also, it makes sure to hide the rest of the page content from assistive technologies, until support for aria-modal="true" improves.

Additionally:

  • moves the modal H1 heading to the beginning of the modal content
  • changes the modal left menu position to make visual and DOM order match
  • improves the wp.media.view.FocusManager documentation

Props afercia.
Merges [45572] to the 5.2 branch.
Fixes #47145.

#19 @SergeyBiryukov
5 days ago

In 45867:

Docs: Update @since tag for new JS functions and variables introduced in [45572].

Props garrett-eclipse.
See #47145.

#20 @SergeyBiryukov
5 days ago

In 45868:

Docs: Update @since tag for new JS functions and variables introduced in [45572].

Props garrett-eclipse.
Merges [45867] to the 5.2 branch.
See #47145.

Note: See TracTickets for help on using tickets.