Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#5914 closed enhancement (fixed)

Registration form: Proposed enhancements for touch devices

Reported by: standardspace Owned by: r-a-y
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Templates Keywords: good-first-bug has-patch
Cc:

Description

I've not submitted a ticket her before so apologies if this is not the correct protocol.
The default theme registration page:
/bp-themes/bp-default/registration/register.php
contains two fields that cause problems on iOS and, potentially, other touch interfaces.

Input to the "signup_username" and "signgup_email" field is auto-capitalised and auto-corrected. In addition the email field is of type="text" rather than type="email" ans so doesn't active the email entry optimised keyboard with the prominent @ symbol.

My suggestions are that:
The "signup_username' field should have the following attributes added:

 autocorrect="off" autocapitalize="off" 

The "signup_email" field has the type changed from "text" to "email"

type="email" 

The the following lines would change from

37	                                        <label for="signup_username"><?php _e( 'Username', 'buddypress' ); ?> <?php _e( '(required)', 'buddypress' ); ?></label>
38	                                        <?php do_action( 'bp_signup_username_errors' ); ?>
39	                                        <input type="text" name="signup_username" id="signup_username" value="<?php bp_signup_username_value(); ?>" />
40	
41	                                        <label for="signup_email"><?php _e( 'Email Address', 'buddypress' ); ?> <?php _e( '(required)', 'buddypress' ); ?></label>
42	                                        <?php do_action( 'bp_signup_email_errors' ); ?>
43	                                        <input type="text" name="signup_email" id="signup_email" value="<?php bp_signup_email_value(); ?>" />

to

37	                                        <label for="signup_username"><?php _e( 'Username', 'buddypress' ); ?> <?php _e( '(required)', 'buddypress' ); ?></label>
38	                                        <?php do_action( 'bp_signup_username_errors' ); ?>
39	                                        <input type="text" name="signup_username" id="signup_username" value="<?php bp_signup_username_value(); ?>" autocorrect="off" autocapitalize="off" />
40	
41	                                        <label for="signup_email"><?php _e( 'Email Address', 'buddypress' ); ?> <?php _e( '(required)', 'buddypress' ); ?></label>
42	                                        <?php do_action( 'bp_signup_email_errors' ); ?>
43	                                        <input type="email" name="signup_email" id="signup_email" value="<?php bp_signup_email_value(); ?>" />

Attachments (3)

5914.bp_signup_field_attributes.patch (4.1 KB) - added by r-a-y 5 years ago.
5914.01.patch (7.7 KB) - added by r-a-y 5 years ago.
5914.02.patch (8.7 KB) - added by r-a-y 5 years ago.

Download all attachments as: .zip

Change History (26)

#1 @boonebgorges
5 years ago

  • Milestone changed from Awaiting Review to 2.2

Great suggestions - thanks, standardspace. Let's look at these for 2.2.

#2 @DJPaul
5 years ago

  • Keywords good-first-bug added

#3 @boonebgorges
5 years ago

In 9225:

Use 'type="email"' for email inputs in registration and settings templates.

This provides a better experience on mobile devices and other browsers that
support the 'email' type.

Props standardspace.
See #5914.

#4 @boonebgorges
5 years ago

Circling back around to this ticket.

The type="email" suggestion is a no-brainer. Implemented in [9225].

I'm hesitating somewhat on autocomplete and autocorrect. I personally use similar usernames for different user accounts across the web, and when registering, autocomplete and autocorrect on the username fields is sometimes a helpful convenience.

I wonder if, for username fields, what we really want is spellcheck="false" and autocapitalize="off". Anyone else have thoughts about this?

#5 @hnla
5 years ago

I would agree on principle with not removing autocomplete, but as for the others they are all non standard attributes, would rather not consider them at all, and thus doing nothing here on this score. If devs want to use them then build them in using a wp_is_mobile() check or something similar to swap them in.

#6 @r-a-y
5 years ago

For a comparison, Twitter sets autocomplete="off" for all its registration form fields on both desktop and mobile versions of the site.

Facebook uses the following only on their mobile site:

  • Password field - spellcheck="false" autocomplete="off" autocapitalize="off" autocorrect="off"
  • DOB (text fields) - autocomplete="off" autocapitalize="off" autocorrect="off"
  • Email - autocapitalize="off"
  • First name / surname - autocapitalize="on"

autocorrect and autocapitalize appear to be Safari-only attributes. Chromium has added autocorrect support: https://code.google.com/p/chromium/issues/detail?id=303883.

