WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#28864 closed defect (bug) (fixed)

Cannot access edit menu options with keyboard inside Image Editor

Reported by: davidakennedy Owned by: afercia
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords: has-patch commit
Focuses: ui, accessibility, javascript Cc:

Description

What happened: When I visit /wp-admin/upload.php?item=0000 and navigate to the Edit image panel with a keyboard only, I cannot access the items inside .imgedit-menu These are the rotate, crop buttons, etc.

What I expected: I would expect to be able to focus on those items, visibly see that they have focus and operate the items.

Steps to reproduce: 1. /wp-admin/upload.php?item=0000 2. Navigate into the Edit image tab by pressing the tab key. Press enter. 3. Try to tab to the items inside .imgedit-menu

Thoughts/Solutions: Each of the items are divs. Divs are not naturally focusable by the keyboard. These should be real buttons or links, preferably buttons.

Attachments (9)

28864.patch (19.4 KB) - added by afercia 5 years ago.
28864.2.patch (20.0 KB) - added by afercia 5 years ago.
Handle focus after Scale and Restore
28864.3.patch (20.5 KB) - added by afercia 5 years ago.
Refreshed patch: cross browser input line-height
28864.4.patch (24.7 KB) - added by afercia 5 years ago.
handles buttons disabled state and focus
28864.5.patch (24.4 KB) - added by afercia 5 years ago.
removes some debugging stuff
28864.6.patch (25.9 KB) - added by afercia 4 years ago.
ensure the undo button is enabled before moving focus
28864.7.patch (28.7 KB) - added by afercia 4 years ago.
28864.8.patch (30.2 KB) - added by afercia 4 years ago.
28864.9.patch (30.1 KB) - added by afercia 4 years ago.

Download all attachments as: .zip

Change History (34)

#1 @ocean90
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Cannot access edit menu options with keyboard inside media item modal to Cannot access edit menu options with keyboard inside Image Editor

The issue exists for each screen, where wp_image_editor() is used.

#2 @afercia
5 years ago

  • Focuses ui added
  • Keywords has-patch needs-testing added; needs-patch removed

First try to improve image editor accessibility. Please notice in the media views there's not even a real form element (unless I'm missing something), so just tried to make the fields and controls a bit more semantic.
In the proposed patch:

  • the main buttons are now buttons
  • added some fieldsets and legends to group related controls, screen readers are happy
  • added some missing labels
  • the help toggles are now keyboard accessible and use aria-expanded attribute

Still to address:

  • manage focus after "Scale" button gets activated
  • same for "Restore image"
Last edited 5 years ago by afercia (previous) (diff)

@afercia
5 years ago

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


5 years ago

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


5 years ago

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


5 years ago

#6 @rianrietveld
5 years ago

Tested 28864.patch with keyboard only: all buttons get focus, works good.

The only thing not working good: after clicking the "scale" button the focus drops out of the panel.

Last edited 5 years ago by rianrietveld (previous) (diff)

@afercia
5 years ago

Handle focus after Scale and Restore

#7 @afercia
5 years ago

Refreshed patch restores focus on the "Scale" button after the frame gets refreshed on "Scale" and "Restore" actions.

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

@afercia
5 years ago

Refreshed patch: cross browser input line-height

#8 @afercia
5 years ago

Refreshed patch with better cross-browser line-height for input fields and minor CSS refinements.

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


5 years ago

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


5 years ago

#11 @rianrietveld
5 years ago

Tested patch 28864.3.patch with keyboard only, works, tabbing forward and backward also keeps the focus inside the media-model, so that's good too.

#12 @afercia
5 years ago

@todo

  • disabled buttons should be really disabled (and not focusable), not just "look" disabled
  • new ticket to propose research and development for a new "inline help system"

#13 @florianziegler
5 years ago

As discussed, we should probably use disabled="disabled" to make the buttons "not focusable".

Imho, this makes sense from a semantic perspective, as well.

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


5 years ago

@afercia
5 years ago

handles buttons disabled state and focus

#15 @afercia
5 years ago

  • Focuses javascript added
  • Keywords dev-feedback added; needs-testing removed

Updated patch handles the buttons disabled state and tries to manage focus to avoid it gets lost when buttons gets disabled (e.g. undo, redo, crop). Would recommend some devs with strong JavaScript-fu to review the focus management part.

@afercia
5 years ago

removes some debugging stuff

#16 @afercia
5 years ago

Please discard patch 4, forgot to clean out some debugging stuff.

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


5 years ago

@afercia
4 years ago

ensure the undo button is enabled before moving focus

#18 @afercia
4 years ago

Refreshed patch to be sure the undo button is enabled before moving focus on it, plus some small CSS refinements mainly for input fields and buttons. Open to discuss about those CSS changes if required.

Please notice the responsive mode of the image editor isn't optimal and is inconsistent, in one place based on floats, in other two places based on absolute positioning. I'd propose to create a new ticket for that, any thoughts more than welcome.

@afercia
4 years ago

#19 @afercia
4 years ago

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

Refreshed patch. I would love to see this in WordPress 4.5. Lots of small changes here so maybe now would be the perfect time for commit consideration since we're at the beginning of a new release cycle.

Not saying it's perfect, but it is an improvement. Further accessibility improvements should go in separate tickets, for example:

  • consider to add feedback/confirmation messages using wp.a11y.speak()
  • make the crop selection keyboard accessible

Some testing would be nice :)

Related: #26396.

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


4 years ago

@afercia
4 years ago

#21 @afercia
4 years ago

  • Keywords commit added; dev-feedback removed

Refreshed patch after [36162]. Also, some cleanup and improvements.

Worth reminding this ticket focuses only on keyboard accessibility. The image editor would need further improvements, for example audible confirmation messages with wp.a11y.speak() after image edits complete successfully, etc. Also, the (lack of) responsive view could be improved.

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


4 years ago

@afercia
4 years ago

#23 @afercia
4 years ago

Refreshed patch after [36171].

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


4 years ago

#25 @afercia
4 years ago

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

In 36223:

Media: make the Image Editor usable with a keyboard.

For accessibility, all interactive controls must be operable from the keyboard.
Replaces <div>s used as UI controls with buttons. Groups some logically-related
form elements.

Fixes #28864.

Note: See TracTickets for help on using tickets.