Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#26527 closed defect (bug) (fixed)

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

Reported by: jorbin's profile jorbin Owned by: nacin's profile 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 9 years ago.
26527.2.diff (2.4 KB) - added by lancewillett 9 years ago.
Use real href values, spaces to tabs for indentation, and simpler preventDefault
26527.3.diff (7.4 KB) - added by azaozz 9 years ago.
26527.4.diff (6.4 KB) - added by matveb 9 years ago.
26527.5.diff (6.4 KB) - added by matveb 9 years ago.
26527.6.diff (6.8 KB) - added by matveb 9 years ago.
26527.6.2.diff (6.8 KB) - added by matveb 9 years ago.
26527.7.diff (6.8 KB) - added by matveb 9 years ago.
26527.8.diff (6.8 KB) - added by matveb 9 years ago.
26527.9.diff (7.1 KB) - added by matveb 9 years ago.

Download all attachments as: .zip

Change History (35)

#1 @SergeyBiryukov
9 years ago

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

@jorbin
9 years ago

#2 @jorbin
9 years 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.

#3 @nacin
9 years ago

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

@lancewillett
9 years ago

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

@azaozz
9 years ago

#4 @azaozz
9 years 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.

#5 @nacin
9 years ago

  • Severity changed from critical to blocker

#6 @matveb
9 years 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.

#7 @nacin
9 years ago

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

#8 @matveb
9 years ago

On it.

#9 @jorbin
9 years 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.

@matveb
9 years ago

#10 @matveb
9 years 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.

@matveb
9 years ago

#11 @matveb
9 years ago

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

@matveb
9 years ago

@matveb
9 years ago

@matveb
9 years ago

@matveb
9 years ago

@matveb
9 years ago

#12 @ocean90
9 years ago

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

#13 @nacin
9 years ago

  • Keywords has-patch commit added

#14 @grahamarmfield
9 years 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.

#15 follow-up: @nacin
9 years 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.

#16 in reply to: ↑ 15 @grahamarmfield
9 years 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 9 years ago by grahamarmfield (previous) (diff)

#17 follow-up: @jorbin
9 years 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.

#18 in reply to: ↑ 17 ; follow-up: @grahamarmfield
9 years 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.

#19 @matt
9 years 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.

#20 @nacin
9 years ago

  • Milestone changed from 3.8 to 3.8.1

#21 @toscho
9 years ago

  • Cc info@… added

#22 in reply to: ↑ 18 @SergeyBiryukov
9 years ago

Replying to grahamarmfield:

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

Follow-up: #26602

#23 @nacin
9 years ago

In 26922:

Keyboard navigation friendliness for themes.php.

props matveb, azaozz, jorbin.
see #26527.

#24 @nacin
9 years ago

  • Keywords fixed-major added

#25 @nacin
9 years 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.