← Back to team overview

launchpad-reviewers team mailing list archive

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

 

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #907443 in Launchpad itself: "port inline help to YUI"
  https://bugs.launchpad.net/launchpad/+bug/907443

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

= Summary =
Move the inline help javascript to YUI. This is in an effort to move towards an all YUI code base as part of Bug #294656.

== Proposed Fix ==
Add a new lp.app.inlinehelp module that can be used for showing the inline help.

== Implementation Details ==
The inlinehelp module adds an InlineHelpOverlay that extends the PrettyOverlay. The iframe for the help content is loaded into that overlay. This brings some consistency to the overlay used.

The init_help method call will build an event delegate on the <body> tag of the page. This helps keep only one event binding for the entire page regardless of how many help objects there are on the page. It can be called over and over as content changes via ajax or other dynamic methods.

== Tests ==
google-chrome lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.html

== Demo and Q/A ==
Click on any help icon (question mark) and the help for that should load centered on the page. A prime example is the heat help during the bug listings. Clicking on the heat icon should load up a scrolling overlay with the help information for bug heat.

== Lint ==
Linting changed files:
  lib/lp/app/javascript/inlinehelp/inlinehelp.js
  lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.js
  lib/lp/app/templates/base-layout-macros.pt
-- 
https://code.launchpad.net/~rharding/launchpad/port_inlinehelp_907443/+merge/87384
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/port_inlinehelp_907443 into lp:launchpad.
=== modified file 'buildout-templates/bin/combine-css.in'
--- buildout-templates/bin/combine-css.in	2011-12-06 20:10:56 +0000
+++ buildout-templates/bin/combine-css.in	2012-01-03 17:07:31 +0000
@@ -35,6 +35,7 @@
     'build/autocomplete/assets/skins/sam/autocomplete.css',
     'build/overlay/assets/skins/sam/pretty-overlay.css',
     'build/formoverlay/assets/formoverlay-core.css',
+    'build/inlinehelp/assets/inlinehelp-core.css',
     'build/indicator/assets/indicator-core.css',
     'build/confirmationoverlay/assets/confirmationoverlay-core.css',
     'build/picker/assets/skins/sam/picker.css',

=== added directory 'lib/lp/app/javascript/inlinehelp'
=== removed symlink 'lib/lp/app/javascript/inlinehelp'
=== target was u'../../../lp/services/inlinehelp/javascript'
=== added directory 'lib/lp/app/javascript/inlinehelp/assets'
=== added file 'lib/lp/app/javascript/inlinehelp/assets/inlinehelp-core.css'
--- lib/lp/app/javascript/inlinehelp/assets/inlinehelp-core.css	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/inlinehelp/assets/inlinehelp-core.css	2012-01-03 17:07:31 +0000
@@ -0,0 +1,16 @@
+.yui3-inlinehelp-overlay {
+    width: 600px;
+}
+
+.yui3-inlinehelp-overlay iframe {
+    width: 100%;
+    height: 300px;
+    border: 0;
+    padding: 0;
+    margin: 0;
+    margin-top: 1em;
+}
+
+.yui3-inlinehelp-overlay-hidden {
+    visibility: hidden;
+}

=== added directory 'lib/lp/app/javascript/inlinehelp/assets/skins'
=== added directory 'lib/lp/app/javascript/inlinehelp/assets/skins/sam'
=== added file 'lib/lp/app/javascript/inlinehelp/assets/skins/sam/inlinehelp-skin.css'
--- lib/lp/app/javascript/inlinehelp/assets/skins/sam/inlinehelp-skin.css	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/inlinehelp/assets/skins/sam/inlinehelp-skin.css	2012-01-03 17:07:31 +0000
@@ -0,0 +1,3 @@
+/* Copyright (c) 2009, Canonical Ltd. All rights reserved. */
+
+/* Placeholder for skinning of the InlineHelp Overlay Widget */

