Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 4 years ago

#37188 closed defect (bug) (fixed)

Add New Media button does not give aria feedback

Reported by: mantismamita's profile mantismamita Owned by: afercia's profile 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 8 years ago.
Upload Plugin behaviour
Screen Shot 2016-06-26 at 16.08.26.jpg (46.4 KB) - added by mantismamita 8 years ago.
Add New Media behaviour
patch.diff (1.3 KB) - added by mantismamita 8 years ago.
from upload.php
37188.diff (3.8 KB) - added by afercia 7 years ago.
37188.2.diff (5.2 KB) - added by afercia 7 years ago.
37188.3.diff (4.7 KB) - added by afercia 7 years ago.
37188.4.diff (4.9 KB) - added by afercia 7 years ago.
37188.5.diff (6.9 KB) - added by afercia 7 years ago.
37188.6.diff (8.3 KB) - added by afercia 7 years ago.

Download all attachments as: .zip

Change History (32)

@mantismamita
8 years ago

Upload Plugin behaviour

@mantismamita
8 years ago

Add New Media behaviour

#1 @afercia
8 years 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
8 years 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 8 years ago by mantismamita (previous) (diff)

#3 @afercia
8 years 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
8 years ago

from upload.php

#4 @afercia
7 years 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
7 years 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
7 years ago

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

@afercia
7 years ago

#7 @afercia
7 years ago

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

Screenshot with new patch applied:

https://cldup.com/tOVDSvna6y.png

#8 @afercia
7 years 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
7 years ago

#9 follow-up: @afercia
7 years 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
7 years ago

#10 @afercia
7 years ago

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

#11 @afercia
7 years 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.


7 years ago

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


7 years ago

#14 in reply to: ↑ 9 ; follow-up: @adamsilverstein
7 years 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 years 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 years ago

#16 @afercia
7 years 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 years ago

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


7 years ago

@afercia
7 years ago

#19 @afercia
7 years 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
7 years 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
7 years ago

#21 @afercia
7 years 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 7 years ago by afercia (previous) (diff)

#22 @afercia
7 years 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.

#23 @afercia
4 years ago

In 47217:

Accessibility: Media: Make the toolbar and inline uploader visual order always match the DOM order.

Depending on the specific media view, the media UI elements are placed in a
different visual order. This change makes sure their visual order always
matches the DOM order.

Propos syhc, audrasjb, afercia.
See #37188.
Fixes #48403.

Note: See TracTickets for help on using tickets.