WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#29348 closed enhancement (fixed)

Add classes to form containers on user-edit.php

Reported by: jarednova Owned by: SergeyBiryukov
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.0
Component: Users Keywords: has-patch
Focuses: ui, administration Cc:
PR Number:

Description

This is related to #28196 and applies the same methodology to the user-edit screen. Currently all the field containers are simply <tr> wrappers.

This makes it difficult for a developer to hide or modify specific fields on the page. This patch applies a universal class convention to each tr like...

<tr class="user-description-wrap">
<tr class="user-capabilities-wrap">

Classes are used instead of IDs because IDs are already occasionally used, such as...

<tr class="user-password-wrap" id="password">

http://i.imgur.com/rIqYoKY.png

Attachments (1)

edit-user-classes.txt (7.8 KB) - added by jarednova 5 years ago.
Git diff to add classes to <tr> wrappers on user-edit.php

Download all attachments as: .zip

Change History (11)

@jarednova
5 years ago

Git diff to add classes to <tr> wrappers on user-edit.php

#1 @jarednova
5 years ago

  • Keywords has-patch added

#2 @SergeyBiryukov
5 years ago

  • Component changed from Administration to Users
  • Focuses ui added; template removed
  • Milestone changed from Awaiting Review to 4.1

#3 @SergeyBiryukov
5 years ago

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

In 29804:

Add classes to form containers on Edit User screen.

props jarednova.
fixes #29348.

#4 @cais
5 years ago

I'm all for adding classes to better identify and expose various elements for whatever reason but isn't appending -wrap to each class a bit redundant? Otherwise ... kudos, @jarednova!

#5 @obenland
5 years ago

Yeah, I'm not a big fan of the -wrap suffix either FWIW.

#6 follow-up: @pampfelimetten
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It's great, that this is in, I've been waiting for it for 5 years ;-), see #12295, which can now be closed I think.

BUT: Is it possible to also add classes to the <tr> tags, as jarednova did in the second patch?
If we are at it, we should go all the way.

#7 @jarednova
5 years ago

@pampfelimetten the classes here are added to the <tr> tags -- is there a specific part you're referring to that I missed?

@oberland and @cais: my reasoning for the -wrap suffix was to be 100% clear that these don't class the fields/inputs themselves, but rather the entire set. The class should speak for itself rather than rely on always being attached to a certain element type. While they're on <tr>s today, I can imagine a future when the admin is no longer a table-based layout. The -wrap adds some more characters, but it also removes any possible confusion about what part of the form these relate to.

#8 @chrisvanpatten
5 years ago

I'm with @jarednova on -wrap. Unless there's a specific legacy/consistency reason not to, the extra specificity is nice.

#9 in reply to: ↑ 6 @SergeyBiryukov
5 years ago

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

Reclosing as fixed in [29804], see comment:7.

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


5 years ago

Note: See TracTickets for help on using tickets.