From 9751db1805f7a3ab3e4987b88769d867121e3881 Mon Sep 17 00:00:00 2001 From: Shane Tomlinson <stomlinson@mozilla.com> Date: Tue, 24 Jul 2012 17:44:21 +0100 Subject: [PATCH] Usability updates to the signup page. * If the user enters an unknown secondary email, redirect them to the /signup page. * If the user arrives with a known secondary email in storage, show the password field. --- resources/static/pages/css/style.css | 5 +- resources/static/pages/js/signin.js | 21 +++-- .../static/test/cases/pages/js/signin.js | 87 ++++++++++++++++--- resources/static/test/testHelpers/helpers.js | 9 ++ resources/views/signin.ejs | 8 -- 5 files changed, 100 insertions(+), 30 deletions(-) diff --git a/resources/static/pages/css/style.css b/resources/static/pages/css/style.css index e61db1ce5..fa2028b93 100644 --- a/resources/static/pages/css/style.css +++ b/resources/static/pages/css/style.css @@ -462,7 +462,7 @@ button.delete:active { margin-bottom: 10px; } -.siteinfo, #congrats, .password_entry, #verify_password, .enter_password .hint, #unknown_secondary, .verify_primary .forminputs { +.siteinfo, #congrats, .password_entry, #verify_password, .enter_password .hint, .verify_primary .forminputs { display: none; } @@ -478,8 +478,7 @@ button.delete:active { font-size: 13px; } -.enter_password .password_entry, .enter_verify_password #verify_password, .known_secondary .password_entry, -.unknown_secondary #unknown_secondary, .verify_primary #verify_primary { +.enter_password .password_entry, .enter_verify_password #verify_password, .known_secondary .password_entry, .verify_primary #verify_primary { display: block; } diff --git a/resources/static/pages/js/signin.js b/resources/static/pages/js/signin.js index edc585011..918be1d87 100644 --- a/resources/static/pages/js/signin.js +++ b/resources/static/pages/js/signin.js @@ -16,16 +16,13 @@ BrowserID.signIn = (function() { tooltip = bid.Tooltip, doc = document, winchan = window.WinChan, + complete = helpers.complete, verifyEmail, verifyURL, addressInfo, sc, lastEmail; - function complete(oncomplete, status) { - oncomplete && oncomplete(status); - } - function provisionPrimaryUser(email, info, callback) { // primary user who is authenticated with the primary. user.provisionPrimaryUser(email, info, function(status, provInfo) { @@ -44,6 +41,7 @@ BrowserID.signIn = (function() { } function emailSubmit(oncomplete) { + /*jshint validthis: true*/ var self=this, email = helpers.getAndValidateEmail("#email"); @@ -60,8 +58,7 @@ BrowserID.signIn = (function() { self.submit = passwordSubmit; } else { - dom.setInner("#unknown_email", email); - dom.addClass("body", "unknown_secondary"); + doc.location = "/signup"; } complete(oncomplete); @@ -126,6 +123,8 @@ BrowserID.signIn = (function() { } function onEmailChange(event) { + /*jshint validthis: true*/ + // this is basically a state reset. var email = dom.getInner("#email"); if(email !== lastEmail) { @@ -151,6 +150,16 @@ BrowserID.signIn = (function() { self.bind("#email", "keyup", onEmailChange); sc.start.call(self, options); + + // If there is an email already set up in pageHelpers.setupEmail, see if + // the email address is a primary, secondary, known or unknown. Redirect + // if needed. + if (dom.getInner("#email")) { + self.submit(options.ready); + } + else { + complete(options.ready); + } }, submit: emailSubmit diff --git a/resources/static/test/cases/pages/js/signin.js b/resources/static/test/cases/pages/js/signin.js index 9e07840d1..161e10d29 100644 --- a/resources/static/test/cases/pages/js/signin.js +++ b/resources/static/test/cases/pages/js/signin.js @@ -12,21 +12,35 @@ xhr = bid.Mocks.xhr, WinChanMock = bid.Mocks.WinChan, provisioning = bid.Mocks.Provisioning, + WindowMock = bid.Mocks.WindowMock, testHelpers = bid.TestHelpers, - docMock = { - location: "signin" - }, + testDocumentRedirected = testHelpers.testDocumentRedirected, + testDocumentNotRedirected = testHelpers.testDocumentNotRedirected, + testHasClass = testHelpers.testHasClass, + pageHelpers = bid.PageHelpers, + docMock, controller, winchan; + function createController(options) { + winchan = new WinChanMock(); + docMock = new WindowMock().document; + + options = options || {}; + _.extend(options, { + document: docMock, + winchan: winchan + }); + + controller = bid.signIn.create(); + controller.start(options); + } + module("pages/js/signin", { setup: function() { testHelpers.setup(); - docMock.location = "signin"; bid.Renderer.render("#page_head", "site/signin", {}); - winchan = new WinChanMock(); - controller = bid.signIn.create(); - controller.start({document: docMock, winchan: winchan}); + createController(); }, teardown: function() { testHelpers.teardown(); @@ -36,12 +50,60 @@ function testUserNotSignedIn(extraTests) { controller.passwordSubmit(function() { - equal(docMock.location, "signin", "user not signed in"); + testDocumentNotRedirected(docMock, "user not signed in"); if (extraTests) extraTests(); start(); }); } + asyncTest("start with no email stored - nothing fancy", function() { + createController({ + ready: function() { + testDocumentNotRedirected(docMock, "user not signed in"); + start(); + } + }); + }); + + asyncTest("start with unknown secondary email stored - redirect to /signup", function() { + xhr.useResult("unknown_secondary"); + pageHelpers.setStoredEmail("unregistered@testuser.com"); + createController({ + ready: function() { + testDocumentRedirected(docMock, "/signup", "user sent to /signup page"); + start(); + } + }); + }); + + asyncTest("start with known secondary email stored - show password", function() { + xhr.useResult("known_secondary"); + pageHelpers.setStoredEmail("registered@testuser.com"); + createController({ + ready: function() { + testHasClass("body", "known_secondary", "known_secondary class added to body"); + testDocumentNotRedirected(docMock); + + start(); + } + }); + }); + + asyncTest("start with known primary email stored - show verify primary", function() { + xhr.useResult("primary"); + provisioning.setStatus(provisioning.NOT_AUTHENTICATED); + pageHelpers.setStoredEmail("registered@testuser.com"); + + createController({ + ready: function() { + testHasClass("body", "verify_primary", "verify_primary class added to body"); + + testDocumentNotRedirected(docMock); + start(); + } + }); + }); + asyncTest("emailSubmit with invalid email - show tooltip", function() { controller.emailSubmit(function() { testHelpers.testTooltipVisible(); @@ -59,13 +121,12 @@ }); }); - asyncTest("unknown_secondary: emailSubmit - add unknown_secondary to body", function() { + asyncTest("unknown_secondary: emailSubmit - redirect to /signup", function() { xhr.useResult("unknown_secondary"); $("#email").val("unregistered@testuser.com"); controller.emailSubmit(function() { - equal($("body").hasClass("unknown_secondary"), true, "unknown_secondary class added to body"); - equal(controller.submit, controller.emailSubmit, "submit remains emailSubmit"); + testDocumentRedirected(docMock, "/signup", "user redirected to /signup"); start(); }); }); @@ -75,7 +136,7 @@ $("#email").val("registered@testuser.com"); controller.emailSubmit(function() { - equal($("body").hasClass("known_secondary"), true, "known_secondary class added to body"); + testHasClass("body", "known_secondary", "known_secondary class added to body"); equal(controller.submit, controller.passwordSubmit, "submit has changed to passwordSubmit"); start(); }); @@ -101,7 +162,7 @@ $("#email").val("registered@testuser.com"); controller.emailSubmit(function() { - equal($("body").hasClass("verify_primary"), true, "verify_primary class added to body"); + testHasClass("body", "verify_primary", "verify_primary class added to body"); equal(controller.submit, controller.authWithPrimary, "submit updated to authWithPrimary"); start(); }); diff --git a/resources/static/test/testHelpers/helpers.js b/resources/static/test/testHelpers/helpers.js index ce7ee1174..25f9adc99 100644 --- a/resources/static/test/testHelpers/helpers.js +++ b/resources/static/test/testHelpers/helpers.js @@ -330,6 +330,15 @@ BrowserID.TestHelpers = (function() { var emailInfo = storage.getEmail(email); equal(emailInfo && emailInfo.verified, true, "verified bit set for " + email); + }, + + testDocumentRedirected: function(doc, expectedHref, msg) { + equal(doc.location, expectedHref, msg || "document redirected to " + expectedHref); + }, + + testDocumentNotRedirected: function(doc, msg) { + equal(doc.location.href, document.location.href, msg || "document not redirected"); + } }; diff --git a/resources/views/signin.ejs b/resources/views/signin.ejs index 38cb227c5..1db91fa2c 100644 --- a/resources/views/signin.ejs +++ b/resources/views/signin.ejs @@ -8,14 +8,6 @@ <form id="signUpForm" class="cf authform" novalidate> <h1><%- gettext('Sign In') %></h1> - <ul class="notifications"> - <li class="notification" id="unknown_secondary"> - <%- format(gettext('<strong %(emailId)>Email</strong> is not registered. Would you like to <a %(signUpLink)>sign up</a> instead?'), - { emailId: 'id="unknown_email"', signUpLink: 'class="action" href="/signup"' }) %> - </li> - - </ul> - <ul class="inputs"> <li> <label for="email"><%- gettext('Email Address') %></label> -- GitLab