WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#34492 closed defect (bug) (fixed)

#a11y-headings - Adapt title in Settings API to new hierarchy

Reported by: neoxx Owned by: afercia
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.4
Component: Options, Meta APIs Keywords: has-patch
Focuses: ui, accessibility Cc:

Description (last modified by afercia)

Following https://make.wordpress.org/core/2015/10/28/headings-hierarchy-changes-in-the-admin-screens/ the title in do_settings_sections() should move up one level, too.

#a11y-headings

Attachments (1)

34492.diff (479 bytes) - added by neoxx 4 years ago.

Download all attachments as: .zip

Change History (13)

#1 @neoxx
4 years ago

  • Keywords has-patch added

@neoxx
4 years ago

#2 @SergeyBiryukov
4 years ago

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

#3 @afercia
4 years ago

  • Description modified (diff)
  • Focuses ui added
  • Summary changed from Adapt title in Settings API to new hierarchy to #a11y-headings - Adapt title in Settings API to new hierarchy

@neoxx good catch, thanks very much :) Wondering what to do here, in some cases should be a <h3>:

https://cldup.com/32zicOgUD7.png

Without the "tabs", should be a <h2>. We've discussed a bit the "tabs", probably they shouldn't be within a heading in the first place and they're more a navigation sub-menu. I'd be inclined to keep things simple and just change the heading output by do_settings_sections() in a <h2>. As a side note, this is one of the reasons why I'm not so happy when core outputs some HTML assuming it's the proper markup in any case, without giving users the ability to filter it some way :)
Will ask the accessibility team for some feedback, the proposed patch looks the best option to me.

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


4 years ago

#5 @joedolson
4 years ago

I feel like we ought to treat this as if the tabs default markup was *not* a heading, and make plans to remove the heading wrapping the tabs in a future release. I don't feel that usage of a heading is at all appropriate, and it's something we should really fix; I'd rather accept the slightly incorrect headings hierarchy temporarily with the long-term goal of resolving if by fixing the tabs structure.

#6 @afercia
4 years ago

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

In 35459:

Accessibility: Bump the Settings API sections heading one level up.

Also, fix a typo in the do_settings_sections() DocBlock.

Props neoxx.
Fixes #34492.

#7 @theZedt
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Is this absolutely necessary?

We are relying on the section titles being wrapped in h3 tags in our theme options and this change completely breaks 4 of our 5 themes settings pages which use accordions.
I don't think we'd be the only ones affected...

Adding a filter for the title would be a good workaround so we can change the wrapping ourselves in our usage scenario.

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


4 years ago

#9 @afercia
4 years ago

@theZedt hi, as far as I see this specific change doesn't touch anything related to accordions (meaning the built-in accordions provided by WordPress, see do_accordion_sections(), accordion.js ). Is yours a custom implementation?

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


4 years ago

#11 follow-up: @theZedt
4 years ago

Our themes have their roots before Wordpress 3.6 introduced do_accordion_sections() so they still rely on the older do_settings_sections() with the JS accordion manually applied on top (onto the h3 tags outputted by that).

By the look of the codex entry for do_accordion_sections(), it wasn't too publicized or documented...

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

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

Replying to theZedt:

still rely on the older do_settings_sections() with the JS accordion manually applied on top (onto the h3 tags outputted by that).

Thanks for the details. So, since /wp-admin/js/accordion.js targets any element with a CSS class .accordion-section-title I'm assuming you're not using that but you're using a custom implementation that targets the h3. If so, you can simply update the jQuery selector. Core can't predict or guarantee support for custom implementations.

Note: See TracTickets for help on using tickets.