=== added file 'lib/lp/app/javascript/inlinehelp/inlinehelp.js'
--- lib/lp/app/javascript/inlinehelp/inlinehelp.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/inlinehelp/inlinehelp.js	2012-01-03 17:07:31 +0000
@@ -0,0 +1,129 @@
+/**
+ * Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * @module lp.app.inlinehelp
+ *
+ * Usage:
+ *      lp.app.inlinehelp.init_help();
+ *
+ */
+YUI.add('lp.app.inlinehelp', function (Y) {
+
+    var module = Y.namespace('lp.app.inlinehelp');
+    var HELP_LINK_SELECTOR = 'a[target=help]';
+    var HELP_CSS = 'help';
+    var CLICK_DELEGATE = false;
+
+    /**
+     * Handle the clicking of a help link in the body
+     * This is a delegated handler so this == the object clicked
+     *
+     * @method _show_help
+     * @private
+     */
+    module._show_help = function (e) {
+        e.preventDefault();
+        var target_link = e.currentTarget;
+
+        // init the overlay and show it
+        var overlay = new module.InlineHelpOverlay({
+            'contentUrl': target_link.get('href'),
+            'centered': true,
+            'constrain': true
+        });
+        overlay.render();
+    };
+
+    /**
+     * The single entry point used to bind the buttons for launching help
+     *
+     * @method init_help
+     * @public
+     */
+    module.init_help =  function () {
+        // find the help links
+        var links = Y.all(HELP_LINK_SELECTOR);
+
+        // add the help class
+        links.addClass(HELP_CSS);
+
+        // bind the click events but unbind it first in case we're re-running
+        // init more than once (say on ajax loading of new help content)
+        var body = Y.one('body');
+        if (CLICK_DELEGATE !== false) {
+            CLICK_DELEGATE.detach();
+        }
+        CLICK_DELEGATE = body.delegate(
+            'click',
+            module._show_help,
+            HELP_LINK_SELECTOR
+        );
+    };
+
+    module.InlineHelpOverlay = Y.Base.create(
+        'inlinehelp-overlay',
+        Y.lazr.PrettyOverlay,
+        [],
+        {
+            /**
+             * Generate the iframe used for displaying help content in the
+             * overlay.
+             *
+             * @method _getContent
+             * @private
+             */
+            _getContent: function () {
+                var help_page = Y.Node.create('<iframe/>');
+                help_page.set('src', this.get('contentUrl'));
+
+                // use the overlay bodyContent as the home of the iframe
+                this.set('bodyContent', help_page);
+            },
+
+            initializer: function (cfg) {
+                this._getContent();
+            },
+
+            hide: function() {
+                this.constructor.superclass.hide.call(this);
+                this.get('boundingBox').setStyle('display', 'none');
+            },
+
+            show: function() {
+                this.constructor.superclass.show.call(this);
+                this.get('boundingBox').setStyle('display', 'block');
+            }
+        },
+        {
+            ATTRS: {
+                /**
+                 * URI of the location of the help content.
+                 *
+                 * This is loaded into our iFrame and should be a full page vs
+                 * a data payload.
+                 *
+                 * @attribute contentUrl
+                 * @type string
+                 * @default ''
+                 */
+                contentUrl: {
+                    value: ''
+                },
+
+                /**
+                 * There's no multi steps so hard code the underlying overlays
+                 * bar to false
+                 *
+                 * @attribute progressbar
+                 * @type bool
+                 * @default false
+                 */
+                progressbar: {
+                    value: false
+                }
+            }
+        }
+    );
+
+}, "0.1", { "requires": ['lazr.overlay', 'io', 'log'] });

=== added directory 'lib/lp/app/javascript/inlinehelp/tests'
=== added file 'lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.html'
--- lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.html	2012-01-03 17:07:31 +0000
@@ -0,0 +1,31 @@
+<!--
+Copyright 2011 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd";>
+<html>
+  <head>
+    <title>Test InlineHelp </title>
+    <!-- YUI and test setup -->
+    <script type="text/javascript"
+            src="../../../../../canonical/launchpad/icing/yui/yui/yui.js">
+    </script>
+    <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
+    <script type="text/javascript"
+            src="../../../../app/javascript/testing/testrunner.js"></script>
+
+    <script type="text/javascript" src="../../overlay/overlay.js"></script>
+
+    <!-- The module under test -->
+    <script type="text/javascript" src="../inlinehelp.js"></script>
+
+    <!-- The test suite -->
+    <script type="text/javascript" src="test_inlinehelp.js"></script>
+  </head>
+  <body class="yui3-skin-sam">
+      <ul id="suites">
+          <li>lp.app.inlinehelp.test</li>
+      </ul>
+  </body>
+</html>
+

