Make WordPress Core

Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#36627 closed defect (bug) (fixed)

Theme preview/details: change non-links to buttons and remove disabled buttons from tabindex

Reported by: walbo's profile walbo Owned by: afercia's profile afercia
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch semantic-buttons
Focuses: accessibility, javascript Cc:


The next/prev theme navigation gets the classname .disabled but is still available in the tabindex. Also the navigation is non-links <a href="#"> that should be <button>.

If you previewing a installed theme, there is a non-link thats says installed. The link should not be a link.

Attachments (5)

36627.patch (5.5 KB) - added by walbo 7 years ago.
36627.02.patch (5.5 KB) - added by walbo 7 years ago.
disabled Screen Shot 2018-03-12 at 20.27.44.png (24.4 KB) - added by afercia 6 years ago.
This button looks disabled but is still focusable
36627.03.patch (834 bytes) - added by walbo 6 years ago.
36627.diff (2.3 KB) - added by afercia 5 years ago.

Download all attachments as: .zip

Change History (14)

7 years ago

#1 @walbo
7 years ago

  • Keywords has-patch added

7 years ago

#2 @afercia
7 years ago

  • Focuses javascript added
  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to Future Release

Thanks @walbo yes the theme browser needs some accessibility improvements and many of the points you outlined are perfectly valid. Many things have changed in the last months, see for example [38084] that added a disabled attribute on the next/prev navigation to take them out from the tab order when they're... disabled :)

Other things need to be fixed yet, for example some buttons miss a type="button" attribute and the JS part could be probably simplified. Also, some links (with a few exceptions for the ones that still work when JS is off) need to be buttons.

I'd suggest to keep things separated though and open a separate ticket for the theme installer. Keeping the issues related to the theme browser in this ticket and addressing the theme installer issues in a separate ticket would help to make things more clear for other contributors and reviewers.

#3 @afercia
6 years ago

  • Keywords semantic-buttons added

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

6 years ago

#5 @afercia
6 years ago

  • Milestone changed from Future Release to 5.0

Just checked again this and seems to me a few things have changed and there's only one point to address:

in the theme installer, the buttons that look "disabled" are still focusable and it is possibel to Tab to them. They should use a disabled attribute. See screenshot below.

6 years ago

This button looks disabled but is still focusable

6 years ago

#6 @walbo
6 years ago

@afercia Updated the patch so to add disabled attribute to the buttons when disabled.

5 years ago

#7 @afercia
5 years ago

  • Keywords needs-refresh removed

36627.diff avoids a focus loss when navigation is at the first or last theme, and improves the buttons accessibility text.

#8 @afercia
5 years ago

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

In 43020:

Accessibility: Improve the Themes Installer navigation buttons accessibility.

  • really disables buttons when they look disabled (when navigation is at the first or last theme)
  • when navigation is at the first or last theme, moves focus to the other navigation button, to avoid a focus loss
  • improves the buttons visually hidden accessibility text

Props walbo, afercia.
Fixes #36627.

#9 @johnbillion
5 years ago

  • Milestone changed from 5.0 to 5.1
Note: See TracTickets for help on using tickets.