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/92024

= Summary =
The LPJS object isn't created in the load-javascript template metal, but in the page-javascript one. Not all pages use the page-javascript, it's completely the standard so it should be moved to load-javascript.

== Proposed Fix ==
Move the LPJS definition up to load-javascript.

== Demo and Q/A ==
All JS should work. Tests still pass, and we can check that it loads correctly from the timeline graph as well.

The bug report url was
https://launchpad.net/versioneer/+timeline-graph?:73
-- 
https://code.launchpad.net/~rharding/launchpad/graph_lpjs_928500/+merge/92024
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-01 15:36:13 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2012-02-08 13:30:26 +0000
@@ -124,6 +124,13 @@
             }
         }">
    </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>
 </metal:load-javascript>
 
@@ -139,10 +146,6 @@
   <metal:load-lavascript use-macro="context/@@+base-layout-macros/load-javascript" />
 
     <script id="base-layout-load-scripts" 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();
         LPJS.use('base', 'node', 'console', 'event',
             'oop', 'lp', 'lp.app.privacy',
             'lp.app.beta_features', 'lp.app.foldables','lp.app.sorttable',