Merge lp:~wallyworld/launchpad/bug-javascript-1011611 into lp:launchpad

[Ian Booth]

Ensure edit icons appear on file bug choice popups on Chrome, fix rendering issues on bug privacy portlet.

== Implementation ==

Fix markup generated for the filebug edit icons to remove a ' ' between the span and anchor.
Ensure the bug privacy portlet lock icon is updated when the popup value is saved, like is done for description.
Fix error rendering - disable standard choice popup animation and render the success and error animations manually. These callbacks are hard wired into the widget so a few tweaks to the widget were required. Previously, nothing in the lp codebase asked the choice popup to render the error condition.

== Tests ==

Update yui tests for information type javascript.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/choice.js
  lib/lp/app/javascript/choiceedit/choiceedit.js
  lib/lp/bugs/javascript/information_type_choice.js
  lib/lp/bugs/javascript/tests/test_information_type_choice.js
  lib/lp/bugs/templates/bug-portlet-privacy.pt

[Steve Kowalik (stevenk)]

This looks excellent. My only concern is on lines 48-49 you call ._showFailed() on the widget and then update the privacy portlet, whereas on lines 108-109, you update the privacy portlet and then call .showSuccessful().

review: Approve (code)

[Ian Booth (wallyworld)]

The show failed comes before the update because we want to flash the displayed 'bad' text.
The show success comes after the update because we want to flash the good text.

=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2012-06-05 10:09:35 +0000
+++ lib/lp/app/javascript/choice.js	2012-06-13 01:32:21 +0000
@@ -75,7 +75,7 @@
     var choice_node = Y.Node.create([
         '<span id="' + field_name + '-content"><span class="value"></span>',
         '<a class="sprite edit editicon actionicon" href="#"></a></span>'
-        ].join(' '));
+        ].join(''));
     if (show_description) {
         choice_node.append(Y.Node.create('<div class="formHelp"></div>'));
     }

=== modified file 'lib/lp/app/javascript/choiceedit/choiceedit.js'
--- lib/lp/app/javascript/choiceedit/choiceedit.js	2012-05-15 16:50:00 +0000
+++ lib/lp/app/javascript/choiceedit/choiceedit.js	2012-06-13 01:32:21 +0000
@@ -145,6 +145,10 @@
       }
     },
 
+    flashEnabled: {
+        value: true
+    },
+
     backgroundColor: {
       value: null
     },
@@ -191,7 +195,9 @@
 
         this.after("valueChange", function(e) {
             this.syncUI();
-            this._showSucceeded();
+            if (this.get('flashEnabled')) {
+                this._showSucceeded();
+            }
         });
     },
 

=== modified file 'lib/lp/bugs/javascript/information_type_choice.js'
--- lib/lp/bugs/javascript/information_type_choice.js	2012-06-07 17:56:00 +0000
+++ lib/lp/bugs/javascript/information_type_choice.js	2012-06-13 01:32:21 +0000
@@ -21,7 +21,8 @@
     error_handler.handleError = function(ioId, response) {
         var orig_value = LP.cache.bug.information_type;
         widget.set('value', orig_value);
-        update_information_type_description(orig_value);
+        widget._showFailed();
+        update_privacy_portlet(orig_value);
         return false;
     };
     var config = {
@@ -29,7 +30,7 @@
             start: Y.bind(widget._uiSetWaiting),
             end: Y.bind(widget._uiClearWaiting),
             success: function(id, response) {
-                namespace.information_type_save_success(value);
+                namespace.information_type_save_success(widget, value);
             },
             failure: error_handler.getFailureHandler()
         },
@@ -41,12 +42,34 @@
         LP.cache.bug.self_link, 'transitionToInformationType', config);
 };
 
-var update_information_type_description = function(value) {
+var update_privacy_portlet = function(value) {
     var description = information_type_descriptions[value];
     var desc_node = Y.one('#information-type-description');
     if (Y.Lang.isValue(desc_node)) {
         desc_node.set('text', description);
     }
+    var summary = Y.one('#information-type-summary');
+    var private_type = (Y.Array.indexOf(LP.cache.private_types, value) >= 0);
+    if (private_type) {
+        summary.replaceClass('public', 'private');
+    } else {
+        summary.replaceClass('private', 'public');
+    }
+};
+
+var update_privacy_banner = function(value) {
+    var body = Y.one('body');
+    var privacy_banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+    var private_type = (Y.Array.indexOf(LP.cache.private_types, value) >= 0);
+    if (private_type) {
+        body.replaceClass('public', 'private');
+        var banner_text = namespace.get_information_type_banner_text(value);
+        privacy_banner.updateText(banner_text);
+        privacy_banner.show();
+    } else {
+        body.replaceClass('private', 'public');
+        privacy_banner.hide();
+    }
 };
 
 namespace.get_information_type_banner_text = function(value) {
@@ -63,24 +86,12 @@
     }
 };
 