=== added file 'lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.js'
--- lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/inlinehelp/tests/test_inlinehelp.js	2012-01-03 17:07:31 +0000
@@ -0,0 +1,138 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.app.inlinehelp.test', function (Y) {
+
+    var suite = new Y.Test.Suite('InlineHelp Tests');
+    var Assert = Y.Assert;
+    var test_module = Y.namespace('lp.app.inlinehelp.test');
+
+    suite.add(new Y.Test.Case({
+        name: 'inlinehelp.init_help',
+
+        setUp: function () {
+            var link_html = Y.Node.create(
+                '<a href="/+help-bugs/bug-heat.html" target="help"/>');
+            Y.one('body').appendChild(link_html);
+        },
+
+        tearDown: function () {
+            Y.all('a[target="help"]').remove();
+            Y.one('body').detach('click');
+            Y.all('.pretty-overlay-window').remove();
+        },
+
+        test_adding_css_class: function () {
+            // calling init help should add a help css class to all links with
+            // target=help
+            var called = false;
+            Y.lp.app.inlinehelp.init_help();
+            Y.all('a[target="help"]').each(function (node) {
+                called = true;
+                Y.Assert.isTrue(node.hasClass('help'),
+                    'Each link should have the class "help"');
+            });
+
+            Y.Assert.isTrue(called, 'We should have called our class check');
+        },
+
+        test_binding_click_link: function () {
+            // calling init help should a delegated click handler for the help
+            // links
+
+            // we need to mock out the inlinehelp.show_help function to add a
+            // callable to run tests for us when clicked
+            var orig_show_help = Y.lp.app.inlinehelp._show_help;
+            var called = false;
+
+            Y.lp.app.inlinehelp._show_help = function (e) {
+                e.preventDefault();
+                called = true;
+
+                Y.Assert.areEqual(e.target.get('target'), 'help',
+                    'The event target should be our <a> with target = help');
+            };
+
+            Y.lp.app.inlinehelp.init_help();
+
+            Y.one('a[target="help"]').simulate('click');
+            Y.Assert.isTrue(
+                called,
+                'We should have called our show_help function'
+            );
+
+            // restore the original show_help method for future tests
+            Y.lp.app.inlinehelp._show_help = orig_show_help;
+        },
+
+        test_binding_click_only_once: function () {
+            //verify that multiple calls to init_help only causes one click
+            //event to fire
+            var orig_show_help = Y.lp.app.inlinehelp._show_help;
+            var called = 0;
+
+            Y.lp.app.inlinehelp._show_help = function (e) {
+                e.preventDefault();
+                called = called + 1;
+            };
+
+            Y.lp.app.inlinehelp.init_help();
+            Y.lp.app.inlinehelp.init_help();
+            Y.lp.app.inlinehelp.init_help();
+            Y.lp.app.inlinehelp.init_help();
+
+            Y.one('a[target="help"]').simulate('click');
+            Y.Assert.areEqual(
+                called,
+                1,
+                'We should have called our show_help function only once'
+            );
+            // restore the original show_help method for future tests
+            Y.lp.app.inlinehelp._show_help = orig_show_help;
+        },
+
+        test_click_gets_overlay: function () {
+            // clicking on the link should get us an overlay
+            Y.lp.app.inlinehelp.init_help();
+            Y.one('a[target="help"]').simulate('click');
+            Y.Assert.isObject(Y.one('.yui3-inlinehelp-overlay'),
+                'Should find a node for the overlay');
+        },
+
+        test_click_get_content: function () {
+            // if the contentUrl exists, we should get content. Fudge the ajax
+            // response to return some known html.
+            var orig_show_help = Y.lp.app.inlinehelp._show_help;
+            var good_html =
+                '<iframe src="file:///+help-bugs/bug-heat.html"></iframe>';
+
+            Y.lp.app.inlinehelp._show_help = function (e) {
+                e.preventDefault();
+                var target_link = e.target;
+
+                // init the overlay and show it
+                overlay = new Y.lp.app.inlinehelp.InlineHelpOverlay({
+                    'contentUrl': target_link.get('href')
+                });
+                overlay.render();
+            };
+
+            Y.lp.app.inlinehelp.init_help();
+            Y.one('a[target="help"]').simulate('click');
+
+            Y.Assert.areEqual(
+                good_html,
+                Y.one('.yui3-widget-bd').get('innerHTML'),
+                'The body content should be an iframe with our link target'
+            );
+
+            Y.lp.app.inlinehelp._show_help = orig_show_help;
+        }
+    }));
+
+test_module.suite = suite;
+
+}, '0.1', {
+    'requires': ['node', 'console', 'test', 'lp.app.inlinehelp',
+        'node-event-simulate'
+    ]
+});

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2012-01-03 07:15:56 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2012-01-03 17:07:31 +0000
@@ -115,10 +115,11 @@
     </script>
 
   <script id="base-layout-load-scripts" type="text/javascript">
