Merge lp:~abentley/launchpad/sharing-spinners into lp:launchpad

[Aaron Bentley]

Add spinners to translation-sharing actions.

= Summary =
Fix bug #758922: Missing spinners on translations +sharing-details js actions

== Proposed fix ==
Add hidden spinners to web page, show them as needed.

== Pre-implementation notes ==
None

== Implementation details ==
Added a new "pending" boolean to checklist items. When this is true, the TranslationSharingController will show the spinner and hide the picker. When this is false, the TranslationSharingController will hide the spinner and may show the picker.

== Tests ==
firefox lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html

== Demo and Q/A ==
Go to the +sharing-details page for a SourcePackage. Ensure that when each setting is changed, a spinner is shown. This should happen both when the list item was formlerly uncheckd and when it was formerly checked.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/templates/sourcepackage-sharing-details.pt
  lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js
  lib/lp/translations/javascript/sourcepackage_sharing_details.js
  lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'lib/lp/translations/javascript/sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/sourcepackage_sharing_details.js	2011-04-18 13:27:33 +0000
+++ lib/lp/translations/javascript/sourcepackage_sharing_details.js	2011-04-18 13:41:39 +0000
@@ -26,7 +26,8 @@
         }
     },
     // The HTML identifier of the item.
-    identifier: null
+    identifier: null,
+    pending: {value: false}
 };
 Y.extend(CheckItem, Y.Base, {
 });
@@ -224,9 +225,8 @@
 }
 
 
-function IOHandler(flash_target, error_handler) {
+function IOHandler(controller, check, error_handler) {
     that = this;
-    this.flash_target = flash_target;
     if (!Y.Lang.isValue(error_handler)){
         this.error_handler = new Y.lp.client.ErrorHandler();
     }
@@ -234,7 +234,10 @@
         this.error_handler = error_handler;
     }
     this.error_handler.showError = function(error_msg) {
-        Y.lp.app.errors.display_error(Y.one(that.flash_target), error_msg);
+        check.set('pending', false);
+        controller.update_check(check);
+        var flash_target = Y.one(controller.visible_check_selector(check));
+        Y.lp.app.errors.display_error(Y.one(flash_target), error_msg);
     };
 }
 
@@ -373,6 +376,8 @@
         var productseries_check = that.get('tsconfig').get('product_series');
         var lp_client = new Y.lp.client.Launchpad();
         function save_productseries(config) {
+            productseries_check.set('pending', true);
+            that.update_check(productseries_check);
             var source_package = that.get('source_package');
             config.parameters = {
                 productseries: productseries_summary.api_uri};
@@ -398,11 +403,11 @@
         }
         function set_usage(product) {
             that.replace_product(product);
+            productseries_check.set('pending', false);
             that.update();
             that.flash_check_green(productseries_check);
         }
-        var css_selector = this.visible_check_selector(productseries_check);
-        var io_handler = new IOHandler(css_selector);
+        var io_handler = new IOHandler(this, productseries_check);
         save_productseries(io_handler.chain_config(
             get_productseries, cache_productseries, cache_branch, set_usage));
     },
@@ -421,6 +426,8 @@
          * closure, such as "that" and "branch_summary".
          */
         function save_branch(config) {
+            branch_check.set('pending', true);
+            that.update_check(branch_check);
             productseries.set('branch_link', branch_summary.api_uri);
             productseries.lp_save(config);
         }
@@ -433,11 +440,11 @@
         }
         function finish(new_productseries){
             that.replace_productseries(new_productseries);
+            branch_check.set('pending', false);
             that.update();
             that.flash_check_green(branch_check);
         }
