launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/noticeable-add-node-errors into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/noticeable-add-node-errors/+merge/99256

Okay, I'll admit it: this is not my work.

Francis added a comment on a different but related bug I was fixing, pointing out that global form errors on the add-node form (as distinct from field-specific errors) don't stand out at all.  You'll almost certainly miss them because they appear in the same style as regular text.

Raphaël then wrote up a diff for the CSS, and posted a link to it on my MP.

All I did was more or less hand-apply the diff.  This may conflict with trunk initially, since my other branch will land in the meantime.

We may still have to reconsider the styling: to me, the styling of errors is very hard to distinguish from that of the “add another MAC” action link on the same form.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/noticeable-add-node-errors/+merge/99256
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/noticeable-add-node-errors into lp:maas.

=== modified file 'src/maasserver/static/css/forms.css'
--- src/maasserver/static/css/forms.css	2012-03-05 05:52:32 +0000
+++ src/maasserver/static/css/forms.css	2012-03-26 05:23:18 +0000
@@ -12,7 +12,8 @@
     text-align: right;
     }
 .form-errors,
-form .errors {
+form .errors,
+form .field-error {
     color: #DE3E25;
     }
 label {

=== modified file 'src/maasserver/static/js/node_add.js'
--- src/maasserver/static/js/node_add.js	2012-03-26 04:48:59 +0000
+++ src/maasserver/static/js/node_add.js	2012-03-26 05:23:18 +0000
@@ -122,7 +122,7 @@
             .set('name', 'op')
             .set('value', 'new');
         var global_error = Y.Node.create('<p />')
-            .addClass('form-global-errors');
+            .addClass('form-errors');
         var addnodeform = Y.Node.create('<form />')
             .set('method', 'post')
             .append(global_error)
@@ -153,8 +153,7 @@
             error_node = error;
         }
 
-        this.get(
-            'srcNode').one('.form-global-errors').empty().append(error_node);
+        this.get('srcNode').one('.form-errors').empty().append(error_node);
      },
 
    /**

=== modified file 'src/maasserver/static/js/tests/test_node_add.js'
--- src/maasserver/static/js/tests/test_node_add.js	2012-03-26 04:48:59 +0000
+++ src/maasserver/static/js/tests/test_node_add.js	2012-03-26 05:23:18 +0000
@@ -179,6 +179,7 @@
         Y.Assert.isTrue(fired);
     },
 
+<<<<<<< TREE
     testValidationErrorInJSONGoesToFieldsNotGlobalErrors: function() {
         this.mockFailure('{"architecture": ["Xur."]}', module, 400);
         submit_add_node();
@@ -194,12 +195,44 @@
         submit_add_node();
         var error_message = find_global_errors().get('innerHTML');
         Y.Assert.areNotEqual(-1, error_message.search("Blergh."));
+=======
+    testGenericErrorMessage: function() {
+        var mockXhr = new Y.Base();
+        mockXhr.send = function(url, cfg) {
+            cfg.on.failure(3, {status: 500});
+        };
+        this.mockIO(mockXhr, module);
+        module.showAddNodeWidget();
+        var panel = module._add_node_singleton;
+        panel.get('srcNode').one('#id_hostname').set('value', 'host');
+        var button = panel.get('srcNode').one('.yui3-button');
+        button.simulate('click');
+        var error_message = panel.get('srcNode').one('.form-errors').get(
+            'innerHTML');
+        var message_position = error_message.search("Unable to create Node.");
+        Y.Assert.areNotEqual(-1, error_message);
+>>>>>>> MERGE-SOURCE
     },
 
     testLoggedOffErrorMessage: function() {
+<<<<<<< TREE
         this.mockFailure("You are not logged in.", module, 401);
         submit_add_node();
         var error_message = find_global_errors().get('innerHTML');
+=======
+        var mockXhr = new Y.Base();
+        mockXhr.send = function(url, cfg) {
+            cfg.on.failure(3, {status: 401});
+        };
+        this.mockIO(mockXhr, module);
+        module.showAddNodeWidget();
+        var panel = module._add_node_singleton;
+        panel.get('srcNode').one('#id_hostname').set('value', 'host');
+        var button = panel.get('srcNode').one('.yui3-button');
+        button.simulate('click');
+        var error_message = panel.get('srcNode').one('.form-errors').get(
+            'innerHTML');
+>>>>>>> MERGE-SOURCE
         // The link to the login page is present in the error message.
         var link_position = error_message.search(MAAS_config.uris.login);
         Y.Assert.areNotEqual(-1, link_position);