WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 2 months ago

#40462 accepted enhancement

Add placeholders to wp_login_form()

Reported by: ramiy Owned by: voldemortensen
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: has-patch close
Focuses: accessibility, template Cc:

Description

Currently, if someone uses the wp_login_form() function in a theme or a plugin, and this someone wants to add placeholders to the input fields, he can't do that.

The attached patch adds placeholders to the wp_login_form() function using two new arguments. To maintain backwards compatibility, the placeholders are empty by default.

Attachments (6)

40462.patch (2.7 KB) - added by ramiy 8 months ago.
40462.2.patch (5.6 KB) - added by ramiy 3 months ago.
phpDocs alignment
40462.3.patch (5.8 KB) - added by ramiy 3 months ago.
Add phpDocs @since entry
40462.4.patch (5.8 KB) - added by ramiy 3 months ago.
Add @since entry to phpDocs
40462.4.2.patch (5.8 KB) - added by voldemortensen 3 months ago.
40462.5.patch (6.7 KB) - added by ramiy 2 months ago.
add label classes

Download all attachments as: .zip

Change History (36)

@ramiy
8 months ago

#1 @ramiy
8 months ago

  • Focuses template added
  • Keywords has-patch added

With this patch developers can add placeholders to their login forms:

wp_login_form( array(
	'placeholder_username' => __( 'Your username...' ),
	'placeholder_password' => __( 'Your password...' )
) );

#3 @ramiy
8 months ago

Related: #24324

#4 @rel78
8 months ago

It can be a great fix for the login form!
I found myself more than once using JS to modify the form,
when the client delivered a design for the form with placeholders..

$('#user_login').attr( 'placeholder', 'User Name' );
$('#user_pass').attr( 'placeholder', 'Password' );

#5 @ramiy
6 months ago

What problem does is solves?

Currently, plugin and theme developers they can set only custom labels. The placeholders are hardcoded. This change will add extra customization options for developers. Either to delete the placeholders entirely or to set custom placeholders.

#6 @ramiy
4 months ago

  • Component changed from General to Login and Registration

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


3 months ago

#8 @voldemortensen
3 months ago

  • Keywords commit added
  • Type changed from defect (bug) to enhancement

Patch looks good to me, the only nitpick would be a little alignment issue in the docblock.

#9 @jbpaul17
3 months ago

@voldemortensen are you ok with setting you as owner and milestone to 4.9 then?

@ramiy
3 months ago

phpDocs alignment

#10 @SergeyBiryukov
3 months ago

  • Milestone changed from Awaiting Review to 4.9

#11 @voldemortensen
3 months ago

@jbpaul17, sure. It's good to go as soon as someone can commit it.

#12 @voldemortensen
3 months ago

  • Owner set to voldemortensen
  • Status changed from new to accepted

#13 @johnbillion
3 months ago

This needs a @since entry in the function docblock for the new $args options.

@ramiy
3 months ago

Add phpDocs @since entry

#14 @ramiy
3 months ago

@johnbillion Done!

@ramiy
3 months ago

Add @since entry to phpDocs

#15 @voldemortensen
3 months ago

40462.4.2.patch fixed a very small nitpick in the @since entry. I think this is ready to go now.

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


3 months ago

#17 @afercia
3 months ago

  • Focuses accessibility added

#18 @afercia
3 months ago

Worth noting using placeholders as replacement for labels is a bad practice for accessibility. See #40331.

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


3 months ago

#20 follow-up: @afercia
3 months ago

Quoting from the discussion during latest accessibility team meeting on Slack:

@joedolson: Can we see any specific benefit to allowing it? As I see it, it will almost 100% be used to hide labels and use placeholders instead, but if there's a legitimate and beneficial use, I'd be willing to listen to it.

Worth considering WordPress shouldn't encourage accessibility bad practices, and using placeholders as replacement for labels is a bad practice for the reasons explained in #40331.

#21 in reply to: ↑ 20 @ramiy
3 months ago

Replying to afercia:

@joedolson: As I see it, it will almost 100% be used to hide labels and use placeholders instead

Not sure I agree with that direction of thinking. Core decisions should be based on more established arguments. I can easily contradict his claim - in most of my projects I use both labels and placeholders, like many of my colleagues.

