launchpad-reviewers team mailing list archive

[Huw Wilkins <huw.wilkins@xxxxxxxxxxxxx>]

Huw Wilkins has proposed merging lp:~huwshimi/launchpad/bug-listing-spinner-position-904416 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #904416 in Launchpad itself: "bug listing spinner is (typically) not visible"
  https://bugs.launchpad.net/launchpad/+bug/904416

For more details, see:
https://code.launchpad.net/~huwshimi/launchpad/bug-listing-spinner-position-904416/+merge/85997

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.
-- 
https://code.launchpad.net/~huwshimi/launchpad/bug-listing-spinner-position-904416/+merge/85997
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~huwshimi/launchpad/bug-listing-spinner-position-904416 into lp:launchpad.

=== 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');
         },
 
         /**