launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #928500 in Launchpad itself: "'Series and Milestones' graph not loading - LPJS is not defined"
  https://bugs.launchpad.net/launchpad/+bug/928500

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

= Summary =
The LPJS definition was accidentally inside of the feature flag condition check when it should be outside since it occurs regardless of combo loader or not.

== Proposed Fix ==
Move the LPJS definition outside the tal condition.

== Demo and Q/A ==
Make sure JS isn't broken for non combo loader feature users.

-- 
https://code.launchpad.net/~rharding/launchpad/graph_lpjs_928500/+merge/92257
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/graph_lpjs_928500 into lp:launchpad.

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2012-02-08 13:25:45 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2012-02-09 11:53:00 +0000
@@ -125,13 +125,14 @@
         }">
    </script>
 
-   <script type="text/javascript">
-       // we need this to create a single YUI instance all events and code
-       // talks across. All instances of YUI().use should be based off of
-       // LPJS instead.
-       LPJS = new YUI();
-   </script>
   </tal:js-loader>
+  <script type="text/javascript">
+      // we need this to create a single YUI instance all events and code
+      // talks across. All instances of YUI().use should be based off of
+      // LPJS instead.
+      LPJS = new YUI();
+  </script>
+
 </metal:load-javascript>
 
 <metal:page-javascript define-macro="page-javascript"