← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rharding/launchpad/heat_help_894740 into lp:launchpad

 

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #894740 in Launchpad itself: "bug heat contextual help opens in a new tab in chromium"
  https://bugs.launchpad.net/launchpad/+bug/894740

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

= Summary =
In Google Chrome the overlay help ui isn't loading. We also need to make sure that it gets loaded on all ajax navigation calls from the new buglisting ui.

== Implementation Details ==
Chrome fires a history pop event on page load that caused the ui to update after the help click events had normally been attached. I added a call to initInlineHelp as part of the render cycle so that whenever we update the bug listing html we'll also rebind the help javascript.

This function is idempotent, so multiple calls don't adversely effect other help items elsewhere on the page. It will only hit once per target.

I double checked the rest of the normal javascript ui events that occur on page load, we didn't need to duplicate any others for our ui. These can be found in the base-layout-macros.pt [#base-layout-load-scripts].

== Demo && Q/A ==
Load any page with the new buglisting and make sure that you display the heat field. Clicking on the heat icon should load an overlay with the details. Clicking next and paging through the results should keep the heat icons clickable and loading the overlay.

== Tests ==
google-chrome lib/lp/bugs/javascript/tests/test_buglisting.html  
-- 
https://code.launchpad.net/~rharding/launchpad/heat_help_894740/+merge/85901
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/heat_help_894740 into lp:launchpad.
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2011-12-14 07:47:38 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2011-12-15 17:16:26 +0000
@@ -122,7 +122,7 @@
             Y.lp.activate_collapsibles();
             activateFoldables();
             activateConstrainBugExpiration();
-	    Y.lp.app.links.check_valid_lp_links();
+            Y.lp.app.links.check_valid_lp_links();
             // Longpolling will only start if
             // LP.cache.longpoll is populated.
             // We use Y.later to work around a Safari/Chrome 'feature':

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2011-12-14 19:23:02 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2011-12-15 17:16:26 +0000
@@ -121,6 +121,10 @@
 Y.extend(
     module.BugListingNavigator,
     Y.lp.app.listing_navigator.ListingNavigator, {
+    _bindUI: function () {
+        initInlineHelp();
+    },
+
     initializer: function(config) {
         this.constructor.superclass.initializer.apply(this, arguments);
         this.get('model').get('history').after(
@@ -135,6 +139,7 @@
             var batch = this.get('batches')[batch_key];
             this.pre_fetch_batches();
             this.render();
+            this._bindUI();
         }
         else {
             // Handle Chrom(e|ium)'s initial popstate.

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.html'
--- lib/lp/bugs/javascript/tests/test_buglisting.html	2011-12-13 18:33:07 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.html	2011-12-15 17:16:26 +0000
@@ -1,6 +1,12 @@
 <html>
   <head>
   <title>Bug task listing</title>
+  <script type="text/javascript">
+      // this is a fake out of the global help overlay tool we need to call
+      var initInlineHelp = function () {
+          return;
+      };
+  </script>
 
   <!-- YUI and test setup -->
   <script type="text/javascript"