WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#27521 closed defect (bug) (fixed)

Theme Install Page is unusable via the keyboard

Reported by: jorbin Owned by: nacin
Milestone: 3.9 Priority: high
Severity: major Version: 3.9
Component: Themes Keywords: has-patch commit
Focuses: accessibility, javascript Cc:

Description

While testing wp-admin/theme-install.php, navigating the page via the tab key makes the page completely unusable.

First things noted on initial testing:

.theme-navigation links are not actually links so you cannot focus on them.

When you are focused on a theme (divs with the theme class) and press tab, you are moved to the theme preview screen.

Attachments (20)

27521.diff (2.5 KB) - added by jorbin 4 years ago.
27521.2.diff (4.1 KB) - added by jorbin 4 years ago.
27521.3.diff (4.3 KB) - added by jorbin 4 years ago.
27521.4.diff (9.2 KB) - added by jorbin 4 years ago.
27521.5.diff (5.4 KB) - added by jorbin 4 years ago.
27521.6.diff (5.0 KB) - added by jorbin 4 years ago.
27521.7.diff (5.0 KB) - added by jorbin 4 years ago.
27521.8.diff (9.0 KB) - added by jorbin 4 years ago.
27521.9.diff (733 bytes) - added by helen 4 years ago.
27521.10.diff (1001 bytes) - added by matveb 4 years ago.
27521.11.diff (1.5 KB) - added by adamsilverstein 4 years ago.
Theme install preview - Disable next/prev buttons when you reach an dead end.
27521.12.diff (1.5 KB) - added by adamsilverstein 4 years ago.
Theme install preview - Disable next/prev buttons when you reach an dead end.
27521.13.diff (1.3 KB) - added by adamsilverstein 4 years ago.
corrected: Disable next/prev buttons when you reach an dead end.
27521.14.diff (1.2 KB) - added by adamsilverstein 4 years ago.
Escape to close Preview
27521.15.diff (1.2 KB) - added by adamsilverstein 4 years ago.
corrected: Escape to close Preview (removed blank line)
27521.16.diff (2.8 KB) - added by mikeschroder 4 years ago.
Refreshed, Cleaned up, and Combined 13 & 15.
27521.17.diff (783 bytes) - added by adamsilverstein 4 years ago.
add left/right arrow key events to install preview mode
27521.18.diff (5.6 KB) - added by matveb 4 years ago.
27521.19.diff (5.3 KB) - added by matveb 4 years ago.
typo
27521.20.diff (362 bytes) - added by adamsilverstein 4 years ago.
only navigate to previous theme ._once

Download all attachments as: .zip

Change History (41)

@jorbin
4 years ago

#1 @jorbin
4 years ago

First pass on this fixes the main issues on the overview page.

  • Filters are navigable and selectable via the keyboard
  • You can navigate from theme to theme via the keyboard
  • Global keyboard shortcuts like CMD-L actually work

@todo the theme detail page doesn't work at all right now.

@jorbin
4 years ago

#2 @jorbin
4 years ago

I still haven't touched the preview panel, bt the above patch takels much of the issues on the overview panel.

This ticket was mentioned in IRC in #wordpress-ui by nacin. View the logs.


4 years ago

@jorbin
4 years ago

@jorbin
4 years ago

@jorbin
4 years ago

@jorbin
4 years ago

@jorbin
4 years ago

@jorbin
4 years ago

#4 follow-up: @jorbin
4 years ago

The above patch ( 27521.8.diff ) corrects all the issues I know about on the theme install screen. It doesn't do anything on the theme install details page.

One change that also affects the theme screen is that the action buttons are now focusable.

#5 in reply to: ↑ 4 @helen
4 years ago

Replying to jorbin:

One change that also affects the theme screen is that the action buttons are now focusable.

YAY!

#6 @helen
4 years ago

In 27804:

Bring keyboard accessibility to the theme install screen and theme action buttons. props jorbin. see #27521.

@helen
4 years ago

#7 @helen
4 years ago

  • Keywords has-patch added

