#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)
Change History (26)
#4
@
10 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
@
10 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
@
10 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.
#7
follow-up:
↓ 8
@
10 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
@
10 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
@
10 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
@
10 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 addingautocapitalize
here because BP's username compatibility might be on, and WP accepts uppercase characters in this case).
#11
@
10 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
@
10 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
@
10 years ago
01.patch
is updated with everyone's recommendations.
Is the function name bp_form_field_extra_attributes()
too verbose?
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
10 years ago
#15
@
10 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
@
10 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.
10 years ago
#18
@
10 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.
Great suggestions - thanks, standardspace. Let's look at these for 2.2.