WordPress.org

Make WordPress Core

Opened 14 months ago

Closed 5 months ago

#37188 closed defect (bug) (fixed)

Add New Media button does not give aria feedback

Reported by: mantismamita Owned by: afercia
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords: has-ui-feedback has-screenshots has-patch commit
Focuses: ui, accessibility Cc:

Description

Button Upload Plugin and Upload Theme both have a role of button and aria-expanded=true onclick. The expanded panel then directly under it which is expected behaviour for screen-readers and is semantically logical. (see screen-shot)

On the other hand, the Add New button on the Media Page is a link without a role of button and there is no aria feedback provided. The expanded panel is then followed by the navigation bar rather than the expanded panel.

Attachments (9)

Screen Shot 2016-06-26 at 16.06.42.jpg (40.0 KB) - added by mantismamita 14 months ago.
Upload Plugin behaviour
Screen Shot 2016-06-26 at 16.08.26.jpg (46.4 KB) - added by mantismamita 14 months ago.
Add New Media behaviour
patch.diff (1.3 KB) - added by mantismamita 14 months ago.
from upload.php
37188.diff (3.8 KB) - added by afercia 8 months ago.
37188.2.diff (5.2 KB) - added by afercia 8 months ago.
37188.3.diff (4.7 KB) - added by afercia 8 months ago.
37188.4.diff (4.9 KB) - added by afercia 7 months ago.
37188.5.diff (6.9 KB) - added by afercia 6 months ago.
37188.6.diff (8.3 KB) - added by afercia 5 months ago.

Download all attachments as: .zip

Change History (31)

@mantismamita
14 months ago

Upload Plugin behaviour

@mantismamita
14 months ago

Add New Media behaviour

#1 @afercia
14 months ago

  • Focuses ui added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from trunk to 4.0

Thanks very much for opening this ticket @mantismamita and for your contribution at Vienna's Contributor Day :) I think a first good step would be using the CSS class aria-button-if-js on the link in order to add a role="button" when JavaScript is on (you can see what it does in common.js). Worth reminding this link should remain a link because when JavaScript is off it points to media-new.php

Setting the version to 4.0 as that should be the version the media grid and the uploader panel were introduced.

#2 @mantismamita
14 months ago

Hello @afercia, I was using the hide-if-no-js class and adding the aria-expanded=false but will try your way. So does this mean we are patching 4.0 ? I'm not sure I understand. I didn't know that was possible ;) I guess I shouldn't submit the patch I had going as it was using trunk?

Last edited 14 months ago by mantismamita (previous) (diff)

#3 @afercia
14 months ago

Hi @mantismamita. It's just a convention on Trac to set the version number to indicate the WordPress version the "bug" was first introduced :) Patches should always be built from trunk of the develop repo which means you should check out WordPress trunk or if you're using VVV from www/wordpress-develop .

@mantismamita
14 months ago

from upload.php

#4 @afercia
11 months ago

  • Keywords ui-feedback added

For better accessibility, and consistency with other screens (Plugins, Themes), the drop zone should immediately follow the toggle button and be placed before the media toolbar, see screenshot below. Would greatly appreciate a quick feedback from the UI team :)

https://cldup.com/URlgMXmfWe.png

#5 @karmatosed
10 months ago

  • Keywords has-ui-feedback added; ui-feedback removed

I think moving the dropzone up makes a lot of sense. You are clicking 'add' and having it appear below another UI element is a weird experience. For reference here is what we currently have and shows the UX hitch.

https://cldup.com/0iudSZn_RT.gif

#6 @afercia
8 months ago

  • Milestone changed from Future Release to 4.8
  • Owner set to afercia
  • Status changed from new to assigned

@afercia
8 months ago

#7 @afercia
8 months ago

  • Keywords has-screenshots has-patch commit added; needs-patch removed

Screenshot with new patch applied:

https://cldup.com/tOVDSvna6y.png