27521.9.diff brings focus into the overlay and then puts it back on close. It copies some the existing pattern from theme management, but doesn't quite do it all (_.delay() or a separate function for setting/containing focus). Not sure what the preference is here.

#8 @matveb
4 years ago

Nice — quick mention that if we switch the Preview view back to the Details view we have in themes.php (updating the templates) we should get most of the work that went into keyboard accessibility for themes.php for free. I'll be tackling that soonish after finishing the other priority items if no one gets to that first.

#9 @nacin
4 years ago

  • Priority changed from highest omg bbq to high
  • Severity changed from blocker to major

This page is much better now, and we have a plan of action for finishing it off, so I'm turning off the red.

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


4 years ago

@matveb
4 years ago

#11 follow-up: @matveb
4 years ago

Related to this, some nice things to have:

  • Escape to close Preview / go back to sidebar if you are in iframe.
  • Disable next/prev buttons when you reach an dead end.
  • Contain focus to sidebar + preview.

@adamsilverstein
4 years ago

Theme install preview - Disable next/prev buttons when you reach an dead end.

@adamsilverstein
4 years ago

Theme install preview - Disable next/prev buttons when you reach an dead end.

@adamsilverstein
4 years ago

corrected: Disable next/prev buttons when you reach an dead end.

@adamsilverstein
4 years ago

Escape to close Preview

@adamsilverstein
4 years ago

corrected: Escape to close Preview (removed blank line)

#12 in reply to: ↑ 11 @adamsilverstein
4 years ago

Replying to matveb:

Related to this, some nice things to have:

  • Escape to close Preview / go back to sidebar if you are in iframe.
  • Disable next/prev buttons when you reach an dead end.
  • Contain focus to sidebar + preview.

Adding these incrementally for clarity; I can definitely seeing some code duplication that could be combined.

This ticket was mentioned in IRC in #wordpress-dev by DH-Shredder. View the logs.


4 years ago

@mikeschroder
4 years ago

Refreshed, Cleaned up, and Combined 13 & 15.

#14 @mikeschroder
4 years ago

Refreshed (since 27521.13.diff was no longer applying) and cleaned up 27521.15.diff and 27521.13.diff, and put them into: 27521.16.diff

Since I was testing them together, right now they're in the same patch. If a committer would rather apply them in separate commits, I can split them out again.

#15 @mikeschroder
4 years ago

  • Keywords commit added

#16 @nacin
4 years ago

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

In 28033:

Theme Installer: Disable prev/next buttons on first/last themes, add Esc handling, use IDs.

props adamsilverstein, DH-Shredder.
fixes #27521.

#17 @nacin
4 years ago

The overlay issue is not new. New ticket for post-3.9: #27705. A simple fix *could* be pulled in post RC for 3.9.

@adamsilverstein
4 years ago

add left/right arrow key events to install preview mode

#18 @nacin
4 years ago

In 28036:

Theme Installer: Left/right arrow keys in the overlay.

props adamsilverstein.
see #27521.

@matveb
4 years ago

@matveb
4 years ago

typo

#19 follow-up: @matveb
4 years ago

Use Backbone events to handle keyups on Preview. Fix bugs with the current model state persisting after you opened-navigate-close the Preview. Also hard prevents previous theme from going into a loop.

$themeInstaller could be preview.$el instead if we were to pass it to the function.

#20 @nacin
4 years ago

In 28049:

Theme installer keyboard fixes. Updates [28033] and [28036].

props matveb.
see #27521.

@adamsilverstein
4 years ago

only navigate to previous theme ._once

#21 in reply to: ↑ 19 @adamsilverstein
4 years ago

Oh, Backbone events - thats how to handle events! :)

I noticed you added ._once around the nextTheme call on key-right, it was left off for previousTheme/key-left; Unless there is a specific reason for leaving off, we may want to add that, see 27521.20.diff

Replying to matveb:

Use Backbone events to handle keyups on Preview. Fix bugs with the current model state persisting after you opened-navigate-close the Preview. Also hard prevents previous theme from going into a loop.

$themeInstaller could be preview.$el instead if we were to pass it to the function.

Note: See TracTickets for help on using tickets.