-    LPS.use('node', 'event-delegate', 'lp', 'lp.app.links', 'lp.app.longpoll', function(Y) {
+    LPS.use('node', 'event-delegate', 'lp', 'lp.app.links', 'lp.app.longpoll',
+        'lp.app.inlinehelp', function(Y) {
         Y.on('load', function(e) {
             sortables_init();
-            initInlineHelp();
+            Y.lp.app.inlinehelp.init_help();
             Y.lp.activate_collapsibles();
             activateFoldables();
             activateConstrainBugExpiration();

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2011-12-16 18:25:29 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2012-01-03 17:07:31 +0000
@@ -122,7 +122,7 @@
     module.BugListingNavigator,
     Y.lp.app.listing_navigator.ListingNavigator, {
     _bindUI: function () {
-        initInlineHelp();
+        Y.lp.app.inlinehelp.init_help();
     },
 
     initializer: function(config) {
@@ -224,5 +224,5 @@
 
 }, "0.1", {
     "requires": [
-        "history", "node", 'lp.app.listing_navigator']
+        "history", "node", 'lp.app.listing_navigator', 'lp.app.inlinehelp']
 });

=== removed directory 'lib/lp/services/inlinehelp/javascript'
=== removed file 'lib/lp/services/inlinehelp/javascript/inlinehelp.js'
--- lib/lp/services/inlinehelp/javascript/inlinehelp.js	2011-03-17 23:25:23 +0000
+++ lib/lp/services/inlinehelp/javascript/inlinehelp.js	1970-01-01 00:00:00 +0000
@@ -1,207 +0,0 @@
-/*
- * Copyright 2009 Canonical Ltd.  This software is licensed under the
- * GNU Affero General Public License version 3 (see the file LICENSE).
- *
- * This script defines functions for popping up a 'Help' dialog for an
- * external site.  All links that have a 'target="help"' attribute will
- * be turned into pop-up help links.  A single popup is present on the
- * screen at a time - opening another help link will open a new dialog,
- * and clicking on the same link again closes the dialog.
- *
- * This library depends on the MochiKit JavaScript library v1.4+.
- *
- */
-
-
-/*
- * Page Setup
- */
-
-
-function initInlineHelp() {
-    /*
-      Activate the popup help system by connecting all of the actionable
-      page elements.
-    */
-    // The button is inserted in the page dynamically:
-    // Changed from an <input type=button> to a <button> since
-    // IE8 doesn't handle style.css's input{visibility:inherit} correctly.
-    $('help-close').innerHTML =
-        '<button id="help-close-btn">Close</button>';
-    forEach(findHelpLinks(), setupHelpTrigger);
-    initHelpPane();
-}
-
-function findHelpLinks() {
-    /*
-      Return all of the links in the document that have a target="help"
-      attribute value.
-    */
-    has_help_target = function (elem) {
-        return getNodeAttribute(elem, 'target') == 'help';
-    };
-    return filter(has_help_target,
-                  currentDocument().getElementsByTagName('a'));
-}
-
-function setupHelpTrigger(elem) {
-    /*
-      Turn the specified element into a proper help link: add the
-      'class="help"' attribute if it is missing, and connect the
-      necessary event handlers.
-    */
-    // We want this to be idempotent, so we treat the 'help' class as a
-    // marker.
-    if (!hasElementClass(elem, 'help')) {
-        addElementClass(elem, 'help');
-        connect(elem, 'onclick', handleClickOnHelp);
-    }
-}
-
-
-/*
- * Functions for using the help window.
- */
-
-
-// We need to keep track of last element that triggered a help window.
-var last_help_trigger = null;
-
-
-function initHelpPane() {
-    /*
-      Link the popup help pane to its events, set its visibility, etc.
-    */
-    connect('help-close-btn', 'onclick', handleClickOnClose);
-    dismissHelp();
-}
-
-function showHelpFor(trigger) {
-    /*
-      Show the help popup for a particular trigger element.
-    */
-
-    // Assume we are using an <iframe> for the help.
-    // Also assume an <a> tag is the source, and we want to target the
-    // <iframe> at its href.
-
-    // Let our "Loading..." background gif show through.
-    makeInvisible('help-pane-content');
-
-    // Set our 'onload' event handler *outside* of the MochiKit.Signal
-    // framework.  Normally we should not do this, but we need
-    // to work around a bug where the 'onload' signal falls silent
-    // for all events after the first.
-    $('help-pane-content').onload = handleHelpLoaded;
-
-    setNodeAttribute('help-pane-content', 'src', trigger.href);
-
-    var help_pane = $('help-pane');
-
-    /* The help pane is positioned in the center of the screen: */
-    var viewport_dim = getViewportDimensions();
-    var help_pane_dim = elementDimensions('help-pane');
-    var pos_x = Math.round(viewport_dim.w / 2) - (help_pane_dim.w / 2);
-    var pos_y = Math.round(viewport_dim.h / 2) - (help_pane_dim.h / 2);
-    var viewport_pos = getViewportPosition();
-    pos_y += viewport_pos.y;
-    setElementPosition(help_pane, new Coordinates(pos_x, pos_y));
-    makeVisible(help_pane);
-
-    // XXX mars 2008-05-19
-    // Work-around for MochiKit bug #274.  The previous call to
-    // setElementPosition() sets "style=none;" as a side-effect!!!
-    setStyle(help_pane, {'display': ''});
-}
-
-function dismissHelp() {
-    makeInvisible('help-pane');
-}
-
-function handleClickOnHelp(event) {
-    // We don't want <a> tags to navigate.
-    event.stop();
-    var trigger = event.src();
-
-    if (!isVisible('help-pane')) {
-        showHelpFor(trigger);
-    } else if (trigger == last_help_trigger) {
-        // Clicking on the same link that opened a help window closes it
-        // again.
-        dismissHelp();
-    } else {
-        // The user clicked on a different help link, so open it instead.
-        dismissHelp();
-        showHelpFor(trigger);
-    }
-    last_help_trigger = trigger;
-}
-
-function handleClickOnClose(event) {
-    // Prevent the <a> tag from navigating.
-    event.stop();
-    dismissHelp();
-}
-
-function handleHelpLoaded(event) {
-    /*
-      Show the help contents after the help <iframe> has finished
-      loading.
-    */
-    makeVisible('help-pane-content');
-}
-
-function handleClickOnPage(event) {
-    /*
-      Check to see if a click was inside a help window.  If it wasn't,
-      and the window is open, then dismiss it.
-    */
-    var help = $('help-pane');
-    if (isVisible(help) &&
-        !isInside(event.mouse().page, help)) {
-        dismissHelp();
-    }
-}
-
-
-/*
- * Helpers and utility functions.
- */
-
-
-function toggleVisible(elem) {
-    toggleElementClass("invisible", elem);
-}
-
-function makeVisible(elem) {
-    removeElementClass(elem, "invisible");
-}
-
-function makeInvisible(elem) {
-    addElementClass(elem, "invisible");
-}
-
-function isVisible(elem) {
-    // You may also want to check for:
-    // getElement(elem).style.display == "none"
-    return !hasElementClass(elem, "invisible");
-}
-
-function isInside(point, element) {
-    /*
-      Is 'point' inside the supplied 'element'?
-    */
-    return intersect(point,
-                     getElementPosition(element),
-                     getElementDimensions(element));
-}
-
-function intersect(point, dim_point, dimensions) {
-    /*
-      Is 'point' inside the box draw by 'dimensions' at point 'dim_point'?
-    */
-    return ((point.x > dim_point.x) &&
-            (point.x < dim_point.x + dimensions.w) &&
-            (point.y > dim_point.y) &&
-            (point.y < dim_point.y + dimensions.h));
-}