WordPress.org

Make WordPress Core

Opened 8 weeks ago

Last modified 8 days ago

#52649 accepted enhancement

Themes admin page: provide labeling context on controls and links

Reported by: alexstine Owned by: alexstine
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.7
Component: Themes Keywords: has-patch needs-testing needs-design-feedback has-screenshots
Focuses: accessibility, administration Cc:

Description

Fix up some accessibility issues on the Themes admin page.

  1. Make sure all links are clear and contain a theme name.
  2. Render Theme Details as a button that can have focus.

These are just some ideas that I think will greatly improve the page, open to hear feedback.

Attachments (4)

52649.2.diff (8.4 KB) - added by joedolson 8 weeks ago.
Patch with focus handling.
52649-poc.patch (14.2 KB) - added by poena 8 weeks ago.
POC: Position details, activate and preview below screenshot
wordpress-themes-20210415-52649-poc.png (389.4 KB) - added by joedolson 8 days ago.
Screenshot with 52649-poc.patch applied. Active theme, hovered theme card, inactive theme card.
wordpress-themes-20210415.png (365.0 KB) - added by joedolson 8 days ago.
Themes in current trunk. Active theme, hovered theme card, inactive theme.

Download all attachments as: .zip

Change History (28)

#1 @alexstine
8 weeks ago

  • Focuses multisite removed
  • Status changed from assigned to accepted

This ticket was mentioned in PR #1043 on WordPress/wordpress-develop by alexstine.


8 weeks ago

  • Keywords has-patch added

#3 @alexstine
8 weeks ago

  • Keywords needs-testing added

To test:

  1. Apply the PR.
  2. Navigate to the Themes page in WordPress Admin.
  3. Test by tabbing around the page.
  4. Start a screen reader such as NV Access or Voiceover and test tabbing around the page. Listen to see if all buttons and links are clearer now. E.g. instead of hearing "Live Preview", you should hear "Live Preview theme_name".

Also open to feedback on if others feel this is a good idea or not, there may be a reason the page was how it was.

Thanks!

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


8 weeks ago

#5 @poena
8 weeks ago

Tested with NVDA and Chrome version 88.0.4324.190, Windows 10.

The code looks good to me.

The context is clearer, but for inactive themes, I need to press tab 4 times.

When I press tab and focus lands on the theme div, I hear:
"clickable View Theme Details for Twenty Eleven button
Twenty Eleven heading level 2 Activate Twenty Eleven link Live Preview Twenty Eleven"

When I press tab again, nothing is announced and I can no longer see where the focus is. There is no visible focus on the theme div, activation or preview link.

Next tab: "Activate Twenty Eleven link".
There is a visible border on the activation link.

Next tab: "Live Preview Twenty Eleven link"
There is a visible border on the preview link.

For the active theme, I need to press tab 3 times, and it behaves the same with a silent tab with no visual focus between the theme div and the customize link.

Last edited 8 weeks ago by poena (previous) (diff)

#6 @alexstine
8 weeks ago

@poena Just pushed another update. I've removed tabindex and aria-describedby from the parent div. Now the tab order feels a lot better. Agree or disagree?

Thanks.

#7 @poena
8 weeks ago

Theme admin page:
There is no visible focus indicator on the first tab.

The span with the text "Theme Details" is not visible when the theme div is focused, sighted keyboard users won't know how to view the theme details.

The change to the JavaScript also affects the add themes page:
It seems that the theme div is no longer focusable.
"Details & Preview" is not announced by the screen reader.

When I press tab, the focus order is:
install theme link
preview theme button
install theme link, for the next theme.

#8 @alexstine
8 weeks ago

@poena How is it now? I converted the spans to real buttons, there may be visual indication now. If not, may need to get a CSS expert to help me with the rest of this because from a screen reader perspective it's working perfectly for me.

Tab order:

  1. Theme Details
  2. Customize/Activate
  3. Live Preview
  4. Next theme starting from #1

Thanks.

#9 @johnjamesjacoby
8 weeks ago

