Closed
Bug 1200765
Opened 9 years ago
Closed 9 years ago
Make login UX mobile friendly to assist mobile authentication workflow
Categories
(bugzilla.mozilla.org :: General, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dylan, Assigned: dylan)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
11.93 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
Need to do this for BMO first. The changes to upstream bugzilla will be a bit different due to divergence, and we need this in BMO soon for the 2fa change that makes api keys more of a requirement.
Assignee | ||
Comment 1•9 years ago
|
||
Tested in Firefox for Android, chrome for android, and the Responsive view in firefox desktop. The solution is generalized a bit because we might want the same treatment for error pages. "Request Desktop Site" is honored.
Attachment #8656205 -
Flags: review?(glob)
Summary: Make oauthy type UX mobile friendly → Make login UX mobile friendly to assist mobile authentication workflow
Comment on attachment 8656205 [details] [diff] [review] 1200765_1.patch Review of attachment 8656205 [details] [diff] [review]: ----------------------------------------------------------------- the screen is too wide for iphone 5 and earlier (320 x 568). we should also increase the font size on mobile. ::: .htaccess @@ +91,4 @@ > RewriteRule ^(?:latest|1\.2|1\.3)/(.*)$ extensions/BzAPI/bin/rest.cgi/$1 [NE] > RewriteRule ^bzapi/(.*)$ extensions/BzAPI/bin/rest.cgi/$1 [NE] > RewriteRule ^data/assets/ZeroClipboard.swf(.*)$ extensions/BugModal/web/ZeroClipboard/ZeroClipboard.swf$1 [NE] > +RewriteRule ^login$ index.cgi?GoAheadAndLogIn=1 [NE] this doesn't appear to be used. ::: Bugzilla/Template.pm @@ +1124,5 @@ > + 'is_mobile_browser' => sub { > + my $cgi = Bugzilla->cgi; > + > + return 1; > + return $cgi->user_agent =~ /Mobi/; this will always return true (debugging i guess). nit: no need to create the $cgi var here ::: skins/custom/mobile.css @@ +1,1 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public as this is part of the core header move this from /custom/ to /standard/
Attachment #8656205 -
Flags: review?(glob) → review-
Assignee | ||
Comment 3•9 years ago
|
||
This looks pretty reasonable on an assortment of phones (and emulations of phones).
Attachment #8656205 -
Attachment is obsolete: true
Attachment #8657989 -
Flags: review?(glob)
Comment on attachment 8657989 [details] [diff] [review] 1200765_3.patch Review of attachment 8657989 [details] [diff] [review]: ----------------------------------------------------------------- looks good on mobile, but there's some regressions on desktop that should be fixed. ::: skins/custom/mobile.css @@ +25,5 @@ > + > + body { > + font-size: normal; > + } > +} \ No newline at end of file this file appears to be redundant and should be deleted. ::: template/en/default/account/auth/login.html.tmpl @@ +44,5 @@ > +<div id="login" class="login-form"> > + <form name="login" action="[% target FILTER html %]" method="POST" > + [%- IF Bugzilla.cgi.param("data") %] enctype="multipart/form-data"[% END %]> > + <div class="field-login"> > + <label for="Bugzilla_login">Email Address:</label> the labels should be bold, and vertically centered @@ +46,5 @@ > + [%- IF Bugzilla.cgi.param("data") %] enctype="multipart/form-data"[% END %]> > + <div class="field-login"> > + <label for="Bugzilla_login">Email Address:</label> > + <input id="Bugzilla_login" name="Bugzilla_login" > + [%- ' type="email"' UNLESS Param('emailsuffix') %]> the _login and _password fields are touching - add a gap to avoid this @@ +52,5 @@ > + </div> > + > + <div class="field-password"> > + <label for="Bugzilla_password">Password:</label> > + <input type="password" id="Bugzilla_password" name="Bugzilla_password"> on desktop the width of the fields is now smaller. we have quite a few long email addresses so this now looks weird. i suggest a width of 20em for both fields on non-mobile displays. @@ +62,5 @@ > + <input type="checkbox" id="Bugzilla_remember" name="Bugzilla_remember" value="on" > + [%+ "checked" IF Param('rememberlogin') == "defaulton" %]> > + <label for="Bugzilla_remember" class="checkbox-note"> > + Remember my email address > + </label> on desktop -remember and -restrict should have a 7em left margin to match the current layout @@ +80,5 @@ > > + <div class="field-submit"> > + <input type="hidden" name="Bugzilla_login_token" > + value="[% get_login_request_token() FILTER html %]"> > + <input type="submit" name="GoAheadAndLogIn" value="Sign in" id="log_in"> you've changed "Log in" to "Sign in" - revert this change (we use "Log in" consistently).
Attachment #8657989 -
Flags: review?(glob) → review-
Assignee | ||
Comment 5•9 years ago
|
||
aligned the fields, added padding, etc.
Attachment #8657989 -
Attachment is obsolete: true
Attachment #8669744 -
Flags: review?(glob)
Comment on attachment 8669744 [details] [diff] [review] 1200765_4.patch Review of attachment 8669744 [details] [diff] [review]: ----------------------------------------------------------------- looks much better on desktop - full review later this week. ::: skins/standard/mobile.css @@ +21,5 @@ > + > + #login .field-restrict, #login .field-remember { > + } > + > + remove trailing whitespace
Comment on attachment 8669744 [details] [diff] [review] 1200765_4.patch Review of attachment 8669744 [details] [diff] [review]: ----------------------------------------------------------------- this looks excellent, except the 2fa verification pages also need mobile love (totp just needs larger input elements, see https://www.duosecurity.com/docs/duoweb for duo). ::: skins/custom/mobile.css @@ +1,1 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public as per irc this file should be deleted
Attachment #8669744 -
Flags: review?(glob) → review-
Assignee | ||
Comment 8•9 years ago
|
||
The duo CSS for the iframe looks simple but as duo hasn't been reviewed yet, I have not included it in this patch. This changes the totp elements to be bigger and adds allow_mobile to the verify-totp template.
Attachment #8669744 -
Attachment is obsolete: true
Attachment #8670880 -
Flags: review?(glob)
(In reply to Dylan William Hardison [:dylan] from comment #8) > The duo CSS for the iframe looks simple but as duo hasn't been reviewed yet, > I have not included it in this patch. oh yeah .. i've been in duo land for so long i forgot it isn't in the tree yet :)
Comment 10•9 years ago
|
||
Comment on attachment 8670880 [details] [diff] [review] 1200765_5.patch Review of attachment 8670880 [details] [diff] [review]: ----------------------------------------------------------------- r=glob
Attachment #8670880 -
Flags: review?(glob) → review+
Assignee | ||
Comment 11•9 years ago
|
||
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git 4ce3037..d6f47aa master -> master
Assignee | ||
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•