#8 @afercia
8 months ago

  • Keywords needs-refresh added; commit removed

The approach used in 37188.diff won't work correctly when clicking on the "X" button to close the uploader. Needs a refresh with a new approach, probably using the event toggle:upload:attachment.

@afercia
8 months ago

#9 follow-up: @afercia
8 months ago

  • Keywords needs-refresh removed

37188.2.diff uses a new approach and stores the Add New button as a property of the frame. Not sure why this event was delegated, since the .page-title-action element is always printed out in the page. Maybe I'm missing something, would greatly appreciate some feedback /cc @wonderboymusic @ericlewis @adamsilverstein 🙂

@afercia
8 months ago

#10 @afercia
8 months ago

37188.3.diff removes unrelated change to wp-includes/class-wp-editor.php.

#11 @afercia
8 months ago

Screenshot for the responsive view, before and after:

https://cldup.com/9V0D5TFVsO.png

Reminder: the "Drop files anywhere to upload" instruction gets hidden when Plupload detects there's no support for drag and drop.

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


8 months ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


7 months ago

#14 in reply to: ↑ 9 ; follow-up: @adamsilverstein
7 months ago

Replying to afercia:

37188.2.diff uses a new approach and stores the Add New button as a property of the frame. Not sure why this event was delegated, since the .page-title-action element is always printed out in the page.

The only reason I could think this would be delegated is if the button could sometimes be added or the event was needed after the script ran, but I don't see that happening here.

I tested the patch and this looks good; everything worked well in the media library, however when I tried reaching click "Add Media" or "Set Featured Image" on the post edit page, I got a JavaScript error:
Uncaught TypeError: Cannot read property 'attr' of undefined likely because there isn't a .page-title-action element on the page.

#15 in reply to: ↑ 14 @afercia
7 months ago

  • Keywords needs-refresh added

Replying to adamsilverstein:

I tested the patch and this looks good; everything worked well in the media library, however when I tried reaching click "Add Media" or "Set Featured Image" on the post edit page, I got a JavaScript error

Yep, confirmed. I think the controller is not the same as in the Library.

@afercia
7 months ago

#16 @afercia
7 months ago

  • Keywords needs-refresh removed

37188.4.diff just checks for this.controller.$uploaderToggler before trying to use it.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


7 months ago

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


6 months ago

@afercia
6 months ago

#19 @afercia
6 months ago

37188.5.diff refreshes the patch after recent changes, also adds some inline comments.

Screenshot before the patch: errors and uploader displayed after the toolbar:

https://cldup.com/LlpVv_aZWC.png

Screenshot with the patch applied: errors and uploader displayed before the toolbar:

https://cldup.com/apyRPtlqC9.png

#20 @afercia
5 months ago

Couple things still to do:

  • the uploader close "X" button misses a focus style so on webkit on Mac it displays the native focus style and on Firefox no style at all (see screenshot below)
  • when closing the uploader, keyboard focus should be moved back to the "Add New" toggle button

https://cldup.com/Yu2j0jfs0t.png

@afercia
5 months ago

#21 @afercia
5 months ago

  • Keywords commit added

37188.6.diff adds the missing focus style and improves the color contrast of the "X" close button. Also improves a bit the JS part.

Last edited 5 months ago by afercia (previous) (diff)

#22 @afercia
5 months ago

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

In 40359:

Accessibility: Improve the Media Library inline uploader accessibility.

For better accessibility, expandable panels should be placed immediately after
the control that expands them. This change moves the Media Library inline
uploader up, right after the "Add New" button, also introducing consistency with
the Plugin and Theme uploaders.
Adds a proper ARIA role on the button and an aria-expanded attribute to give
better feedback to assistive technologies users about the uploader's expanded state.
Improves the focus handling when closing the uploader, improves the focus style
and color contrast ratio of the uploader "close" button.

Props mantismamita, karmatosed, adamsilverstein, afercia.
Fixes #37188.

Note: See TracTickets for help on using tickets.