WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 6 weeks ago

#18650 new enhancement

Make archives and categories widgets dropdown ada compliant

Reported by: jlevandowski Owned by:
Milestone: Future Release Priority: normal
Severity: major Version: 3.2.1
Component: Widgets Keywords: has-patch
Focuses: accessibility Cc:

Description

Conditionally add the <label> tag for the archives and categories widgets so they are ada compliant.

http://webaim.org/techniques/forms/controls

Attachments (5)

widgets-patch.diff (1.8 KB) - added by jlevandowski 3 years ago.
18650-patch.diff (2.5 KB) - added by jlevandowski 3 years ago.
18650.2.patch (3.0 KB) - added by SergeyBiryukov 14 months ago.
18650.patch (4.1 KB) - added by DrewAPicture 11 months ago.
esc_attr_e
18650.diff (4.1 KB) - added by nacin 6 weeks ago.

Download all attachments as: .zip

Change History (21)

jlevandowski3 years ago

comment:2 GaryJ3 years ago

I'd prefer to see the <select> element get an id attribute that had the widget ID as a suffix - that way, more than one archive dropdown widget can be used on a page with valid unique ids.

jlevandowski3 years ago

comment:3 jlevandowski3 years ago

Added patch that adds widget id so that more than one widget of the same kind can be used on a page.

comment:4 jlevandowski21 months ago

  • Component changed from Widgets to Accessibility
  • Keywords dev-feedback added

comment:5 webstoc14 months ago

  • Severity changed from normal to major

We verified that the patch fixes red flags in the WAVE toolbar. Curious what the hold up is on reviewing the patch by jlevandowski and getting it into the widgets? There are dozens (if not hundreds) of .gov sites using WordPress that need these fixes. As an "accessibility" issue, this more than of a "major" than "normal" severity.

Last edited 14 months ago by webstoc (previous) (diff)

comment:6 webstoc14 months ago

  • Cc webstoc added

SergeyBiryukov14 months ago

comment:7 follow-up: SergeyBiryukov14 months ago

  • Milestone changed from Awaiting Review to 3.6

Refreshed the patch.

Not sure if changing the existing id attribute for Categories widget is a good idea though. I guess it may lead to unexpected results in some themes.

comment:8 SergeyBiryukov14 months ago

#16527 was marked as a duplicate.

DrewAPicture11 months ago

esc_attr_e

comment:9 in reply to: ↑ 7 DrewAPicture11 months ago

Replying to SergeyBiryukov:

Not sure if changing the existing id attribute for Categories widget is a good idea though. I guess it may lead to unexpected results in some themes.

I see where you're coming from on this. And I can see why the inner select id was left alone -- the main widget id is unique for each instance just as with the other core widgets. On that same token, themes shouldn't have been targeting ids since they are by nature meant to be unique anyway.

18650.patch builds off 18650.2.patch and swaps out echo esc_attr( __( 'String' ) ) for esc_attr_e( 'String' )

comment:10 ocean9010 months ago

  • Milestone changed from 3.6 to Future Release

comment:11 grahamarmfield4 months ago

  • Cc graham.armfield@… added

comment:12 nacin3 months ago

  • Component changed from Accessibility to Widgets
  • Focuses accessibility added

comment:13 joedolson3 months ago

Is there something in particular that this ticket is waiting on?

comment:14 nacin3 months ago

  • Keywords dev-feedback removed
  • Milestone changed from Future Release to 3.9

Changing the IDs for these are precarious as themes may be using them for styling (the first instance, anyway). I could go for trying this out, though I wouldn't mind seeing if we could tweak it.

There is a lot of repetitive string building in this patch. A property or method specific to that widget would likely help clean that up.

comment:15 GaryJ3 months ago

Worth adding a class to the select elements, so that themes can start targeting that instead?

nacin6 weeks ago

comment:16 nacin6 weeks ago

  • Milestone changed from 3.9 to Future Release

So basically:

  • The first instance of the category widget widget should use the current IDs.
  • Subsequent instances should use incremented IDs.

The archives widget doesn't currently have an ID on the select so it doesn't require this juggling.

I've attached 18650.diff to do this. The problem is, I think the labels wrapping the title have the potential to break a theme's styling of the header. (Imagine <h3><label>Title</label></h3> with label styling overriding h3 styling. An alternative would probably be screen reader text, which we actually don't necessarily have the classes for on the frontend.

As it is, these are not accessible due to the lack of a submit button, while at the very least the first options are "Select Month" and "Select Category", which provide a clue it's a JS jump menu. I'm not inclined to touch this without further consideration.

Note: See TracTickets for help on using tickets.