Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#21373 closed enhancement (fixed)

Unify <label>s in options-discussion.php

Reported by: latz's profile Latz Owned by: ryan's profile 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 13 years ago.
21373.1.diff (1.9 KB) - added by Latz 13 years ago.
21373.2.diff (2.3 KB) - added by lessbloat 13 years ago.
21373.3.diff (2.0 KB) - added by SergeyBiryukov 13 years ago.

Download all attachments as: .zip

Change History (14)

@Latz
13 years ago

#1 @ocean90
13 years 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.

#2 follow-up: @nacin
13 years 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.

#3 in reply to: ↑ 2 @Latz
13 years 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.

@Latz
13 years ago

#4 follow-up: @helenyhou
13 years 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 13 years ago by helenyhou (previous) (diff)

#5 @lightningspirit
13 years 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...

#6 in reply to: ↑ 4 @obenland
13 years 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.

@lessbloat
13 years ago

#7 @lessbloat
13 years 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.

#8 @SergeyBiryukov
13 years 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

#9 @helenyhou
13 years ago

  • Keywords commit added

#10 @ryan
13 years 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.