← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~deryck/launchpad/buglists-loading-885272-final into lp:launchpad

 

Deryck Hodge has proposed merging lp:~deryck/launchpad/buglists-loading-885272-final into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~deryck/launchpad/buglists-loading-885272-final/+merge/84183

This branch creates a nicer loading/spinner interaction for the new CustomBugListings.  The spinner overlays bug lists now and jumps to the top of the results if you're not already positioned there.

See it here:
http://uploads.mitechie.com/lp/lp_spinner_indicator.png

This is combined work from both Rick Harding and I as we're hacking at an induction sprint together.  The bulk of the change here is to add an OverlayIndicator widget that takes a "target" node in its config, which is the node our indicator will cover.  There is full test coverage for this widget.  We also integrate the widget into buglisting.js, which manages list navigation in the new feature.  ListingNavigator originally maintained pending state and clearing spinners, but all of that is handled by the indicator now.  This allows us to scroll to the correct position based on the covered node, without duplicating that code in ListingNavigator.

Also, we had to fix up tests for ListingNavigator, since a target is now required to be passed in.

We created OverlayIndicator as a new module in lp.app.javascript.indicators so that we can move all such future loading indicators here.  For example, the activator code that virtually no one uses for inline spinners could be moved here and cleaned up to follow a similar pattern.

OverlayIndicator takes a config that supports optional success and error actions.  These are callbacks that will be executed when success and error are called.  If nothing is provided, we simply hide the overlay.  Call sites can create individual actions, or this module can provide common actions.  The only success action we needed was the scroll to action -- i.e. we scroll to the target location on success -- but we wrote the action code is such a way that additional actions, like green flash, etc. could be easily added as needed.

Lint free.
-- 
https://code.launchpad.net/~deryck/launchpad/buglists-loading-885272-final/+merge/84183
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/buglists-loading-885272-final into lp:launchpad.
=== modified file 'buildout-templates/bin/combine-css.in'
--- buildout-templates/bin/combine-css.in	2011-11-22 02:46:23 +0000
+++ buildout-templates/bin/combine-css.in	2011-12-02 14:58:28 +0000
@@ -34,6 +34,7 @@
     'build/autocomplete/assets/skins/sam/autocomplete.css',
     'build/overlay/assets/skins/sam/pretty-overlay.css',
     'build/formoverlay/assets/formoverlay-core.css',
+    'build/indicator/assets/indicator-core.css',
     'build/confirmationoverlay/assets/confirmationoverlay-core.css',
     'build/picker/assets/skins/sam/picker.css',
     'build/activator/assets/skins/sam/activator.css',

=== added file 'lib/canonical/launchpad/images/spinner-big.gif'
Binary files lib/canonical/launchpad/images/spinner-big.gif	1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/spinner-big.gif	2011-12-02 14:58:28 +0000 differ
=== added directory 'lib/lp/app/javascript/indicator'
=== added directory 'lib/lp/app/javascript/indicator/assets'
=== added file 'lib/lp/app/javascript/indicator/assets/indicator-core.css'
--- lib/lp/app/javascript/indicator/assets/indicator-core.css	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/indicator/assets/indicator-core.css	2011-12-02 14:58:28 +0000
@@ -0,0 +1,18 @@
+.yui3-overlay-indicator {
+    background-color: rgb(255,255,255);
+    /* for css3 browsers we can limit opacity via alpha channel */
+    background-color: rgba(255,255,255,0.6);
+    position: absolute;
+    z-index: 999;
+    }
+
+.yui3-overlay-indicator img {
+    display: block;
+    margin: auto;
+    max-width: 100%;
+    position: absolute;
+    }
+
+.yui3-overlay-indicator-hidden {
+    visibility: hidden;
+    }

=== added directory 'lib/lp/app/javascript/indicator/assets/skins'
=== added directory 'lib/lp/app/javascript/indicator/assets/skins/sam'
=== added file 'lib/lp/app/javascript/indicator/assets/skins/sam/indicator-skin.css'
--- lib/lp/app/javascript/indicator/assets/skins/sam/indicator-skin.css	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/indicator/assets/skins/sam/indicator-skin.css	2011-12-02 14:58:28 +0000
@@ -0,0 +1,3 @@
+/* Copyright (c) 2010, Canonical Ltd. All rights reserved. */
+
+/* Placeholder for skinning of the OverlayIndicator. */

