WordPress.org

Make WordPress Core

#21373 closed enhancement (fixed)

Unify <label>s in options-discussion.php

Reported by: Latz Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: minor Version: 3.4.1
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

On the "Discussion Settings" page some text is not connected to an input element via <label> though most text is.

This patch will unify the behaviour of the elements. It connects the text "days" and "levels deep" to their corresponding input elements by changing the <labels>.

Additional benefit: This way the text can be accessed by jQuery selectors.

Attachments (4)

21373.diff (2.1 KB) - added by Latz 21 months ago.
21373.1.diff (1.9 KB) - added by Latz 21 months ago.
21373.2.diff (2.3 KB) - added by lessbloat 20 months ago.
21373.3.diff (2.0 KB) - added by SergeyBiryukov 20 months ago.

Download all attachments as: .zip

Change History (14)

Latz21 months ago

comment:1 ocean9021 months ago

  • Keywords needs-patch added; has-patch removed

Seems like you missed the l18n for "levels deep".

And we can't use "days" alone, since each language has it's own word ordering.

comment:2 follow-up: nacin21 months ago

  • Milestone changed from Awaiting Review to 3.5

We can wrap the entire sentences in a <label>. We do this elsewhere on the page.

comment:3 in reply to: ↑ 2 Latz21 months ago

  • Keywords has-patch added; needs-patch removed

Added a new patch which is simply moving the </label> (almost) to the end of the lines and out of the printf() parameters so word ordering does not interfere anymore.

Latz21 months ago

comment:4 follow-up: helenyhou20 months ago

  • Keywords needs-patch added; has-patch removed

Label can't wrap multiple input elements. Not sure what the right solution is, but that's not it.

Last edited 20 months ago by helenyhou (previous) (diff)

comment:5 lightningspirit20 months ago

This is my *real* opinion for years: I think this page needs a good new UI development. All the elements above the <textarea>s seem so confusing. I don't remember to see elsewhere input boxes inside entire sentences. This behavior is strange for me.

Take the example of options-media.php in Embeds section. One sentence to activate the functionality and another line to customize the functionality options.

I think I'm not saying anything stupid...

comment:6 in reply to: ↑ 4 obenland20 months ago

Replying to helenyhou:

Label can't wrap multiple input elements. Not sure what the right solution is, but that's not it.

As far as I can tell from W3C docs for the label element in HTML4.01 and HTML5, it should be all good, as long as a control is explicitly associated through the for attribute.

On a different note, I agree with @lightningspirit. By splitting up the sentences we'd not only gain label sanity but probably also contribute to a little cleaner settings page.

lessbloat20 months ago

comment:7 lessbloat20 months ago

21373.2.diff adds additional labels to these line to keep them consistent with the "Break comments into pages..." line, and the "Comments should be displayed..." line which also have text after an inline text box or select box.

I agree, all of settings is due for an overhaul, but not in 3.5.

SergeyBiryukov20 months ago

comment:8 SergeyBiryukov20 months ago

  • Keywords has-patch added; needs-patch removed

I'd prefer not to break the strings here (that would make them harder to translate), unless absolutely necessary.

21373.3.diff adds the labels without breaking the strings or wrapping multiple input elements.

The label for #default_comments_page was actually added in the same way ([9456]):
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-admin/options-discussion.php#L97

comment:9 helenyhou20 months ago

  • Keywords commit added

comment:10 ryan20 months ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [21746]:

Add labels for all inputs in options-discussion. Props SergeyBiryukov, lessbloat, Latz. fixes #21373

Note: See TracTickets for help on using tickets.