-        css_selector = that.visible_check_selector(branch_check);
-        var io_handler = new IOHandler(css_selector);
+        var io_handler = new IOHandler(this, branch_check);
         save_branch(io_handler.chain_config(get_branch, set_link, finish));
     },
     /**
@@ -475,6 +482,7 @@
      */
     update_check: function(check){
         var complete = Y.one(this.check_selector(check, true));
+        var hide_picker = !check.get('enabled') || check.get('pending');
         var link = complete.one('.link a');
         if (link !== null){
             link.set('href', check.get('url'));
@@ -484,14 +492,19 @@
         complete.toggleClass('lowlight', !check.get('enabled'));
         var complete_picker = Y.one(this.picker_selector(check, true));
         if (complete_picker !== null) {
-            complete_picker.toggleClass('unseen', !check.get('enabled'));
+            complete_picker.toggleClass('unseen', hide_picker);
         }
         var incomplete = Y.one(this.check_selector(check, false));
         incomplete.toggleClass('unseen', check.get('complete'));
         incomplete.toggleClass('lowlight', !check.get('enabled'));
         var incomplete_picker = Y.one(this.picker_selector(check, false));
         if (incomplete_picker !== null) {
-            incomplete_picker.toggleClass('unseen', !check.get('enabled'));
+            incomplete_picker.toggleClass('unseen', hide_picker);
+        }
+        var selector = this.visible_check_selector(check) + '-spinner';
+        var spinner = Y.one(selector);
+        if (Y.Lang.isValue(spinner)){
+            spinner.toggleClass('unseen', !check.get('pending'));
         }
     },
     flash_check_green: function(check) {
@@ -542,14 +555,15 @@
         product_series.set('translations_autoimport_mode', mode);
         var autoimport_check = sharing_controller.get(
             'tsconfig').get('autoimport');
-        var css_selector = sharing_controller.visible_check_selector(
-            autoimport_check);
-        handler = new IOHandler(css_selector);
+        handler = new IOHandler(sharing_controller, autoimport_check);
         function update_controller() {
             sharing_controller.set_autoimport_mode(mode);
+            autoimport_check.set('pending', false);
             sharing_controller.update();
             sharing_controller.flash_check_green(autoimport_check);
         }
+        autoimport_check.set('pending', true);
+        sharing_controller.update_check(autoimport_check);
         /* XXX: AaronBentley 2011-04-04 bug=369293: Avoid 412 on repeated
          * changes.  This does not increase the risk of changing from a
          * stale value, because the staleness check is not reasonable.
@@ -581,12 +595,14 @@
         }
         function replace_product(new_product) {
             sharing_controller.replace_product(new_product);
+            usage.set('pending', false);
             sharing_controller.update();
             sharing_controller.flash_check_green(usage);
         }
-        css_selector = sharing_controller.visible_check_selector(usage);
         var io_handler = new IOHandler(
-            css_selector, new Y.lp.client.FormErrorHandler());
+            sharing_controller, usage, new Y.lp.client.FormErrorHandler());
+        usage.set('pending', true);
+        sharing_controller.update_check(usage);
         var config = io_handler.chain_config(
             get_productseries, replace_productseries, get_product,
             replace_product);

=== modified file 'lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html'
--- lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html	2011-04-04 19:36:27 +0000
+++ lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html	2011-04-18 13:41:39 +0000
@@ -46,8 +46,13 @@
       <div id="branch-complete">Branch selected: <span class="link"><a
           href="#" class="link"></a></span>
           <span id="branch-complete-picker"><a href="#"></a></span>
-      </div>
-      <div id="branch-incomplete">No branch selected.</div>
+          <img src="../../../../canonical/launchpad/images/spinner.gif"
+               id="branch-complete-spinner"/>
+      </div>
+      <div id="branch-incomplete">No branch selected.
+          <img src="../../../../canonical/launchpad/images/spinner.gif"
+               id="branch-incomplete-spinner"/>
+      </div>
       <span id="branch-incomplete-picker"><a href="#"></a></span>
       <div id="translation-incomplete">
         Translations are not enabled on the upstream project.
@@ -58,12 +63,16 @@
         <a href="#enable-translations"></a>
       </div>
       <div  id="upstream-sync-incomplete">
-        Automatic synchronization of translations is not enabled.
-        <a href="#enable-sync"></a>
+          Automatic synchronization of translations is not enabled.
+        <img src="../../../../canonical/launchpad/images/spinner.gif"
+             id="upstream-sync-incomplete-spinner"/>
+        <span id="upstream-sync-incomplete-picker"><a href="#enable-sync"></a></span>
       </div>
       <div  id="upstream-sync-complete">
         Automatic synchronization of translations is enabled.
-        <a href="#enable-sync"></a>
+        <span id="upstream-sync-complete-picker"><a href="#enable-sync"></a></span>
+        <img src="../../../../canonical/launchpad/images/spinner.gif"
+             id="upstream-sync-complete-spinner"/>
       </div>
     </div>
     <!-- The test output -->

=== modified file 'lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js	2011-04-14 15:43:14 +0000
+++ lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js	2011-04-18 13:41:39 +0000
@@ -210,6 +210,39 @@
             ctrl.update_check(branch);
             Y.Assert.isFalse(incomplete.hasClass('lowlight'));
         },
+        test_update_check_pending: function(){
+            var incomplete_spinner = Y.one(
+                '#upstream-sync-incomplete-spinner');
+            var ctrl = new TranslationSharingController();
+            var autoimport = ctrl.get('tsconfig').get('autoimport');
+            var branch = ctrl.get('tsconfig').get('branch');
+            branch.set_link('a', 'b');
+            var incomplete_picker = Y.one(
+                ctrl.picker_selector(autoimport, false));
+            ctrl.update_check(autoimport);
+            Y.Assert.isTrue(
+                incomplete_spinner.hasClass('unseen'), 'spinner unseen');
+            Y.Assert.isFalse(
+                incomplete_picker.hasClass('unseen'), 'picker seen');
+            autoimport.set('pending', true);
+            ctrl.update_check(autoimport);
+            Y.Assert.isFalse(
+                incomplete_spinner.hasClass('unseen'), 'spinner seen');
+            Y.Assert.isTrue(
+                incomplete_picker.hasClass('unseen'), 'picker unseen');
+            autoimport.set('complete', true);
+            var complete_spinner = Y.one(
+                '#upstream-sync-complete-spinner');
+            var complete_picker = Y.one(
+                ctrl.picker_selector(autoimport, true));
+            ctrl.update_check(autoimport);
+            Y.Assert.isFalse(complete_spinner.hasClass('unseen'));
+            Y.Assert.isTrue(complete_picker.hasClass('unseen'));
+            autoimport.set('pending', false);
+            ctrl.update_check(autoimport);
+            Y.Assert.isTrue(complete_spinner.hasClass('unseen'));
+            Y.Assert.isFalse(complete_picker.hasClass('unseen'));
+        },
         test_set_autoimport_mode: function() {
             var ctrl = new TranslationSharingController();
             var check = ctrl.get('tsconfig').get('autoimport');

=== modified file 'lib/lp/translations/templates/sourcepackage-sharing-details.pt'
--- lib/lp/translations/templates/sourcepackage-sharing-details.pt	2011-04-11 17:16:28 +0000
+++ lib/lp/translations/templates/sourcepackage-sharing-details.pt	2011-04-18 13:41:39 +0000
@@ -39,6 +39,8 @@
                 <span id="packaging-incomplete-picker">
                   <a tal:replace="structure view/set_packaging_link/escapedtext" />
                 </span>
+                <img src="/@@/spinner" class="unseen"
+                     id="packaging-incomplete-spinner"/>
             </li>
             <li tal:attributes="class view/packaging_complete_class"
                 id="packaging-complete">
@@ -51,6 +53,8 @@
               <span id="packaging-complete-picker">
                 <a tal:replace="structure view/change_packaging_link/escapedtext" />
               </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="packaging-complete-spinner"/>
               <a tal:replace="structure view/remove_packaging_link/escapedtext" />
             </li>
             <li tal:attributes="class view/branch_incomplete_class"
@@ -59,6 +63,8 @@
               <span id="branch-incomplete-picker">
                 <a tal:replace="structure view/new_branch_link/escapedtext" />
               </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="branch-incomplete-spinner"/>
             </li>
             <li tal:attributes="class view/branch_complete_class"
                 id="branch-complete">
@@ -69,6 +75,8 @@
               <span id="branch-complete-picker">
                 <a tal:replace="structure view/change_branch_link/escapedtext" />
               </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="branch-complete-spinner"/>
             </li>
             <li tal:attributes="class view/translations_disabled_class"
                 id="translation-incomplete">
@@ -76,6 +84,8 @@
               <span id="translation-incomplete-picker">
                   <a tal:replace="structure view/configure_translations_link_unconfigured/escapedtext" />
               </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="translation-incomplete-spinner"/>
             </li>
             <li tal:attributes="class view/translations_enabled_class"
                 id="translation-complete">
@@ -83,6 +93,8 @@
               <span id="translation-complete-picker">
               <a tal:replace="structure view/configure_translations_link_configured/escapedtext" />
              </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="translation-complete-spinner"/>
             </li>
             <li tal:attributes="class view/upstream_sync_disabled_class"
                 id="upstream-sync-incomplete">
@@ -90,6 +102,8 @@
               <span id="upstream-sync-incomplete-picker">
               <a tal:replace="structure view/translation_sync_link_unconfigured/escapedtext" />
               </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="upstream-sync-incomplete-spinner"/>
             </li>
             <li tal:attributes="class view/upstream_sync_enabled_class"
                 id="upstream-sync-complete">
@@ -97,6 +111,8 @@
               <span id="upstream-sync-complete-picker">
               <a tal:replace="structure view/translation_sync_link_configured/escapedtext" />
               </span>
+              <img src="/@@/spinner" class="unseen"
+                   id="upstream-sync-complete-spinner"/>
             </li>
           </ul>
         </dd>