=== added file 'lib/lp/app/javascript/indicator/indicator.js'
--- lib/lp/app/javascript/indicator/indicator.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/indicator/indicator.js	2011-12-02 14:58:28 +0000
@@ -0,0 +1,223 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Client-side rendering of bug listings.
+ *
+ * @module lp.indicator
+ *
+ * Usage:
+ *     adds a .indicator on the target node for api controls
+ *     Y.one('#id').plug(lp.indicator.IndicatorPlugin)
+ *
+ *     or directly without the plugin, but then you have to control the
+ *     disabled flags on it yourself for show/hide/etc
+ *     lp.indicator.IndicatorWidget({
+ *         target: Y.one('#id')
+ *     });
+ *
+ */
+YUI().add('lp.indicator', function (Y) {
+
+    // Our spinner is 32px, we use this for calculating the center position of
+    // the spinner within the overlay div
+    var SPINNER_SIZE = 32;
+
+    var props = {
+        ATTRS: {
+            /**
+             * A reference to the node that we're going to overlay.
+             *
+             * @attribute target
+             * @type Y.Node
+             * @default null
+             */
+            target: {
+                value: null
+            },
+
+            /**
+             * Callback to fire upon calling success.
+             *
+             * @attribute success_action
+             * @type Function
+             * @default null
+             */
+            success_action: {
+                value: null
+            },
+
+            /**
+             * Callback to fire upon calling error.
+             *
+             * @attribute error_action
+             * @type Function
+             * @default null
+             */
+            error_action: {
+                value: null
+            }
+        }
+    };
+
+    var config = {
+
+        /**
+         * Calc the padding we need to use to center the spinner
+         */
+        _calc_image_padding: function (dim) {
+            var boundingBox = this.get('boundingBox');
+            var content_height =
+                boundingBox.getStyle(dim).replace('px', '');
+            return content_height > SPINNER_SIZE ?
+                (content_height - SPINNER_SIZE) / 2.0 : 0;
+        },
+
+        /**
+         * YUI init
+         *
+         * @method initializer
+         */
+        initializer: function(cfg) {
+            this.hide();
+        },
+
+        _addListeners: function() {
+            this.on('visibleChange', function(e) {
+                if (e.newVal === true) {
+                    this.resizeAndReposition();
+                }
+            }, this);
+        },
+
+        /**
+         * To prevent having to force call sites to pass in
+         * parentNode, we must overridge YUI's built-in _renderUI
+         * method.
+         *
+         * This is a copy of the YUI method, except for using our
+         * own parentNode.  This is needed so the spinner overlays
+         * correctly.
+         *
+         * @method _renderUI
+         */
+         _renderUI: function() {
+             var local_parent = this.get('target').get('parentNode');
+             this._renderBoxClassNames();
+             this._renderBox(local_parent);
+         },
+
+        /**
+         * Build the indicator overlay itself
+         *
+         * @method renderUI
+         */
+        renderUI: function () {
+            var node_html = '<img/>';
+            var img = Y.Node.create(node_html);
+            img.set('src', '/@@/spinner-big');
+            this.get('contentBox').append(img);
+        },
+
+        bindUI: function() {
+            this._addListeners();
+        },
+
+        // Whenever the target changes it's position, we resize catching the
+        // event from the Y.Widget.Position extension.
+        resizeAndReposition: function () {
+            var boundingBox = this.get('boundingBox');
+            var target = this.get('target');
+            var width = target.get('offsetWidth');
+            var height = target.get('offsetHeight');
+            boundingBox.set('offsetWidth', width);
+            boundingBox.set('offsetHeight', height);
+            // Now do position too.
+            boundingBox.setXY(target.getXY());
+            // update the padding on the image to fit
+            var img = boundingBox.one('img');
+            img.setStyle('top', this._calc_image_padding('height') + 'px');
+            img.setStyle('left', this._calc_image_padding('width') + 'px');
+        },
+
+        /**
+         * Mark the loading or busy action as in progress,
+         * and show the overlay.
+         *
+         * @method setBusy
+         */
+        setBusy: function() {
+            this.show();
+        },
+
+        success: function() {
+            this.hide();
+            var callback = this.get('success_action');
+            if (Y.Lang.isFunction(callback)) {
+                callback.call(this);
+            }
+        },
+
+        error: function() {
+            this.hide();
+            var callback = this.get('error_action');
+            if (Y.Lang.isFunction(callback)) {
+                callback.call(this);
+            }
+        }
+    };
+
+
+    /**
+     * This is using the YUI concept of class extensions to get us min-in like
+     * features from the Widget classes. This keeps us from having to do a
+     * bunch of position work ourselves.
+     */
+    var OverlayIndicator = Y.Base.create(
+            'overlay-indicator',
+            Y.Widget,
+            [],
+            config,
+            props
+    );
+
+    /**
+     * Actions are pre-built responses that can be used as success and
+     * error actions
+     *
+     * All actions are called within the scope of the widget created and
+     * assigned to.
+     *
+     */
+    var actions = {
+        /**
+         * Scroll to the top left point of the target node we're working off
+         * of.
+         *
+         * We're only going to change focus if the target's XY position
+         * isn't in the current viewport.
+         */
+        scroll_to_target: function () {
+           var target = this.get('target').getXY();
+           var viewport = Y.DOM.viewportRegion();
+           // Do we want to do the scroll move?
+           var scroll = false;
+           // If our target X is too far left or right for us to see, scroll
+           // our viewport.
+           if (target[0] < viewport.left || target[0] > viewport.right) {
+               scroll = true;
+           }
+           // Verify if our Y is out of viewport scope.
+           if (target[1] < viewport.top || target[1] > viewport.bottom) {
+               scroll = true;
+           }
+           if (scroll) {
+               window.scrollTo(target[0], target[1]);
+           }
+        }
+    };
+
+    var indicator = Y.namespace('lp.indicator');
+    indicator.OverlayIndicator = OverlayIndicator;
+    indicator.actions = actions;
+
+}, '0.1', {requires: ['base', 'widget']});

