launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/recipe-picker-spinners into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #719785 Recipe owner and archive pickers need spinners
  https://bugs.launchpad.net/bugs/719785

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/recipe-picker-spinners/+merge/51253

Simple fix to add spinners to the popup vocab picker widget.

== Implementation ==

The picker recently had an option added to eagerly load vocabs which were not huge. In such cases, the search button would not be shown. However, the picker could take a little while to load the vocab. In the case of searchable vocabs, the spinner goes on the search button. But in the eager load case with no search button, a new spinner was required.

The spinner appears just above where the results would be displayed. The first time the picker is used, it is there for a short time. Subsequent invocations, it only briefly flashes but is required nonetheless since a back end call is made to reload the vocab each time.

== Tests ==

None. There's no existing tests for spinners AFAIK.

== Lint ==

Linting changed files:
  lib/lp/app/javascript/picker.js

./lib/lp/app/javascript/picker.js
     113: Line exceeds 78 characters.
     124: Line exceeds 78 characters.
     222: Line exceeds 78 characters.

-- 
https://code.launchpad.net/~wallyworld/launchpad/recipe-picker-spinners/+merge/51253
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/recipe-picker-spinners into lp:launchpad.

=== modified file 'lib/lp/app/javascript/picker.js'
--- lib/lp/app/javascript/picker.js	2011-02-23 22:32:15 +0000
+++ lib/lp/app/javascript/picker.js	2011-02-25 06:13:06 +0000
@@ -211,8 +211,12 @@
     }
     picker.set('footer_slot', extra_buttons);
 
+    // The spinner displays a "Loading..." message while vocab loads.
+    config.temp_spinner = create_temporary_spinner(picker);
+
     activator.subscribe('act', function (e) {
         if (!show_search_box) {
+          config.temp_spinner.removeClass('unseen');
           picker.set('min_search_chars', 0);
           picker.fire('search', '');
           picker.get('contentBox').one('.yui3-picker-search-box').addClass('unseen');
@@ -226,6 +230,27 @@
     return picker;
 };
 
+/*
+ * Show the Loading.... spinner (used when we preload the entire vocab).
+ */
+function create_temporary_spinner(picker) {
+    var node = picker.get('contentBox').one('.yui3-picker-batches');
+    var temp_spinner = Y.Node.create([
+    '<div class="unseen" align="center">',
+    '<img src="/@@/spinner"/>Loading...',
+    '</div>'].join(''));
+    node.insert(temp_spinner, node);
+    return temp_spinner;
+}
+
+/*
+ * Remove the Loading.... spinner (if it exists).
+ */
+function hide_temporary_spinner(temp_spinner) {
+    if( temp_spinner != null )
+        temp_spinner.addClass('unseen');
+}
+
 /**
   * Creates a picker widget that has already been rendered and hidden.
   *
@@ -302,6 +327,7 @@
             var start = entry.start;
             var results = entry.entries;
 
+            hide_temporary_spinner(config.temp_spinner);
             if (total_size > (MAX_BATCHES * BATCH_SIZE))  {
                 picker.set('error',
                     'Too many matches. Please try to narrow your search.');
@@ -347,6 +373,7 @@
             on: {
                 success: success_handler,
                 failure: function (arg) {
+                    hide_temporary_spinner(config.temp_spinner);
                     picker.set('error', 'Loading results failed.');
                     picker.set('search_mode', false);
                     Y.log("Loading " + uri + " failed.");