WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 9 days ago

Last modified 4 days ago

#31476 closed defect (bug) (fixed)

Semantic elements for non-link links: /wp-admin/includes/widgets.php

Reported by: afercia Owned by: afercia
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.1
Component: Widgets Keywords: has-patch semantic-buttons commit
Focuses: ui, accessibility, javascript Cc:

Description

See main ticket #26504

As agreed, will need styling by the UI/design team. Uses .button-reset

Attachments (4)

31476.patch (5.3 KB) - added by afercia 2 years ago.
31476.2.patch (8.7 KB) - added by afercia 18 months ago.
31476.3.patch (10.6 KB) - added by afercia 18 months ago.
31476.diff (13.8 KB) - added by afercia 4 weeks ago.

Download all attachments as: .zip

Change History (34)

@afercia
2 years ago

#1 @afercia
2 years ago

  • Keywords has-patch added

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


2 years ago

#3 @Cheffheid
2 years ago

The widget headers themselves are also clickable, and produce a list of clickable list items, intended to allow adding widgets to sidebars without dragging. I can not get to any of this with my keyboard and literally only just found out this functionality exists.

For example, after clicking the Calendar widget under "Available Widgets":
http://i.imgur.com/493fV6P.png

I presume they need semantic treatment too, but I'm not entirely sure what to do with them.

Last edited 2 years ago by Cheffheid (previous) (diff)

#4 @afercia
2 years ago

Thanks Jeff, this patch is just for /wp-admin/includes/widgets.php
Then, there's the JavaScript part and jQuery UI to tackle :)
Then, there's also /wp-admin/widgets.php
Please try disabling JavaScript in your browser to see the widget screen in pure PHP. Instead, when JavaScript is enabled, some controls are changed and no more focusable, as you pointed out. That should be handled in the JS part.

I'm proceeding file by file, as agreed, though in some cases editing some files requires to change also other files, I would try to be as clean as possible with patches, otherwise we will end up with very big "monster" patches, very difficult to handle.

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


2 years ago

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


2 years ago

@afercia
18 months ago

#7 @afercia
18 months ago

Refreshed patch. Takes care of the buttons in the Customizer too. Also, a better string for translation. In the screenshot below, see the links in question, before and after the patch: they should be buttons. Please notice for a proper styling this patch needs to wait for #34242 and the review of the .button-link style.

About the accessibility part, there's a bug in Firefox/NVDA that prevents NVDA to announce the dynamic change of the aria-expanded attribute, see http://community.nvda-project.org/ticket/5247. We already faced this bug in the Customizer and the only workaround would be changing the markup. This issue should go in a separate ticket though.

https://cldup.com/7Jj0tPrItq.png

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


18 months ago

@afercia
18 months ago

#9 @afercia
18 months ago

Refreshed patch, now that the .button-link style was reviewed and committed in [35636]. Wondering if the .button-link CSS class should provide also the default/hover/active/focus blue colors. cc @helen
Please notice the Delete/Remove button is an edge case and has its own rule for the "red".

Screenshot with the patch applied and the buttons focused:

https://cldup.com/-FbKnKcFZL.png

Left: in the Widgets screen, right: in the Widgets panel in the Customizer.

#10 @westonruter
18 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to afercia
  • Status changed from new to assigned

#11 @afercia
17 months ago

  • Milestone changed from Future Release to 4.5

#12 follow-ups: @michaelarestad
17 months ago

Wondering if the .button-link CSS class should provide also the default/hover/active/focus blue colors.

@afercia Good question. I'm not sure if it's been discussed elsewhere, but I'd lean toward a yes on this versus trying to implement them every place the link style is used.

#13 in reply to: ↑ 12 @afercia
17 months ago

Replying to michaelarestad:

I'd lean toward a yes on this versus trying to implement them every place the link style is used.

I was thinking the same thing, after all if they should look like default links, it would be a bit annoying (and inefficient) having to implement the missing properties in every place.

#14 in reply to: ↑ 12 @afercia
17 months ago

Replying to michaelarestad:

Wondering if the .button-link CSS class should provide also the default/hover/active/focus blue colors.

@afercia Good question. I'm not sure if it's been discussed elsewhere

Should we open a separate ticket?

#15 @michaelarestad
17 months ago

@afercia Yep!

#16 @afercia
16 months ago

Related ticket for the .button-link improvements: #35126

#17 @afercia
14 months ago

  • Keywords ui-feedback added

To recap: blocked by #35126.

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


14 months ago

#19 @afercia
14 months ago

  • Milestone changed from 4.5 to Future Release

Punting, as #35126 was punted too.

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


5 months ago

#21 @afercia
5 months ago

  • Keywords needs-refresh added
  • Milestone changed from Future Release to 4.8

#22 @afercia
4 months ago

  • Keywords semantic-buttons added

#23 @afercia
3 months ago

Now that the .button-link class has been updated in [40052] it's time to move on and refresh the patch :)

This ticket was mentioned in Slack in #core-customize by afercia. View the logs.


3 months ago

@afercia
4 weeks ago

#25 @afercia
4 weeks ago

  • Keywords commit added; ui-feedback needs-refresh removed

Refreshed patch. After recent changes to the button-link CSS class in [40052], [40059], and [40358] the CSS part is a bit simplified. To recap:

  • changes some links to buttons
  • uses aria-expanded to announce the state of the toggle buttons
  • changes two #f00 red in #dc3232, see #35622
  • increases a bit the clickable area of the widget toggles
  • ensures the "circular focus" doesn't get cut-off in some browsers by centering the toggle arrows within the button
  • wraps a <span> element with an aria-hidden attribute around the toggle CSS generated icon as already done, for example, for the customizer menu, see comment:7
  • standardizes on .toggle-indicator:before rather than :after

Note: testing with screen readers, seems NVDA has troubles in announcing the aria-expanded state change when a JS animation is running or a CSS class gets applied on an ancestor element; the order in which DOM changes, JS animations, and the aria-expanded change run, does matter and shouldn't be changed. Added a couple inline comments.

In the customizer, the widgets "Remove" and "Close" controls are now underlined, this matches other controls and should be ok based on this @celloexpressions (ping!) comment on 35126. Screenshot:
https://cldup.com/GwFmZ8wPny.png

There's room for further enhancements, they should go in separate tickets. For example, other elements in the widgets screen are still not fully operable with a keyboard. Also, it would be nice to have a standardized CSS class for the "circular focus".

Last edited 9 days ago by afercia (previous) (diff)

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


4 weeks ago

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


3 weeks ago

#28 @celloexpressions
11 days ago

@afercia the added underlines on widget remove is okay I think.

#29 @afercia
9 days ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 40480:

Accessibility: Make some Widgets buttons real buttons.

Links used as UI controls that behave like buttons, should be buttons.

  • changes the widgets "toggle", "Delete", and "Close" links to buttons
  • uses aria-expanded to announce the state of the toggle buttons
  • increases a bit the clickable area of the toggle
  • ensures the "circular focus" doesn't get cut-off in some browsers by centering the toggle arrows
  • uses a <span> element with an aria-hidden attribute to hide CSS generated font icons from assistive technologies
  • standardizes on .toggle-indicator:before rather than :after
  • changes two #f00 reds in #dc3232, see #35622

Fixes #31476.

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


4 days ago

Note: See TracTickets for help on using tickets.