=== added directory 'lib/lp/app/javascript/indicator/tests'
=== added file 'lib/lp/app/javascript/indicator/tests/test_indicator.html'
--- lib/lp/app/javascript/indicator/tests/test_indicator.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/indicator/tests/test_indicator.html	2011-12-02 14:58:28 +0000
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+  "http://www.w3.org/TR/html4/strict.dtd";>
+<html>
+  <head>
+  <title>Indicator Tests</title>
+
+  <!-- YUI and test setup -->
+  <script type="text/javascript"
+          src="../../../../../canonical/launchpad/icing/yui/yui/yui.js">
+  </script>
+  <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
+  <link rel="stylesheet" href="../assets/indicator-core.css" />
+  <script type="text/javascript"
+          src="../../../../app/javascript/testing/testrunner.js"></script>
+
+  <!-- The module under test -->
+  <script type="text/javascript" src="../indicator.js"></script>
+
+  <!-- The test suite -->
+  <script type="text/javascript" src="test_indicator.js"></script>
+
+</head>
+<body class="yui3-skin-sam">
+    <ul id="suites">
+        <li>lp.large_indicator.test</li>
+    </ul>
+</body>
+</html>

=== added file 'lib/lp/app/javascript/indicator/tests/test_indicator.js'
--- lib/lp/app/javascript/indicator/tests/test_indicator.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/indicator/tests/test_indicator.js	2011-12-02 14:58:28 +0000
@@ -0,0 +1,220 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.large_indicator.test', function (Y) {
+
+var large_indicator = Y.namespace('lp.large_indicator.test');
+var suite = new Y.Test.Suite('Indicator Tests');
+var Assert = Y.Assert;
+
+suite.add(new Y.Test.Case({
+
+    name: 'indicator_tests',
+
+    setUp: function () {
+        this.div = Y.Node.create('<div></div>');
+        // generate an id so we can keep these around as we work
+        this.div.generateID();
+        // we want to store this for the testing of the target, after that we
+        // can just use this.div
+        this.div_id = this.div.get('id');
+        Y.one('body').appendChild(this.div);
+    },
+
+    tearDown: function () {
+        // delete the reference to this.div so we can recreate new ones for
+        // each test without worry
+        this.div.remove();
+        delete this.div;
+        if (Y.Lang.isValue(this.indicator)) {
+            this.indicator.destroy();
+        }
+    },
+
+    test_target_attribute: function () {
+        // constrain attribute should be set from passing in target.
+        var test_node = Y.one('#' + this.div_id);
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: test_node
+        });
+        this.indicator.render();
+        Assert.areEqual(test_node, this.indicator.get('target'));
+    },
+
+    test_indicator_appended_to_parent: function() {
+        // indicator node is appended to target's parent, rather
+        // than target or body.
+        var child_div = Y.Node.create('<div></div>');
+        // We need to create some nesting to really ensure
+        // the test is good.
+        this.div.appendChild(child_div);
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: child_div
+        });
+        this.indicator.render();
+        // this.div is actually the parentNode now.
+        Assert.areEqual(
+            this.div,
+            this.indicator.get('boundingBox').get('parentNode'));
+    },
+
+    test_indicator_has_loading_icon: function () {
+        // The indicator should have a loading image added
+        // to the contentBox.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        var content = this.indicator.get('boundingBox');
+        var test = content.getContent();
+        var img = content.one('img');
+        Assert.areEqual('file:///@@/spinner-big', img.get('src'));
+    },
+
+    test_indiciator_starts_invisible: function () {
+        // indicator widgets should start hidden.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        Assert.isFalse(this.indicator.get('visible'), 'visible is not set');
+        Assert.isTrue(this.indicator.get('boundingBox').hasClass(
+            'yui3-overlay-indicator-hidden'), 'class does not exist');
+    },
+
+    test_set_busy_shows_overlay: function() {
+        // setBusy should show the overlay.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        this.indicator.setBusy();
+        Assert.isTrue(this.indicator.get('visible'), 'visible is not set');
+        Assert.isFalse(this.indicator.get('boundingBox').hasClass(
+            'yui3-overlay-indicator-hidden'), 'class does not exist');
+    },
+
+    test_size_matches_on_set_busy: function() {
+        // indicator should always resize when target changes size.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        // Mess with the size of target div.
+        var expected_width = 800;
+        var expected_height = 600;
+        this.div.set('offsetWidth', expected_width);
+        this.div.set('offsetHeight', expected_height);
+        Assert.areNotEqual(
+            expected_width,
+            this.indicator.get('boundingBox').get('offsetWidth'));
+        Assert.areNotEqual(
+            expected_height,
+            this.indicator.get('boundingBox').get('offsetHeight'));
+        this.indicator.setBusy();
+        Assert.areEqual(
+            expected_width,
+            this.indicator.get('boundingBox').get('offsetWidth'));
+        Assert.areEqual(
+            expected_height,
+            this.indicator.get('boundingBox').get('offsetHeight'));
+    },
+
+    test_position_matches_on_set_busy: function() {
+        // indicator should always reposition itself before setBusy.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        // Mess with the position of target div.
+        var expected_xy = [100, 300];
+        this.div.setXY(expected_xy);
+        var actual_xy = this.indicator.get('boundingBox').getXY();
+        Assert.areNotEqual(expected_xy[0], actual_xy[0]);
+        Assert.areNotEqual(expected_xy[1], actual_xy[1]);
+        this.indicator.setBusy();
+        var final_xy = this.indicator.get('boundingBox').getXY();
+        Assert.areEqual(expected_xy[0], final_xy[0]);
+        Assert.areEqual(expected_xy[1], final_xy[1]);
+
+    },
+
+    test_success_hides_overlay: function() {
+        // Calling success should hide the overlay.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        this.indicator.setBusy();
+        this.indicator.success();
+        Assert.isFalse(this.indicator.get('visible'), 'visible is not set');
+        Assert.isTrue(this.indicator.get('boundingBox').hasClass(
+            'yui3-overlay-indicator-hidden'), 'class does not exist');
+    },
+
+    test_success_callback: function() {
+        // We should be able to pass in a callback as success_action.
+        var called = false;
+        var callback = function() {
+            called = true;
+        };
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div,
+            success_action: callback
+        });
+        this.indicator.render();
+        this.indicator.success();
+        Assert.isTrue(called);
+    },
+
+    test_focus_target_scrolls_success: function () {
+        // Provided function scroll_to_target should scroll to target.
+        var viewport = Y.DOM.viewportRegion();
+        this.div.set('offsetWidth', viewport.right + 1000);
+        this.div.set('offsetHeight', viewport.bottom + 1000);
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div,
+            success_action: Y.lp.indicator.actions.scroll_to_target
+        });
+        this.indicator.render();
+        window.scrollTo(1000, 1000);
+        Assert.areEqual(1000, Y.DOM.docScrollX());
+        Assert.areEqual(1000, Y.DOM.docScrollY());
+        this.indicator.setBusy();
+        this.indicator.success();
+        var expected_xy = this.indicator.get('target').getXY();
+        Assert.areEqual(expected_xy[0], Y.DOM.docScrollX(), 'expected x');
+        Assert.areEqual(expected_xy[1], Y.DOM.docScrollY(), 'expected y');
+    },
+
+    test_error_hides_overlay: function () {
+        // Calling error should hide the overlay.
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div
+        });
+        this.indicator.render();
+        this.indicator.setBusy();
+        this.indicator.error();
+        Assert.isFalse(this.indicator.get('visible'), 'visible is not set');
+        Assert.isTrue(this.indicator.get('boundingBox').hasClass(
+            'yui3-overlay-indicator-hidden'), 'class does not exist');
+    },
+
+    test_error_callback: function() {
+        // We should be able to pass in a callback as error_action.
+        var called = false;
+        var callback = function() {
+            called = true;
+        };
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: this.div,
+            error_action: callback
+        });
+        this.indicator.render();
+        this.indicator.error();
+        Assert.isTrue(called);
+    }
+}));
+
+large_indicator.suite = suite;
+
+}, '0.1', {'requires': ['test', 'lp.indicator']});

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2011-12-01 14:36:35 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2011-12-02 14:58:28 +0000
@@ -112,8 +112,7 @@
     forwards_navigation: {valueFn: empty_nodelist},
     io_provider: {value: null},
     pre_fetch: {value: false},