spellcheck is part of HTML5.


I think autocomplete and spellcheck are a no-brainer on the password field.

autocomplete can also be used on the username field. The others I'm not so sure about. If we don't care about W3C validation, I would also add in autocorrect as it's becoming a mobile standard.

Version 3, edited 5 years ago by r-a-y (previous) (next) (diff)

#7 follow-up: @boonebgorges
5 years ago

r-a-y - This is really helpful - thanks.

hnla - Aside from the fact that some of these are vendor-specific, which is kind of icky from an aesthetic point of view, are there any negative consequences of adding these attributes? This seems like a case where the "nonstandard" nature of the attributes may well be outweighed by the benefits to the user.

#8 in reply to: ↑ 7 @hnla
5 years ago

Replying to boonebgorges:

hnla - Aside from the fact that some of these are vendor-specific, which is kind of icky from an aesthetic point of view, are there any negative consequences of adding these attributes? This seems like a case where the "nonstandard" nature of the attributes may well be outweighed by the benefits to the user.

I hear your point, there are times when one weighs up benefits Vs Standards, then again one would expect that if these are truly useful attributes that they might be under CR in which case I would have less qualms about using them.

Upholding Standards has been a long hard struggle which continues to this day and likely for evermore, not so much a concern of 'ickynes' as a concern of the slightest return to the code free for all that existed in the early days of web development.

To answer the question though, no likely as not there won't be any negative consequences and I have to acknowledge that the larger entities on the web delivering api's deem it perfectly fine to 'invent' attributes as suits their purposes regardless that they exist in any formal schema.

As to us employing these I'm still not sure it feels right, but as r-a-y points out some of these are mobile specific so we would need to apply them only if mobile device detected really, either using wp_is_mobile() or building our own device detection class; not sure I'm happy to have BP force these on me as as a web dev building a site as I might prefer to make my own judgement call on this sort of thing?

I wonder if, for username fields, what we really want is spellcheck="false" and autocapitalize="off"

As this was your earlier suggestion i.e to use these but not autocomplete='off' I doubt there would be particular issues, but again, ideally would like them set conditionally.

Checking again and 'spellcheck' is actually a W3C CR while 'autocapitalize' remains proprietary - My personal view is that while it remains proprietary and simply to aid a particular UA it should not be used; 'false' ought to have been the default really and thus is more their issue to resolve :) N.B I believe the value has been updated from a boolean one to keyword 'none'

#9 @boonebgorges
5 years ago

Thanks for the feedback, hnla. Since 'spellcheck' is a W3C CR, I think we should use it where appropriate.

Regarding the proprietary attributes. We are different from "the larger entities" in that we are distributing software rather than a website. If browser implementations change tomorrow, Facebook can change their registration page in response. We're distributing something that we no longer have control over once it's out of our hands. So, aside from the ideological reasons for not using propietary/nonstandard attributes, we also have practical reasons for being conservative.

All that being said, I'm pretty much on the fence. Adding these attributes would improve user experience in a non-trivial way. But I want us to be good web citizens too. If anyone wants to step in and break this deadlock, please do :)

#10 @r-a-y
5 years ago

  • Keywords has-patch added

As to us employing these I'm still not sure it feels right, but as r-a-y points out some of these are mobile specific so we would need to apply them only if mobile device detected really, either using wp_is_mobile() or building our own device detection class

Any type of wp_is_mobile() checks will require us to make some type of helper function like bp_signup_field_attributes() to output the attributes with the function being called in the registration template.

