Merge lp:~huwshimi/launchpad/bug-listing-spinner-position-904416 into lp:launchpad

[Huw Wilkins]

[r=adeuring][bug=904416] Repositioned the loading spinner on bug listings to the top.

The spinner that was shown when a bug listing was loading was centered vertically on the listing. This meant that for long listings the spinner was never visible.

This branch moves the spinner to the top of the list. This means the spinner will be visible when the ordering and top pagination controls are used. Unfortunately this means the spinner still won't be visible when the bottom pagination controls are used on long listings, but I feel this is a big enough improvement to consider the fix good enough for now.

Here is a screenshot of the new position: https://launchpadlibrarian.net/87618550/spinner_position.png

The JavaScript code that is removed in this branch is what was previously used to position the spinner. CSS now positions the spinner.

[Abel Deuring (adeuring)]

review: Approve (code)

[Francis J. Lacoste (flacoste)]

Couldn't position relative to the viewport instead of the content, so that it's always visible?

[Huw Wilkins (huwshimi)]

Hi Francis,

That's certainly a possibility, do you mean that it would follow the viewport if you scroll or that it is initially positioned in relation to the viewport?

[Francis J. Lacoste (flacoste)]

I was thinking always positioning it in relation to the viewport. But on the checkpoint call, Rick and Deryck explained that this would be kind of messy because the current interaction grays out the underlying the listing div. So you'd need to position it relative to the intersection of the viewport and the div. Otherwise, the interaction would need to be redesigned to be independent of the div.

=== modified file 'lib/lp/app/javascript/indicator/assets/indicator-core.css'
--- lib/lp/app/javascript/indicator/assets/indicator-core.css	2011-12-01 21:46:27 +0000
+++ lib/lp/app/javascript/indicator/assets/indicator-core.css	2011-12-16 06:40:30 +0000
@@ -6,11 +6,12 @@
     z-index: 999;
     }
 
+.yui3-overlay-indicator-content {
+    text-align: center;
+    }
+
 .yui3-overlay-indicator img {
-    display: block;
-    margin: auto;
-    max-width: 100%;
-    position: absolute;
+    margin-top: 8em;
     }
 
 .yui3-overlay-indicator-hidden {

=== modified file 'lib/lp/app/javascript/indicator/indicator.js'
--- lib/lp/app/javascript/indicator/indicator.js	2011-12-06 16:05:36 +0000
+++ lib/lp/app/javascript/indicator/indicator.js	2011-12-16 06:40:30 +0000
@@ -13,10 +13,6 @@
  */
 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: {
             /**
@@ -56,17 +52,6 @@
 
     var config = {
 
-        /**
-         * Calculate the padding we need to use to center the spinner.
-         */
-        _calc_image_padding: function (dim) {
-            var boundingBox = this.get('boundingBox');
-            var content_dimension =
-                boundingBox.getStyle(dim).replace('px', '');
-            return content_dimension > SPINNER_SIZE ?
-                (content_dimension - SPINNER_SIZE) / 2 : 0;
-        },
-
         initializer: function(cfg) {
             this.hide();
         },
@@ -133,10 +118,6 @@
             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');
         },
 
         /**