-    navigation_indices: {valueFn: empty_nodelist},
-    spinners: {valueFn: empty_nodelist}
+    navigation_indices: {valueFn: empty_nodelist}
 };
 
 Y.extend(namespace.ListingNavigator, Y.Base, {
@@ -122,18 +121,32 @@
         var cache = lp_client.wrap_resource(null, config.cache);
         var batch_key;
         var template = config.template;
+
         this.set('search_params', namespace.get_query(config.current_url));
         delete this.get('search_params').start;
         delete this.get('search_params').memo;
         delete this.get('search_params').direction;
         delete this.get('search_params').orderby;
+
+        this.set('target', config.target);
         this.set('io_provider', config.io_provider);
+
         batch_key = this.handle_new_batch(cache);
         this.set('model', new namespace.BugListingModel({
                 batch_key: batch_key,
                 field_visibility: cache.field_visibility,
                 field_visibility_defaults: cache.field_visibility_defaults
         }));
+
+        // init the indicator plugin on the target
+        // it defaults to invisible, so safe init
+        this.indicator = new Y.lp.indicator.OverlayIndicator({
+            target: config.target.get('parentNode'),
+            success_action: Y.lp.indicator.actions.scroll_to_target
+        });
+
+        this.indicator.render();
+
         this.pre_fetch_batches();
         // Work around mustache.js bug 48 "Blank lines are not preserved."
         // https://github.com/janl/mustache.js/issues/48
@@ -141,8 +154,7 @@
             template = template.replace(/\n/g, '&#10;');
         }
         this.set('template', template);
-        this.set_pending(false);
-        this.set('target', config.target);
+
         if (Y.Lang.isValue(config.navigation_indices)) {
             this.set('navigation_indices', config.navigation_indices);
         }
@@ -150,14 +162,11 @@
             'change', this.history_changed, this);
     },
 