5914.bp_signup_field_attributes.patch is a patch based off this idea with the following set:

  • Password - spellcheck="false" autocomplete="off" autocapitalize="off" autocorrect="off"
  • Email - autocapitalize="off"
  • Username - autocomplete="off" (not adding autocapitalize here because BP's username compatibility might be on, and WP accepts uppercase characters in this case).

#11 @hnla
5 years ago

@r-a-y Brilliant, nice patch. Think this approach justifies us adding these attributes then, accounting for mobile devices only, and filterable so we can modify if necessary.
(taking things to the nth degree and only half suggesting this we could also test for webkit devices, there's quite a good open source class that tests for all manner of devices we could include, but it takes us into new realms that we probably don't want to head towards.)

Couple of points that spring to mind:

1/ Should we not maybe rename the function from 'bp_signup_field_attributes' to ? 'bp_touchdevice_input_attributes' suggesting this as we also need to take into account user settings password/email updating, and also as a helper function is it not more useful if the function is less specific to a screen or action/component than to the purpose the function returns, i.e we may have need of this function again perhaps in the future for form inputs yet to be written but also devs/plugin authors might then use the function for their custom form controls?

2/ Password fields: the default 'autocapitalize' value for general inputs is 'sentences' however for inputs of type="password" the default is 'none', so in theory we need not specify the attribute on password fields technically, this is though for ios >= 5

3/ 'off' is deprecated in favour of 'none' again for ios => 5 so do we we run with deprecated values or take a decisions to support specific versions? It's another factor I'm uncomfortable with having BP now making decisions to support specific devices and needing to review that from time to time?

4/ username fields: I think - but may have read things wrong - that we may actually need to describe 'autocapitalize' here as the default might take effect, we need this 'auto' behaviour disabled leaving the choice to capitalize in the users hands.

Webkit/dveloper.apple reference:
https://developer.apple.com/library/safari/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html

#12 @boonebgorges
5 years ago

Thanks for the patch, r-a-y!

Should we not maybe rename the function

I agree - presumably we'd use the same function on the settings (and xprofile?) templates. In theory, it could also be used for things other than touch-screen-specific attributes (and in fact, r-a-y's patch applies a couple attributes even when not on mobile). How about bp_settings_field_attributes() or even bp_form_field_attributes()? Like hnla suggests, no reason to prevent this useful function from being used by theme authors.

do we we run with deprecated values or take a decisions to support specific versions? It's another factor I'm uncomfortable with having BP now making decisions to support specific devices and needing to review that from time to time?

Yeah, this is a rabbit hole in general. But in this specific case, it seems pretty clear that we should use 'none'. iOS 5 has been obsolescent for a couple years.

we may actually need to describe 'autocapitalize' here as the default might take effect, we need this 'auto' behaviour disabled leaving the choice to capitalize in the users hands.

If I read this correctly, you're saying that 'username' should get 'autocapitalize=none'. I agree.

hnla, thanks for lending your expertise here!

#13 @r-a-y
5 years ago

01.patch is updated with everyone's recommendations.

Is the function name bp_form_field_extra_attributes() too verbose?

@r-a-y
5 years ago

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

#15 @johnjamesjacoby
5 years ago

  • Keywords commit added

This is great, and goes a long way towards funneling common form attributes into one easy to use helper. Like Boone suggested, XProfile would benefit from this pretty hugely, so I like that we accept a $type parameter and filters to help make this more flexible later.

I tried thinking of different function names without _extra_ but can't think or find anything I like more, so this seems like a great addition to me. Thanks everyone!

#16 @johnjamesjacoby
5 years ago

I don't see any reason we can't add this to our XProfile output now, and pass the $field type through it. Even though we aren't handling anything yet, it automatically enables extending the field attributes without needing to override template output.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

@r-a-y
5 years ago

#18 @r-a-y
5 years ago

  • Keywords commit removed

When looking to add in XProfile support, I came across BP_XProfile_Type::get_edit_field_html_elements(), which does something similar to what was in 01.patch.

In order to avoid duplication of code, 02.patch flip-flops the function name back to bp_form_field_attributes() and adds a second argument for attributes you want to pass for output.

For XProfile, the field name is passed rather than the type. For example, if I created a XProfile textbox field called "College", this is what the call looks like:

bp_get_form_field_attributes( 'college', $existing_attributes );

This makes more sense because if you passed 'textbox', you couldn't target individual fields.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

#20 @r-a-y
5 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 9329:

Core: Introduce bp_form_field_attributes() function.

bp_form_field_attributes() is a helper function to output attributes for
a given field.

The function:

  • Sets some default attributes for the username, email and password input fields to better support touchscreen devices.
  • Allows plugin developers to arbitrarily filter the attributes for the given field.
  • Is used in BP_XProfile_Type::get_edit_field_html_elements() to better filter fields by input name.

Props hnla, standardspace, boonebgorges, johnjamesjacoby.

Fixes #5914.

#21 @r-a-y
5 years ago

In 9345:

Core: Do not use array_unique() in bp_get_form_field_attributes().

array_unique() merged the id and name fields when the form values
were identical, which is way wrong and broke the saving of certain
xprofile fields.

Props hnla. Anti-props r-a-y.

See #5914.

#22 @boonebgorges
5 years ago

In 9612:

Use bp_form_field_attributes( 'password' ) to print autocomplete=off attribute in login widget.

This is better than hardcoding. See #5914.

Props r-a-y.
Fixes #6269.

#23 @DJPaul
3 years ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.