-namespace.information_type_save_success = function(value) {
-    var body = Y.one('body');
-    var summary = Y.one('#information-type-summary');
-    var private_type = (Y.Array.indexOf(LP.cache.private_types, value) >= 0);
+namespace.information_type_save_success = function(widget, value) {
+    LP.cache.bug.information_type = value;
+    update_privacy_banner(value);
+    widget._showSucceeded();
     var subscription_ns = Y.lp.bugs.bugtask_index.portlets.subscription;
-    var privacy_banner = Y.lp.app.banner.privacy.getPrivacyBanner();
     subscription_ns.update_subscription_status(skip_animation);
-    if (private_type) {
-        var banner_text = namespace.get_information_type_banner_text(value);
-        privacy_banner.updateText(banner_text);
-        summary.replaceClass('public', 'private');
-        body.replaceClass('public', 'private');
-        privacy_banner.show();
-    } else {
-        summary.replaceClass('private', 'public');
-        body.replaceClass('private', 'public');
-        privacy_banner.hide();
-    }
 };
 
 namespace.setup_information_type_choice = function(privacy_link, lp_client,
@@ -97,7 +108,8 @@
         value: LP.cache.bug.information_type,
         title: "Change information type",
         items: LP.cache.information_types,
-        backgroundColor: '#FFFF99'
+        backgroundColor: '#FFFF99',
+        flashEnabled: false
     });
     Y.lp.app.choice.hook_up_choicesource_spinner(information_type_edit);
     information_type_edit.render();
@@ -108,7 +120,7 @@
         if (value === 'Private') {
             value = 'User Data';
         }
-        update_information_type_description(value);
+        update_privacy_portlet(value);
         namespace.save_information_type(
             information_type_edit, value, lp_client);
     });

=== modified file 'lib/lp/bugs/javascript/tests/test_information_type_choice.js'
--- lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-06-07 17:56:00 +0000
+++ lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-06-13 01:32:21 +0000
@@ -23,7 +23,7 @@
                         self_link: '/bug/1'
                     },
                     show_information_type_in_ui: true,
-                    private_types: ['Private'],
+                    private_types: ['Private', 'User Data'],
                     information_types: [
                         {'value': 'Public', 'name': 'Public',
                             'description': 'Public Description'},
@@ -97,11 +97,12 @@
                 update_flag = true;
             });
 
-            ns.information_type_save_success('Private');
+            ns.information_type_save_success(this.widget, 'Private');
             var body = Y.one('body');
             Y.Assert.isTrue(body.hasClass('private'));
             Y.Assert.isTrue(hide_flag);
             Y.Assert.isTrue(update_flag);
+            Y.Assert.areEqual('Private', LP.cache.bug.information_type);
             this._unshim_privacy_banner(old_func);
         },
 
@@ -114,11 +115,11 @@
             var summary = Y.one('#information-type-summary');
             summary.replaceClass('public', 'private');
 
-            ns.information_type_save_success('Public');
-            Y.Assert.isTrue(summary.hasClass('public'));
+            ns.information_type_save_success(this.widget, 'Public');
             var body = Y.one('body');
             Y.Assert.isTrue(body.hasClass('public'));
             Y.Assert.isTrue(flag);
+            Y.Assert.areEqual('Public', LP.cache.bug.information_type);
             this._unshim_privacy_banner(old_func);
         },
 
@@ -136,6 +137,8 @@
             var description_node = Y.one('#information-type-description');
             Y.Assert.areEqual(
                 'User Data Description', description_node.get('text'));
+            var summary = Y.one('#information-type-summary');
+            Y.Assert.isTrue(summary.hasClass('private'));
             Y.Assert.isTrue(function_called);
             ns.save_information_type = orig_save_information_type;
         },
@@ -157,6 +160,8 @@
             var description_node = Y.one('#information-type-description');
             Y.Assert.areEqual(
                 'Public Description', description_node.get('text'));
+            var summary = Y.one('#information-type-summary');
+            Y.Assert.isTrue(summary.hasClass('public'));
             // The error was displayed.
             Y.Assert.isNotNull(Y.one('.yui3-lazr-formoverlay-errors'));
         }

=== modified file 'lib/lp/bugs/templates/bug-portlet-privacy.pt'
--- lib/lp/bugs/templates/bug-portlet-privacy.pt	2012-06-07 18:27:33 +0000
+++ lib/lp/bugs/templates/bug-portlet-privacy.pt	2012-06-13 01:32:21 +0000
@@ -15,8 +15,7 @@
          contains
          <strong id="information-type" tal:content="view/information_type" />
          information
-       </span>&nbsp;<a
-           class="sprite edit" id="privacy-link"
+       </span>&nbsp;<a class="sprite edit" id="privacy-link"
            tal:attributes="href link/path" tal:condition="link/enabled"></a>
       <div id="information-type-description" style="padding-top: 5px"
                 tal:content="view/information_type_description"></div>