-    get_failure_handler: function(fetch_only){
+    get_failure_handler: function(){
         var error_handler = new Y.lp.client.ErrorHandler();
         error_handler.showError = Y.bind(
             Y.lp.app.errors.display_error, window, null);
-        if (!fetch_only){
-            error_handler.clearProgressUI = Y.bind(
-                this.set_pending, this, false);
-        }
+        this.indicator.error();
         return error_handler.getFailureHandler();
     },
 
@@ -237,19 +246,11 @@
         });
         var content = Mustache.to_html(this.get('template'), model);
         this.get('target').setContent(content);
+
         this.get('navigation_indices').setContent(batch_info);
         this.render_navigation();
     },
 
-    set_pending: function(is_pending){
-        if (is_pending){
-            this.get('spinners').setStyle('visibility', 'visible');
-        }
-        else{
-            this.get('spinners').setStyle('visibility', 'hidden');
-        }
-    },
-
     has_prev: function(){
         return !Y.Lang.isNull(this.get_current_batch().prev);
     },
@@ -273,7 +274,6 @@
         if (fetch_only) {
             return;
         }
-        this.set_pending(false);
         this.update_from_cache(query, batch_key);
     },
 
@@ -288,6 +288,7 @@
         var url = '?' + Y.QueryString.stringify(query);
         this.get('model').get('history').addValue(
             'batch_key', batch_key, {url: url});
+        this.indicator.success();
     },
 
     /**
@@ -332,9 +333,12 @@
      * will be retrieved and cached upon retrieval.
      */
     update: function(config) {
+        this.indicator.setBusy();
+
         var key = this.constructor.get_batch_key(config);
         var cached_batch = this.get('batches')[key];
         var query = this.get_batch_query(config);
+
         if (Y.Lang.isValue(cached_batch)) {
             if (config.fetch_only){
                 return;
@@ -447,17 +451,15 @@
             on: {
                 success: Y.bind(
                     this.update_from_new_model, this, query, fetch_only),
-                failure: this.get_failure_handler(fetch_only)
+                failure: this.get_failure_handler()
             }
         };
         var context = this.get_current_batch().context;
         var view_name = this.get_current_batch().view_name;
+
         if (Y.Lang.isValue(this.get('io_provider'))) {
             load_model_config.io_provider = this.get('io_provider');
         }
-        if (!fetch_only){
-            this.set_pending(true);
-        }
         Y.lp.client.load_model(
             context, view_name, load_model_config, query);
     }
@@ -475,12 +477,6 @@
             new_node.addClass(class_name);
             new_node.setContent(node.getContent());
             node.replace(new_node);
-            if (class_name === 'first'){
-                var spinner_node = Y.Node.create(
-                    '<img class="spinner" src="/@@/spinner"'+
-                    ' alt="Loading..." />');
-                new_node.insertBefore(spinner_node, new_node);
-            }
         });
     });
 };
@@ -509,8 +505,7 @@
         template: LP.mustache_listings,
         target: target,
         navigation_indices: navigation_indices,
-        pre_fetch: Boolean(pre_fetch),
-        spinners: Y.all('.spinner')
+        pre_fetch: Boolean(pre_fetch)
     });
     navigator.set('backwards_navigation', Y.all('.first,.previous'));
     navigator.set('forwards_navigation', Y.all('.last,.next'));
@@ -541,4 +536,8 @@
 };
 
 
-}, "0.1", {"requires": ["history", "node", 'lp.client', 'lp.app.errors']});
+}, "0.1", {
+    "requires": [
+        "history", "node", 'lp.client', 'lp.app.errors', 'lp.indicator'
+    ]
+});

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.html'
--- lib/lp/bugs/javascript/tests/test_buglisting.html	2011-11-10 19:53:21 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.html	2011-12-02 14:58:28 +0000
@@ -30,6 +30,8 @@
       src="../../../contrib/javascript/mustache.js"></script>
   <script type="text/javascript"
       src="../../../app/javascript/overlay/overlay.js"></script>
+  <script type="text/javascript"
+      src="../../../app/javascript/indicator/indicator.js"></script>
 
     <!-- The module under test -->
     <script type="text/javascript"

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js	2011-12-01 14:36:35 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js	2011-12-02 14:58:28 +0000
@@ -11,12 +11,23 @@
 
 suite.add(new Y.Test.Case({
     name: 'ListingNavigator',
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
     test_sets_search_params: function() {
         // search_parms includes all query values that don't control batching
         var navigator = new module.ListingNavigator({
             current_url: 'http://yahoo.com?foo=bar&start=1&memo=2&;' +
                 'direction=3&orderby=4',
-            cache: {next: null, prev: null}});
+            cache: {next: null, prev: null},
+            target: this.target
+        });
         Y.lp.testing.assert.assert_equal_structure(
             {foo: 'bar'}, navigator.get('search_params'));
     },
@@ -29,7 +40,8 @@
                 mustache_model: {bugtasks: [{show_item: true} ] },
                 next: null,
                 prev: null
-            }
+            },
+            target: this.target
         });
         bugtask = navigator.get_current_batch().mustache_model.bugtasks[0];
         Y.Assert.isFalse(bugtask.hasOwnProperty('show_item'));
