WordPress.org

Make WordPress Core

#26527 closed defect (bug) (fixed)

wp-admin/themes.php isn't keyboard navigation friendly

Reported by: jorbin Owned by: nacin
Milestone: 3.8.1 Priority: high
Severity: normal Version: 3.8
Component: Accessibility Keywords: has-patch commit fixed-major
Focuses: Cc:

Description

Since the links are hidden unless you hovering over screenshot, you often have no idea where you are on the page when navigating via keyboard.

Additionally, 'Theme Details' isn't an actual link so it never get's focus (and thus is nearly impossible to click on).

This is a pretty big regression from 3.7 when the page was very keyboard friendly.

Attachments (10)

26527.diff (3.1 KB) - added by jorbin 18 months ago.
26527.2.diff (2.4 KB) - added by lancewillett 18 months ago.
Use real href values, spaces to tabs for indentation, and simpler preventDefault
26527.3.diff (7.4 KB) - added by azaozz 18 months ago.
26527.4.diff (6.4 KB) - added by matveb 18 months ago.
26527.5.diff (6.4 KB) - added by matveb 18 months ago.
26527.6.diff (6.8 KB) - added by matveb 18 months ago.
26527.6.2.diff (6.8 KB) - added by matveb 18 months ago.
26527.7.diff (6.8 KB) - added by matveb 18 months ago.
26527.8.diff (6.8 KB) - added by matveb 18 months ago.
26527.9.diff (7.1 KB) - added by matveb 18 months ago.

Download all attachments as: .zip

Change History (35)

comment:1 @SergeyBiryukov18 months ago

  • Component changed from Themes to Accessibility
  • Milestone changed from Awaiting Review to 3.8

@jorbin18 months ago

comment:2 @jorbin18 months ago

I've added an initial patch that tries to jam keyboard navigation in there. I've only tested this in chrome stable and canary thus far.

The reason I have the preventDefault is that if the anchor tag lacks an href or has an empty, it won't get focus in some browsers.

The reason for the hacky hack for styling is that the opacity is on the parent element, and I have no idea how to style a parent element when the child has focus.

Very open to alternative solutions if others have them.

comment:3 @nacin18 months ago

  • Priority changed from normal to high
  • Severity changed from normal to critical

@lancewillett18 months ago

Use real href values, spaces to tabs for indentation, and simpler preventDefault

@azaozz18 months ago

comment:4 @azaozz18 months ago

One more take on this :)

In 26527.3.diff:

  • Add tabindex="0" to all elements that need to be "tabbable".
  • Use <button> instead of span, div or a link for control elements.
  • Make the theme-overlay "tabbable".
  • Contain "tabbing" inside the theme-overlay when it's open.
  • Add the :hover button (control elements) highlighting for :focus.

comment:5 @nacin18 months ago

  • Severity changed from critical to blocker

comment:6 @matveb18 months ago

Can we make the whole theme element the one that is focused with the outline treatment instead of the "theme details"? So you tab to go through themes, enter opens overlay where you have all the actions. There you can use the arrow keys or escape back to grid. Keep all the hover stuff hidden.

comment:7 @nacin18 months ago

Certainly seems simpler. matveb, can you whip up a patch?

comment:8 @matveb18 months ago

On it.

comment:9 @jorbin18 months ago

Another consideration that we need to make is that once the overlay opens, we need to the put the focus inside the overlay and when focus leaves the overlay, we should close it. I'll start writing up that portion based on my patch for you to use matveb.

@matveb18 months ago

comment:10 @matveb18 months ago

It'd be nice if once you navigate away from a theme using arrows on details view, and then pressed escape to close it, that the focused theme updates on grid as well.

@matveb18 months ago

comment:11 @matveb18 months ago

26527.5 uses "theme details" as a more evident focus hint than the mere outline. (Could also be a 2px dotted outline.)

@matveb18 months ago

@matveb18 months ago

@matveb18 months ago

@matveb18 months ago

@matveb18 months ago

comment:12 @ocean9018 months ago

26527.9.diff looks good in IE 8-11 and works in IE 7.

comment:13 @nacin18 months ago

  • Keywords has-patch commit added

comment:14 @grahamarmfield18 months ago

  • Keywords has-patch commit removed

Have tried in Chrome 31 on Windows 7 machine. The focus is now visible and I can open the modal using the keyboard. Once opened I can move around the modal using the tab key. There is an odd occurence when using shift+tab, the only two items in the modal that can get focus are the Delete button and the previous theme link/button.

In Firefox 25 the situation is the same, except that there seems to be an extra tab stop when tabbing forwards in the modal - after the Close button.

Will try with JAWS next.

comment:15 follow-up: @nacin18 months ago

  • Keywords has-patch commit added

I'm going to leave has-patch commit on here as this is an improvement over what we have currently. Further niggles can be fixed in the future. Thanks for looking.

comment:16 in reply to: ↑ 15 @grahamarmfield18 months ago

Replying to nacin:

I'm going to leave has-patch commit on here as this is an improvement over what we have currently. Further niggles can be fixed in the future. Thanks for looking.

However, the main themes page is not fully accessible by screen readers - just tested with JAWS 14. The divs get focus alright because of the tabindex="0" and JAWS reads out Theme Details but there is nothing else to identify which theme currently has focus. There needs to be some kind of label for screen readers to read out - suggest try aria-label="theme-name" on the div that gets the tabindex="0". Or alternatively stuff the Theme Details string with the theme name - hidden from sighted users.

Last edited 18 months ago by grahamarmfield (previous) (diff)

comment:17 follow-up: @jorbin18 months ago

The aim of this ticket wasn't to fix how the theme page works with all accessible technology, just to make it so that it is usable via the keyboard. I think we should open new tickets that we can target for future versions (possibly even point releases depending on the severity of the issue) for other issues.

comment:18 in reply to: ↑ 17 ; follow-up: @grahamarmfield18 months ago

Replying to jorbin:

The aim of this ticket wasn't to fix how the theme page works with all accessible technology, just to make it so that it is usable via the keyboard. I think we should open new tickets that we can target for future versions (possibly even point releases depending on the severity of the issue) for other issues.

No I understand that. I was just checking if the situation for screen readers had also been degraded. I've just tested the comparable page on 3.7 with JAWS and the situation in 3.8 is no worse than 3.7. Agree we'll need a new ticket to sort screen reader accessibility out - it'll need more than just trying random stuff out at this stage in the game.

comment:19 @matt18 months ago

  • Severity changed from blocker to normal

Accessibility is extremely important to the project, this will be a top priority for 3.8.1 including getting feedback from the accessibility group on the impact this new page has on screen readers and other accessible technology.

comment:20 @nacin18 months ago

  • Milestone changed from 3.8 to 3.8.1

comment:21 @toscho17 months ago

  • Cc info@… added

comment:22 in reply to: ↑ 18 @SergeyBiryukov17 months ago

Replying to grahamarmfield:

Agree we'll need a new ticket to sort screen reader accessibility out

Follow-up: #26602

comment:23 @nacin17 months ago

In 26922:

Keyboard navigation friendliness for themes.php.

props matveb, azaozz, jorbin.
see #26527.

comment:24 @nacin17 months ago

  • Keywords fixed-major added

comment:25 @nacin16 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 26956:

Keyboard navigation friendliness for themes.php.

Merges [26922] to the 3.8 branch.

props matveb, azaozz, jorbin.
fixes #26527.

Note: See TracTickets for help on using tickets.