Besides, as long as placeholders are valid HTML attributes you cannot tell developers not to use them in their projects.

From accessibility point of view we need to examine how WordPress Core uses placeholders, not how external developers might use them. I can tell you that WordPress core won't be affected from this patch because the default values are empty placeholders.

So why add those attributes? because wp_login_form() , like many other WP functions, is used not only by core but also by tens of thousands of external theme/plugins developers.

#22 @afercia
3 months ago

@ramiy have you read #40331 and all the references posted there? Plenty of more established arguments there.

as long as placeholders are valid HTML attributes you cannot tell developers not to use them in their projects.

RIght, they're valid. Then they should also be used in a valid way. Then, developers should also read the HTML51 specification to start with: https://www.w3.org/TR/html51/sec-forms.html#the-placeholder-attribute

According to the spec, placeholders must be exclusively used for "hins" about the expected format or data type to enter.

https://cldup.com/xgR83MonTx.png

In 99% of the cases, these proposed placeholders will be used as replacements for the labels and the labels will be hidden with CSS or something, and this is something WordPress shouldn't encourage.

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


2 months ago

#24 @joedolson
2 months ago

What kind of content would you imagine using in the placeholder field? I strongly feel I need an example of a valid method of using this before giving this serious consideration for inclusion in core.

The login form is very simple, and the existing labels clearly convey what content is needed in the field. I struggle to see a useful way that a placeholder can be used in this form, but I can clearly see - based on hundreds of form implementations I've seen in the past - that it absolutely *will* be used as a label substitute.

#25 @voldemortensen
2 months ago

I can think of one off the top of my head.

It would be helpful to have the placeholder yourusername@companyname.com for places that require specific email address domains (such as corporate intranet sites).

#26 @ramiy
2 months ago

@joedolson If we concern placeholder will be used as a label substitute, we can add two more $arg to control the label class attribute, this way developers can "hide" the label with screen-reader-text class but kipping them visible for screen readers.

wp_login_form( array(
        'label_username'       => __( 'Email Address' ),
        'label_password'       => __( 'Password' ),
        'label_class_username' => 'screen-reader-text',
        'label_class_password' => 'screen-reader-text',
        'placeholder_username' => __( 'Your username...' ),
        'placeholder_password' => __( 'Your password...' )
) );

Also, we can publish a post in https://make.wordpress.org/accessibility/ talking about "Accessibility best practices when using login forms", describing how to "safely" replace labels with placeholders using screen-reader-text class. It's an educational approach.

@ramiy
2 months ago

add label classes

#27 @afercia
2 months ago

  • Keywords close added; commit removed

It would be helpful to have the placeholder yourusername@… for places that require specific email address domains (such as corporate intranet sites).

Yep, I'd say that's one of the few (maybe only?) correct usage cases of a placeholder attribute in a username/email login form. I'd argue the password field instead can't have a placeholder as providing a sample value or the expected format would be questionable for security reasons.

this way developers can "hide" the label with screen-reader-text class but kipping them visible for screen readers.

@ramly <label> elements are not just for screen reader users. Hiding them, even if only visually, defeats the purpose of properly labeling form fields and it is clear in this case the placeholder will be used as a visual replacement.

describing how to "safely" replace labels with placeholders using screen-reader-text class. It's an educational approach.

there's no "safe" way for the reasons explained above. Labels benefit also sighted users. Instead of educating developers, in this case we should educate users and customers to explain them why they should not hide labels.

WordPress shouldn't encourage bad accessibility practices. For this reason, I'd propose to close this ticket as "wontfix", also considering there's a pending effort to review all the placeholders used in core, see #40331.

#28 @voldemortensen
2 months ago

Going on the record to say I disagree with closing this ticket.

I'd argue the password field instead can't have a placeholder as providing a sample value or the expected format would be questionable for security reasons.

This is a valid point.

I would still like to see a placeholder for the username/email field.

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


2 months ago

#30 @jbpaul17
2 months ago

  • Milestone changed from 4.9 to Future Release

Punting this to Future Release per the 4.9 bug scrub earlier today.

Note: See TracTickets for help on using tickets.