@@ -37,7 +49,6 @@
     test_cleans_visibility_from_new_batch: function() {
         // When new batch is handled, field visibility is stripped.
         var bugtask;
-        var target = Y.Node.create('<div id="client-listing"></div>');
         var model = {
             mustache_model: {bugtasks: []},
             memo: 1,
@@ -50,7 +61,7 @@
             current_url: '',
             cache: model,
             template: '',
-            target: target
+            target: this.target
         });
         var batch = {
             mustache_model: {
@@ -70,12 +81,19 @@
 suite.add(new Y.Test.Case({
     name: 'render',
 
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+
     tearDown: function() {
         Y.one('#fixture').setContent('');
+        this.target.remove();
+        delete this.target;
     },
 
     get_render_navigator: function() {
-        var target = Y.Node.create('<div id="client-listing"></div>');
         var lp_cache = {
             mustache_model: {
                 bugtasks: [{foo: 'bar', show_foo: true}]
@@ -92,7 +110,7 @@
         var navigator =  new module.ListingNavigator({
             cache: lp_cache,
             template: template,
-            target: target
+            target: this.target
         });
         var index = Y.Node.create(
             '<div><strong>3</strong> &rarr; <strong>4</strong>' +
@@ -202,6 +220,17 @@
 suite.add(new Y.Test.Case({
     name: 'first_batch',
 
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
+
     /**
      * Return a ListingNavigator ordered by 'intensity'
      */
@@ -222,13 +251,12 @@
             field_visibility: {},
             field_visibility_defaults: {}
         };
-        var target = Y.Node.create('<div id="client-listing"></div>');
         var navigator = new module.ListingNavigator({
             current_url:
                 "http://yahoo.com?start=5&memo=6&direction=backwards";,
             cache: lp_cache,
             template: "<ol>" + "{{#item}}<li>{{name}}</li>{{/item}}</ol>",
-            target: target,
+            target: this.target,
             io_provider: mock_io
         });
         navigator.first_batch('intensity');
@@ -285,6 +313,17 @@
 suite.add(new Y.Test.Case({
     name: 'Batch caching',
 
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
+
     test_update_from_new_model_caches: function() {
         /* update_from_new_model caches the settings in the module.batches. */
         var lp_cache = {
@@ -302,18 +341,18 @@
         };
         var template = "<ol>" +
             "{{#item}}<li>{{name}}</li>{{/item}}</ol>";
-        var target = Y.Node.create('<div id="client-listing"></div>');
         var navigator = new module.ListingNavigator({
             current_url: window.location,
             cache: lp_cache,
             template: template,
-            target: target
+            target: this.target
         });
         var key = module.ListingNavigator.get_batch_key({
             order_by: "intensity",
             memo: 'memo1',
             forwards: true,
-            start: 5
+            start: 5,
+            target: this.target
         });
         var batch = {
             order_by: 'intensity',
@@ -340,6 +379,7 @@
             order_by: 'order_by1',
             memo: 'memo1',
             forwards: true,
+            target: this.target,
             start: 5});
         Y.Assert.areSame('["order_by1","memo1",true,5]', key);
     }
@@ -360,12 +400,24 @@
 suite.add(new Y.Test.Case({
     name: 'get_batch_url',
 
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
+
     /**
      * get_batch_query accepts the order_by param.
      */
     test_get_batch_query_orderby: function() {
         var navigator = new module.ListingNavigator({
             current_url: '?param=1',
+            target: this.target,
             cache: {next: null, prev: null}
         });
         var query = navigator.get_batch_query({order_by: 'importance'});
@@ -378,6 +430,7 @@
     test_get_batch_query_memo: function() {
         var navigator = new module.ListingNavigator({
             current_url: '?param=foo',
+            target: this.target,
             cache: {next: null, prev: null}
         });
         var query = navigator.get_batch_query({memo: 'pi'});
@@ -390,7 +443,8 @@
     test_get_batch_null_memo: function() {
         var navigator = new module.ListingNavigator({
             current_url: '?memo=foo',
-            cache: {next: null, prev: null}
+            cache: {next: null, prev: null},
+            target: this.target
         });
         var query = navigator.get_batch_query({memo: null});
         Y.Assert.areSame(undefined, query.memo);
@@ -401,7 +455,8 @@
     test_get_batch_query_forwards: function() {
         var navigator = new module.ListingNavigator({
             current_url: '?param=pi&direction=backwards',
-            cache: {next: null, prev: null}
+            cache: {next: null, prev: null},
+            target: this.target
         });
         var query = navigator.get_batch_query({forwards: true});
         Y.Assert.areSame('pi', query.param);
@@ -413,7 +468,8 @@
     test_get_batch_query_backwards: function() {
         var navigator = new module.ListingNavigator({
             current_url: '?param=pi',
-            cache: {next: null, prev: null}
+            cache: {next: null, prev: null},
+            target: this.target
         });
         var query = navigator.get_batch_query({forwards: false});
         Y.Assert.areSame('pi', query.param);
@@ -425,7 +481,8 @@
     test_get_batch_query_start: function() {
         var navigator = new module.ListingNavigator({
             current_url: '?start=pi',
-            cache: {next: null, prev:null}
+            cache: {next: null, prev:null},
+            target: this.target
         });
         var query = navigator.get_batch_query({});
         Y.Assert.areSame(undefined, query.start);
@@ -438,9 +495,6 @@
 
 var get_navigator = function(url, config) {
     var mock_io = new Y.lp.testing.mockio.MockIo();
-    if (config === undefined){
-        config = {};
-    }
     lp_cache = {
         context: {
             resource_type_link: 'http://foo_type',
@@ -466,13 +520,12 @@
     if (config.no_prev){
         lp_cache.prev = null;
     }
-    var target = Y.Node.create('<div id="client-listing"></div>');
     var navigator_config = {
         current_url: url,
         cache: lp_cache,
         io_provider: mock_io,
         pre_fetch: config.pre_fetch,
-        target: target,
+        target: config.target,
         template: ''
     };
     if (Y.Lang.isValue(config.spinners)){
@@ -484,8 +537,19 @@
 suite.add(new Y.Test.Case({
     name: 'navigation',
 
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
+
     test_model_uses_view_name: function(){
-        var navigator = get_navigator('');
+        var navigator = get_navigator('', {target: this.target});
         navigator.get_current_batch().view_name = '+funbugs';
         navigator.load_model({});
         Y.Assert.areSame(
@@ -499,7 +563,7 @@
      */
     test_last_batch: function() {
         var navigator = get_navigator(
-            '?memo=pi&direction=backwards&start=57');
+            '?memo=pi&direction=backwards&start=57', {target: this.target});
         navigator.last_batch();
         Y.Assert.areSame(
             '/bar/+bugs/++model++?orderby=foo&memo=&start=23&' +
@@ -512,7 +576,8 @@
      * orderby=navigator.current_batch.order_by.
      */
     test_first_batch: function() {
-        var navigator = get_navigator('?memo=pi&start=26');
+        var navigator = get_navigator(
+            '?memo=pi&start=26', {target: this.target});
         navigator.first_batch();
         Y.Assert.areSame(
             '/bar/+bugs/++model++?orderby=foo&start=0',
@@ -524,7 +589,8 @@
      * current_batch.ordering.
      */
     test_next_batch: function() {
-        var navigator = get_navigator('?memo=pi&start=26');
+        var navigator = get_navigator(
+            '?memo=pi&start=26', {target: this.target});
         navigator.next_batch();
         Y.Assert.areSame(
             '/bar/+bugs/++model++?orderby=foo&memo=467&start=500',
@@ -535,7 +601,8 @@
      * Calling next_batch when there is none is a no-op.
      */
     test_next_batch_missing: function() {
-        var navigator = get_navigator('?memo=pi&start=26', {no_next: true});
+        var navigator = get_navigator(
+            '?memo=pi&start=26', {no_next: true, target: this.target});
         navigator.next_batch();
         Y.Assert.areSame(
             null, navigator.get('io_provider').last_request);
@@ -546,7 +613,8 @@
      * and ordering=current_batch.ordering.
      */
     test_prev_batch: function() {
-        var navigator = get_navigator('?memo=pi&start=26');
+        var navigator = get_navigator(
+            '?memo=pi&start=26', {target: this.target});
         navigator.prev_batch();
         Y.Assert.areSame(
             '/bar/+bugs/++model++?orderby=foo&memo=457&start=400&' +
@@ -559,7 +627,8 @@
      */
     test_prev_batch_missing: function() {
         var navigator = get_navigator(
-            '?memo=pi&start=26', {no_prev: true, no_next: true});
+            '?memo=pi&start=26',
+            {no_prev: true, no_next: true, target: this.target});
         navigator.prev_batch();
         Y.Assert.areSame(
             null, navigator.get('io_provider').last_request);
@@ -569,11 +638,22 @@
 suite.add(new Y.Test.Case({
     name: 'browser history',
 
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
+
     /**
      * Update from cache generates a change event for the specified batch.
      */
     test_update_from_cache_generates_event: function(){
-        var navigator = get_navigator('');
+        var navigator = get_navigator('', {target: this.target});
         var e = null;
         navigator.get('model').get('history').on('change', function(inner_e){
             e = inner_e;
@@ -595,7 +675,7 @@
      * batch and is rendered.
      */
     test_change_event_renders_cache: function(){
-        var navigator = get_navigator('');
+        var navigator = get_navigator('', {target: this.target});
         var batch = {
             mustache_model: {
                 bugtasks: [],
@@ -615,7 +695,6 @@
 
 suite.add(new Y.Test.Case({
     name: 'from_page tests',
-
     setUp: function() {
         window.LP = {
             cache: {
@@ -647,12 +726,20 @@
 
 suite.add(new Y.Test.Case({
     name: "pre-fetching batches",
-
+    setUp: function() {
+        this.target = Y.Node.create('<div></div>').set(
+            'id', 'client-listing');
+        Y.one('body').appendChild(this.target);
+    },
+    tearDown: function() {
+        this.target.remove();
+        delete this.target;
+    },
     /**
      * get_pre_fetch_configs should return a config for the next batch.
      */
     test_get_pre_fetch_configs: function(){
-        var navigator = get_navigator();
+        var navigator = get_navigator('', {target: this.target});
         var configs = navigator.get_pre_fetch_configs();
         var batch_keys = [];
         Y.each(configs, function(value){
@@ -666,7 +753,8 @@
      * get_pre_fetch_configs should return an empty list if no next batch.
      */
     test_get_pre_fetch_configs_no_next: function(){
-        var navigator = get_navigator('', {no_next: true});
+        var navigator = get_navigator(
+            '', {no_next: true, target: this.target});
         var configs = navigator.get_pre_fetch_configs();
         var batch_keys = [];
         Y.each(configs, function(value){
@@ -687,7 +775,7 @@
      * Calling pre_fetch_batches should produce a request for the next batch.
      */
     test_pre_fetch_batches: function(){
-        var navigator = this.get_pre_fetch_navigator();
+        var navigator = this.get_pre_fetch_navigator({target: this.target});
         var io_provider = navigator.get('io_provider');
         navigator.set('pre_fetch', true);
         Y.Assert.isNull(io_provider.last_request);
@@ -703,7 +791,7 @@
      */
     test_pre_fetch_disabled: function(){
         var last_url;
-        var navigator = this.get_pre_fetch_navigator();
+        var navigator = this.get_pre_fetch_navigator({target: this.target});
         navigator.pre_fetch_batches();
         Y.Assert.areSame(null, navigator.get('io_provider').last_request);
     },
@@ -712,7 +800,8 @@
      * Initialization does a pre-fetch.
      */
     test_pre_fetch_on_init: function(){
-        var navigator = get_navigator('', {pre_fetch: true});
+        var navigator = get_navigator(
+            '', {pre_fetch: true, target:this.target});
         var last_url = navigator.get('io_provider').last_request.url;
         Y.Assert.areSame(
             last_url,
@@ -722,7 +811,7 @@
      * update_from_new_model does a pre-fetch.
      */
     test_pre_fetch_on_update_from_new_model: function(){
-        var navigator = get_navigator('');
+        var navigator = get_navigator('', {target: this.target});
         var io_provider = navigator.get('io_provider');
         var lp_client = new Y.lp.client.Launchpad();
         var batch = lp_client.wrap_resource(null, {
@@ -755,97 +844,6 @@
     }
 }));
 
-suite.add(new Y.Test.Case({
-    name: "spinners",
-
-    get_navigator: function() {
-        var spinner = Y.Node.create('<img src="/spinner" />');
-        var navigator = get_navigator(
-            '', {spinners: new Y.NodeList([spinner])});
-        Y.Assert.areSame(navigator.get('spinners').item(0), spinner);
-        return navigator;
-    },
-
-    /**
-     * On init, spinners are hidden.
-     */
-    test_spinners_default_hidden: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        Y.Assert.areEqual('hidden', spinner.getStyle('visibility'));
-    },
-
-    /**
-     * load_model shows the spinners.
-     */
-    test_load_model_shows_spinner: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        navigator.load_model({});
-        Y.Assert.areEqual('visible', spinner.getStyle('visibility'));
-    },
-
-    /**
-     * load_model ignores the spinners if fetch_only is set.
-     */
-    test_load_model_fetch_only_ignores_spinner: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        navigator.load_model({}, true);
-        Y.Assert.areEqual('hidden', spinner.getStyle('visibility'));
-    },
-
-    /**
-     * After a successful load, the spinners are hidden.
-     */
-    test_success_clears_spinner: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        var mock_io = navigator.get('io_provider');
-        navigator.load_model({});
-        mock_io.last_request.successJSON({mustache_model: {bugtasks: []}});
-        Y.Assert.areEqual('hidden', spinner.getStyle('visibility'));
-    },
-
-    /**
-     * After a successful load with fetch_only, spinners are left alone.
-     */
-    test_fetch_only_success_ignores_spinner: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        var mock_io = navigator.get('io_provider');
-        navigator.set_pending(true);
-        navigator.load_model({}, true);
-        mock_io.last_request.successJSON({mustache_model: {bugtasks: []}});
-        Y.Assert.areEqual('visible', spinner.getStyle('visibility'));
-    },
-
-    /**
-     * When a load fails, the spinners are hidden
-     */
-    test_failure_clears_spinner: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        var mock_io = navigator.get('io_provider');
-        navigator.load_model({});
-        mock_io.failure();
-        Y.Assert.areEqual('hidden', spinner.getStyle('visibility'));
-    },
-
-    /**
-     * When a fetch-only load fails, the spinners are left alone.
-     */
-    test_fetch_only_failure_ignores_spinner: function(){
-        var navigator = this.get_navigator();
-        var spinner = navigator.get('spinners').item(0);
-        var mock_io = navigator.get('io_provider');
-        navigator.load_model({}, true);
-        navigator.set_pending(true);
-        mock_io.failure();
-        Y.Assert.areEqual('visible', spinner.getStyle('visibility'));
-    }
-}));
-
 var handle_complete = function(data) {
     window.status = '::::' + JSON.stringify(data);
     };