launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/bug_814697_missing_spinner into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #814697 in Launchpad itself: "Inline diffs in merge proposals don't have a spinner when it's waiting for the diff"
  https://bugs.launchpad.net/launchpad/+bug/814697

For more details, see:
https://code.launchpad.net/~rharding/launchpad/bug_814697_missing_spinner/+merge/83034

= Summary = 
The inline diff loading wasn't showing our spinner graphic. 

== Proposed Fix ==
Added the spinner to the hidden text in the template macro that's shown while the ajax request for the diff is loading.

== Implementation Details ==
Our original spinner was a .gif ona white background. In this location, it needs to display properly on a grey background. Per a conversation with Deryck and notes from Huw, loaded the 'indicator' style spinner from http://ajaxload.info/ on a transparent background with #000 foreground to replace our older spinner graphic.

One note is that since the .gif is transparent, we might see some small artifacts around the edges. The graphic is small and busy enough that I don't think it's an issue and the advantage of one true spinner to rule them all is worth the trade off.

== Q/A ==
Load a page with the diffs within the branch and click "Changes added by revision"
For example: https://code.launchpad.net/~mbp/launchpad/808282-canonical-url/+merge/82811

You should now see a spinner. All other spinner instances should also show the new graphic on a transparent background.
-- 
https://code.launchpad.net/~rharding/launchpad/bug_814697_missing_spinner/+merge/83034
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/bug_814697_missing_spinner into lp:launchpad.

=== modified file 'lib/canonical/launchpad/images/spinner.gif'
Binary files lib/canonical/launchpad/images/spinner.gif	2008-06-18 19:50:01 +0000 and lib/canonical/launchpad/images/spinner.gif	2011-11-22 15:25:51 +0000 differ
=== modified file 'lib/lp/code/templates/branch-macros.pt'
--- lib/lp/code/templates/branch-macros.pt	2011-11-21 04:22:05 +0000
+++ lib/lp/code/templates/branch-macros.pt	2011-11-22 15:25:51 +0000
@@ -214,7 +214,7 @@
       <tal:plural condition="python: start_revno!=last_revno">
         to revision <tal:revno replace="last_revno">2</tal:revno>
       </tal:plural></a>
-  <div class="unseen expander-content">Loading diff</div>
+      <div class="unseen expander-content">Loading diff <img src="/@@/spinner"/></div>
   </div>
       </tal:diff-expander>
     </tal:ajax-revision-diffs>