@poena your feedback here is amazing.

If you’re comfortable making recommendations on the necessary changes to make this page maximally accessible, I for one would love to hear them.

For example, should the theme div actually be focusable? Or even “it’s all bad right now and here’s how it should work” because lots of the multisite admin screens need attention.

I’d love-love to see initiatives like this ticket here be clearly guided by the necessary accessibility improvements, to help reduce the amount of trial & error.

But if you’re both enjoying yourselves and making good progress, don’t let me stop you! 😅

#10 @joedolson
8 weeks ago

  • Summary changed from Themes admin page: fix some accessibility issues to Themes admin page: provide labeling context on controls and links

#11 @poena
8 weeks ago

I am sure we can improve the page for 5.8.
Reviewing and redoing the multipage admin screens is a massive task compared to only adding the context.
I would say it is worth doing but needs careful planning, time and attention. :)

I would prefer if the theme details was a plain link instead of the current overlay.

The theme div does not need to be focusable as long as the theme details, Customize/Activate and
Live Preview can be easily accessed by all users.

#12 @joedolson
8 weeks ago

Looks good! In testing, I found that the removal of tabindex on the .themes div caused two issues: CSS attached to .theme:focus was broken, and the collapse function was no longer able to send focus back to the closed theme.

This patch changes .theme:focus to .theme.focus and sends focus to the theme details button on collapse.

@joedolson
8 weeks ago

Patch with focus handling.

#13 @joedolson
8 weeks ago

This time, the patch was run using SVN. Oddly enough, if you generate your patch using NPM, you don't get the right patch...

#14 @alexstine
8 weeks ago

Just tested your patch @joedolson and it's still working fine. Focus order still works for screen reader. Theme Details, Activate/Customize, Live Preview.

#15 @poena
8 weeks ago

With patch 52649.2.diff, the Add themes screen is unaffected (which is what we want for now).

On the theme admin screen, the theme details button needs some additional styling to override browser specific styles for the button element.
-The position and size does not match the design before the patch, and it has a visible focus style that is inconsistent with the rest of the UI.

What if we instead moved the theme details, activate and preview below the theme name?
Reduce complexity and let these 3 links/buttons be visible at all times and not only on hover and focus, and without obscuring the theme screenshot.

Making space for information below the screenshot would also be needed for further improvements of the theme pages, like displaying the date of the last update, version, and star rating.

Last edited 8 weeks ago by poena (previous) (diff)

@poena
8 weeks ago

POC: Position details, activate and preview below screenshot

#16 @alexstine
8 weeks ago

@poena I gave your patch a test and this works better in the way that the Theme Details button is now below the main heading. This works better if navigating in browse mode rather than focus/edit mode where these details don't matter at much.

#17 @joedolson
4 weeks ago

  • Keywords needs-design-feedback needs-refresh needs-screenshots added
  • Milestone changed from Future Release to 5.8

I'd like to try and get this into 5.8, but we should probably get some design feedback first. I think this is a better workflow, and definitely improves some accessibility issues.

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


10 days ago

#19 @paaljoachim
10 days ago

Ticket was brought up during a design triage.
We need a before and after screenshot to get an idea of what needs design feedback.
Thanks!

#20 @poena
8 days ago

I am afraid I don't follow which one of the patches we need screenshots of.

#21 @joedolson
8 days ago

I think there are only intentional visual changes in your patch, @poena, so I think that's what we need screenshots for.

Attaching screenshots shortly.

@joedolson
8 days ago

Screenshot with 52649-poc.patch applied. Active theme, hovered theme card, inactive theme card.

@joedolson
8 days ago

Themes in current trunk. Active theme, hovered theme card, inactive theme.

#22 @joedolson
8 days ago

  • Keywords has-screenshots added; needs-screenshots removed

#23 @joedolson
8 days ago

  • Keywords needs-refresh removed

#24 @joedolson
8 days ago

Patch applied without error; not sure why needs refresh was there.

Note: See TracTickets for help on using tickets.