From cc5798896c48c51f5fb5389514da64f60439508e Mon Sep 17 00:00:00 2001
From: Shane Tomlinson <stomlinson@mozilla.com>
Date: Fri, 28 Oct 2011 09:47:20 +0100
Subject: [PATCH] Finishing up work on the error message display.
* Cleaning up the messages
* Adding expanded info
* Adding network response codes and jQuery error message
* Styling
* Adding tests for expected behaviors
close #448
---
.../dialog/controllers/page_controller.js | 10 +++++
browserid/static/dialog/css/popup.css | 14 ++++--
.../static/dialog/resources/error-messages.js | 36 ++++++----------
browserid/static/dialog/resources/network.js | 8 +++-
.../controllers/page_controller_unit_test.js | 31 +++++++++++++
.../test/qunit/resources/network_unit_test.js | 6 ++-
browserid/static/dialog/views/error.ejs | 43 +++++++++++++------
browserid/views/unsupported_dialog.ejs | 2 +-
8 files changed, 106 insertions(+), 44 deletions(-)
diff --git a/browserid/static/dialog/controllers/page_controller.js b/browserid/static/dialog/controllers/page_controller.js
index 37389fb1f..70113e8ad 100644
--- a/browserid/static/dialog/controllers/page_controller.js
+++ b/browserid/static/dialog/controllers/page_controller.js
@@ -109,6 +109,16 @@
this.renderTemplates("#error", body, body_vars);
$("body").removeClass("waiting").removeClass("form").addClass("error");
$("#error").stop().css('opacity', 1).hide().fadeIn(ANIMATION_TIME);
+
+ /**
+ * What a big steaming pile, use CSS animations for this!
+ */
+ $("#openMoreInfo").click(function(event) {
+ event.preventDefault();
+
+ $("#moreInfo").slideDown();
+ $("#openMoreInfo").css({visibility: "hidden"});
+ });
},
onSubmit: function(event) {
diff --git a/browserid/static/dialog/css/popup.css b/browserid/static/dialog/css/popup.css
index 72f7272fa..46f393cec 100644
--- a/browserid/static/dialog/css/popup.css
+++ b/browserid/static/dialog/css/popup.css
@@ -131,27 +131,35 @@ section > .contents {
background-color: #fff;
}
+#error ul, #error li {
+ list-style-type: none;
+}
+
#wait strong, #error strong {
color: #222;
font-weight: bold;
}
-#error .vertical {
+#error.unsupported .vertical {
width: 630px;
margin: 0 auto;
display: block;
}
-#error .vertical > div {
+#error.unsupported .vertical > div {
display: table-cell;
vertical-align: middle;
padding: 0 10px;
height: 250px;
}
-#error #alternative a {
+#error #moreInfo {
+ display: none;
+}
+
+#error a {
color: #549FDC;
text-decoration: underline;
}
diff --git a/browserid/static/dialog/resources/error-messages.js b/browserid/static/dialog/resources/error-messages.js
index 4235913d9..a35b91a18 100644
--- a/browserid/static/dialog/resources/error-messages.js
+++ b/browserid/static/dialog/resources/error-messages.js
@@ -37,38 +37,31 @@ BrowserID.Errors = (function(){
var Errors = {
authenticate: {
- title: "Error Authenticating",
- message: "There was a technical problem while trying to log you in. Yucky!"
+ title: "Authenticating User"
},
addEmail: {
- title: "Error Adding Address",
- message: "There was a technical problem while trying to add this email to your account. Yucky!"
+ title: "Adding Address"
},
checkAuthentication: {
- title: "Error Checking Authentication",
- message: "There was a technical problem while trying to log you in. Yucky!"
+ title: "Checking Authentication"
},
createUser: {
- title: "Error Creating Account",
- message: "There was a technical problem while trying to create your account. Yucky!"
+ title: "Creating Account"
},
getAssertion: {
- title: "Error Getting Assertion",
- message: "There was a technical problem while trying to authenticate you. Yucky!"
+ title: "Getting Assertion"
},
isEmailRegistered: {
- title: "Error Checking Email Address",
- message: "There was a technical problem while trying to check that email address. Yucky!"
+ title: "Checking Email Address"
},
logoutUser: {
- title: "Logout Failed",
- message: "An error was encountered while signing you out. Yucky!"
+ title: "Logout Failed"
},
offline: {
@@ -77,28 +70,23 @@ BrowserID.Errors = (function(){
},
registration: {
- title: "Registration Failed",
- message: "An error was encountered and the signup cannot be completed. Yucky!"
+ title: "Registration Failed"
},
requestPasswordReset: {
- title: "Error Resetting Password",
- message: "There was a technical problem while trying to reset your password."
+ title: "Resetting Password"
},
signIn: {
- title: "Signin Failed",
- message: "There was an error signing in. Yucky!"
+ title: "Signin Failed"
},
syncAddress: {
- title: "Error Syncing Address",
- message: "There was a technical problem while trying to synchronize your account. Yucky!"
+ title: "Syncing Address"
},
xhrError: {
- title: "Communication Error",
- message: "It's embarrassing, but for some reason we cannot communicate with BrowserID at the moment. Please try again."
+ title: "Communication Error"
}
};
diff --git a/browserid/static/dialog/resources/network.js b/browserid/static/dialog/resources/network.js
index dadbe3085..855a349ba 100644
--- a/browserid/static/dialog/resources/network.js
+++ b/browserid/static/dialog/resources/network.js
@@ -55,7 +55,13 @@ BrowserID.Network = (function() {
}
function xhrError(cb, info) {
- return function() {
+ return function(jqXHR, textStatus, errorThrown) {
+ info = info || {};
+ var network = info.network = info.network || {};
+
+ network.textStatus = textStatus;
+ network.errorThrown = errorThrown;
+
if (cb) cb(info);
hub && hub.publish("xhrError", info);
};
diff --git a/browserid/static/dialog/test/qunit/controllers/page_controller_unit_test.js b/browserid/static/dialog/test/qunit/controllers/page_controller_unit_test.js
index a7bc08e0e..299b8ed00 100644
--- a/browserid/static/dialog/test/qunit/controllers/page_controller_unit_test.js
+++ b/browserid/static/dialog/test/qunit/controllers/page_controller_unit_test.js
@@ -141,6 +141,37 @@ steal.plugins("jquery").then("/dialog/controllers/page_controller", function() {
ok(html.length, "with error template specified, error text is loaded");
});
+ test("renderError allows us to open expanded error info", function() {
+ controller = el.page({
+ waitTemplate: waitTemplate,
+ waitVars: {
+ title: "Test title",
+ message: "Test message"
+ }
+ }).controller();
+
+ controller.renderError("error.ejs", {
+ action: {
+ title: "expanded action info",
+ message: "expanded message"
+ }
+ });
+
+ var html = el.find("#error .contents").html();
+
+ $("#moreInfo").hide();
+
+ $("#openMoreInfo").click();
+
+ setTimeout(function() {
+ equal($("#showMoreInfo").is(":visible"), false, "button is not visible after clicking expanded info");
+ equal($("#moreInfo").is(":visible"), true, "expanded error info is visible after clicking expanded info");
+ start();
+ }, 500);
+
+ stop();
+ });
+
test("getErrorDialog gets a function that can be used to render an error message", function() {
controller = el.page({
waitTemplate: waitTemplate,
diff --git a/browserid/static/dialog/test/qunit/resources/network_unit_test.js b/browserid/static/dialog/test/qunit/resources/network_unit_test.js
index 74d35d40f..d0a8bda50 100644
--- a/browserid/static/dialog/test/qunit/resources/network_unit_test.js
+++ b/browserid/static/dialog/test/qunit/resources/network_unit_test.js
@@ -65,6 +65,8 @@ steal.plugins("jquery", "funcunit/qunit").then("/dialog/resources/network", func
ok(true, "xhr error notified application");
ok(info.network.url, "url is in network info");
ok(info.network.type, "request type is in network info");
+ equal(info.network.textStatus, "errorStatus", "textStatus is in network info");
+ equal(info.network.errorThrown, "errorThrown", "errorThrown is in response info");
wrappedStart();
OpenAjax.hub.unsubscribe(handle);
};
@@ -91,6 +93,8 @@ steal.plugins("jquery", "funcunit/qunit").then("/dialog/resources/network", func
ok(true, "XHR failure should never pass");
ok(info.network.url, "url is in network info");
ok(info.network.type, "request type is in network info");
+ equal(info.network.textStatus, "errorStatus", "textStatus is in network info");
+ equal(info.network.errorThrown, "errorThrown", "errorThrown is in response info");
wrappedStart();
});
@@ -193,7 +197,7 @@ steal.plugins("jquery", "funcunit/qunit").then("/dialog/resources/network", func
else if (obj.error) {
// Invalid result - either invalid URL, invalid GET/POST or
// invalid resultType
- obj.error();
+ obj.error({}, "errorStatus", "errorThrown");
}
}
}
diff --git a/browserid/static/dialog/views/error.ejs b/browserid/static/dialog/views/error.ejs
index 336fc1d4c..537d2471f 100644
--- a/browserid/static/dialog/views/error.ejs
+++ b/browserid/static/dialog/views/error.ejs
@@ -1,27 +1,42 @@
- <h2>There has been an error!</h2>
+ <h2>We are very sorry, there has been an error!</h2>
<p>
- We are very sorry, but there has been an error. You can open the error message below
- if you care to find out more information. To retry, you will have to close BrowserID and try again.
+ To retry, you will have to close BrowserID and try again. More info can be found by opening the expanded info below.
</p>
- <a href="#">See more info</a>
+ <a href="#" id="openMoreInfo">See more info</a>
- <aside>
+ <ul id="moreInfo">
<% if (typeof action !== "undefined") { %>
- <h3 id="action">Action: <%= action.title %></h3>
- <p>
- <%= action.message %>
- </p>
+ <li>
+ <strong id="action">Action: </strong><%= action.title %>
+
+ <% if (action.message) { %>
+ <p>
+ <%= action.message %>
+ </p>
+ <% } %>
+ </li>
<% } %>
<% if (typeof network !== "undefined") { %>
- <h3 id="network">Network Info: </h3>
- <p>
- <%= network.type %>: <%= network.url %>
- </p>
+ <li>
+
+ <strong id="network">Network Info:</strong> <%= network.type %>: <%= network.url %>
+
+ <p>
+ <strong>Response Code - </strong> <%= network.textStatus %>
+ </p>
+
+ <% if (network.errorThrown) { %>
+ <p>
+ <strong>Error Type:</strong> <%= network.errorThrown %>
+ </p>
+ <% } %>
+
+ </li>
<% } %>
- </aside>
+ </ul>
diff --git a/browserid/views/unsupported_dialog.ejs b/browserid/views/unsupported_dialog.ejs
index acf52333b..eac8bf8ad 100644
--- a/browserid/views/unsupported_dialog.ejs
+++ b/browserid/views/unsupported_dialog.ejs
@@ -1,4 +1,4 @@
- <section id="error" style="display: block">
+ <section id="error" style="display: block" class="unsupported">
<div class="table">
<div class="vertical contents">
<div id="reason">
--
GitLab