WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 7 weeks ago

#26160 closed enhancement (fixed)

More semantic markup for the post formats metabox

Reported by: tsiger Owned by: obenland
Milestone: 4.3 Priority: normal
Severity: normal Version: 3.7
Component: Post Formats Keywords: has-patch commit
Focuses: ui, accessibility, administration Cc:

Description

I think there is room for improvement in the markup used to populate the post formats list in the metabox which currently looks like:

http://f.cl.ly/items/2s2c3w2q272w0K0I1e2k/Screen%20Shot%202013-11-22%20at%203.40.36%20PM.png

Instead of using a div as a container and br tags to keep things in place as a list I replaced the div with ul and put every post format item within a li and now looks like:

http://f.cl.ly/items/1R0T2M203w1F0T241u1v/Screen%20Shot%202013-11-22%20at%203.44.32%20PM.png

I think it's semantically better now (it's a list of items after all). Checked the appearance in all major browsers (RTL too) and looks the same.

Also it is easier now for someone to target with CSS only a specific post format radio button and possible apply some extra styles to its container (li).

Attachments (3)

meta-boxes.php.patch (2.0 KB) - added by tsiger 20 months ago.
26160.patch (1.2 KB) - added by joedolson 5 months ago.
Use fieldset/legend.
26160.2.patch (1.2 KB) - added by afercia 7 weeks ago.

Download all attachments as: .zip

Change History (19)

@tsiger20 months ago

comment:1 @tsiger20 months ago

  • Summary changed from Proposed HTML changes for the post formats metabox in the Add to Proposed HTML changes for the post formats metabox

comment:2 @SergeyBiryukov20 months ago

  • Version changed from trunk to 3.7

comment:3 @helen5 months ago

  • Focuses ui accessibility administration added

Would like to hear from a11y folks if this is helpful, hurtful, or doesn't matter for screen readers and anything else that might be impacted by the markup change.

comment:4 @joedolson5 months ago

I'm not sure it makes a huge difference. It means that screen reader users will probably get notified twice about the number of items in the list - with radio button groups, screen readers generally indicate how many items there are in the radio button group; they also announce how many items are in a list.

I'd like a second opinion on this - I'm going both ways on it: it is a more semantic choice to have this in a list, but it also means that essentially redundant information is being announced for screen reader users. From a usability perspective, I think it makes the experience worse.

Last edited 5 months ago by joedolson (previous) (diff)

comment:5 @slackbot5 months ago

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

comment:6 follow-up: @rianrietveld5 months ago

Why not use as container a fieldset, remove the <br> and let the css decide the layout (inline-block/block). Adding an extra layer of HTML seems not necessary to me, you can style it any way you want, and don't need a <ul> for that.

comment:7 in reply to: ↑ 6 @afercia5 months ago

Replying to rianrietveld:

Why not use as container a fieldset, remove the <br>

+1 for a fieldset, with a nice, screen-reader-text'ed legend too :). Screen readers will read out the legend as "{legend text} grouping", or similar.

comment:8 @helen5 months ago

  • Keywords has-patch removed

@rianrietveld @afercia Do you feel that we need to make changes to this specific metabox, as in would you consider the fieldset suggestion to be necessary?

comment:9 @afercia5 months ago

@helen, fieldset + legend would be a nice addition. Without them users land on these radio buttons and hear just "video", "audio", etc. with no clue what that actually means. With no initial clue, at least :) With a fieldset+legend they would hear "{legend text} grouping". Press This already uses:

<fieldset><legend class="screen-reader-text"><?php _e( 'Post formats' ); ?></legend>
...

comment:10 @helen5 months ago

  • Keywords needs-patch added
  • Summary changed from Proposed HTML changes for the post formats metabox to More semantic markup for the post formats metabox

@joedolson5 months ago

Use fieldset/legend.

comment:11 @joedolson5 months ago

  • Keywords has-patch added; needs-patch removed

@afercia7 weeks ago

comment:12 @afercia7 weeks ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.3

Rebuilt the patch from the root, also removed the last useless <br />. It's a small change and at the same time gives us better semantics, improved accessibility and consistency with Press This. I'm all for it, would propose for commit consideration.

comment:13 @slackbot7 weeks ago

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

comment:14 @afercia7 weeks ago

  • Owner set to davidakennedy
  • Status changed from new to assigned

comment:15 @obenland7 weeks ago

  • Owner changed from davidakennedy to obenland
  • Status changed from assigned to accepted

comment:16 @obenland7 weeks ago

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

In 32720:

More semantic markup for the post formats metabox.

Screen readers will now introduce the group of radio buttons.
Also brings the meta box to parity with Press This.

Props joedolson, afercia.
Fixes #26160.

Note: See TracTickets for help on using tickets.