Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#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:


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 6 years ago.
21373.1.diff (1.9 KB) - added by Latz 6 years ago.
21373.2.diff (2.3 KB) - added by lessbloat 5 years ago.
21373.3.diff (2.0 KB) - added by SergeyBiryukov 5 years ago.

Download all attachments as: .zip

Change History (14)

6 years ago

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

6 years ago

#4 follow-up: @helenyhou
6 years ago

  • Keywords needs-patch added; has-patch removed

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

Version 0, edited 6 years ago by helenyhou (next)

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

5 years ago

#7 @lessbloat
5 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
5 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]):

#9 @helenyhou
5 years ago

  • Keywords commit added

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