← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jcsackett/launchpad/banner-class into lp:launchpad

 

j.c.sackett has proposed merging lp:~jcsackett/launchpad/banner-class into lp:launchpad.

Requested reviews:
  Richard Harding (rharding)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/banner-class/+merge/104984

Summary
=======
There are two banners used in LP--privacy and beta. The code between them is
duplicated. As part of work to make sure the privacy banner works in non js
situations, it's best if this code is reconciled into a shared YUI base
object, which is easier to then use in progressive enhancement.

Preimp
======
Spoke with Rick Harding, Curtis Hovey

Implementation
==============
There is a new app submodule, lp.app.banner. Beta and privacy notifications
are part of this module, but are currently otherwise unchanged. The code
common to both of them has been redone in a YUI object created from
Y.Base.create.

A follow up branch exists to convert privacy and beta-notification into
extensions of Banner. For review purposes, I thought it best to put this up
separately.

Tests
=====
bin/test -vvct test_banner --layer=YUI

QA
==
No QA, this isn't wired in anywhere yet.

LoC
===
The overall arc of this branch will remove quite a bit of code by killing
duplications. Additionally, it's part of the disclosure project, already
resourced.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/banners/tests/test_banner.js
  lib/lp/app/javascript/banners/banner.js
  lib/lp/app/javascript/banners/tests/test_beta_notification.html
  lib/lp/app/javascript/banners/tests/test_banner.html
  lib/lp/app/javascript/banners/tests/test_privacy.html
-- 
https://code.launchpad.net/~jcsackett/launchpad/banner-class/+merge/104984
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== added directory 'lib/lp/app/javascript/banners'
=== added file 'lib/lp/app/javascript/banners/banner.js'
--- lib/lp/app/javascript/banners/banner.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/banners/banner.js	2012-05-07 22:05:21 +0000
@@ -0,0 +1,137 @@
+/* Copyright 2012 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Notification banner widget
+ *
+ * @module lp.app.banner
+ */
+
+YUI.add('lp.app.banner', function (Y) {
+
+var ns = Y.namespace('lp.app.banner');
+
+ns.Banner = Y.Base.create('banner', Y.Widget, [], {
+    initializer: function (cfg) {
+        base_cfg = {
+            notification_text: this.get('notification_text'),
+            banner_icon: this.get('banner_icon'),
+            hidden: true
+        }
+        cfg = Y.merge(base_cfg, cfg);
+        this.set('notification_text', cfg.notification_text);
+        this.set('banner_icon', cfg.banner_icon);
+        this.set('hidden', cfg.hidden);
+    },
+
+    renderUI: function() {
+        var banner_data = {
+                hidden: this.get('hidden'),
+                badge: this.get('banner_icon'),
+                text: this.get('notification_text')
+        };
+        var banner_html = Y.lp.mustache.to_html(
+            this.get('banner_template'),
+            banner_data);
+        this.get('target').append(banner_html);
+        if (!this.get('hidden')) {
+            var body = Y.one('body');
+            body.addClass('global-notification-visible');
+        }
+    },
+
+    show: function () {
+        var body = Y.one('body');
+        body.addClass('global-notification-visible');
+
+        var global_notification = Y.one('.global-notification');
+        global_notification.removeClass('hidden');
+
+        var fade_in = new Y.Anim({
+            node: '.global-notification',
+            to: {opacity: 1},
+            duration: 0.3
+        });
+        var body_space = new Y.Anim({
+            node: 'body',
+            to: {'paddingTop': '40px'},
+            duration: 0.2,
+            easing: Y.Easing.easeOut
+        });
+        var login_space = new Y.Anim({
+            node: '.login-logout',
+            to: {'top': '45px'},
+            duration: 0.2,
+            easing: Y.Easing.easeOut
+        });
+
+        fade_in.on('start', function() {
+            fade_in.get('node').removeClass('hidden'); 
+        });
+        body_space.on('start', function() {
+            Y.one('body').addClass('global-notification-visible');
+        });
+
+        fade_in.run();
+        body_space.run();
+        login_space.run();
+
+        this.fire('banner-shown');
+    },
+
+    hide: function () {
+        var fade_out = new Y.Anim({
+            node: '.global-notification',
+            to: {opacity: 0},
+            duration: 0.3
+        });
+        var body_space = new Y.Anim({
+            node: 'body',
+            to: {'paddingTop': 0},
+            duration: 0.2,
+            easing: Y.Easing.easeOut
+        });
+        var login_space = new Y.Anim({
+            node: '.login-logout',
+            to: {'top': '6px'},
+            duration: 0.2,
+            easing: Y.Easing.easeOut
+        });
+        fade_out.on('end', function() {
+            fade_out.get('node').addClass('hidden');
+        });
+        body_space.on('end', function() {
+            Y.one('body').removeClass('global-notification-visible');
+        });
+
+        fade_out.run();
+        body_space.run();
+        login_space.run();
+
+        this.fire('banner-hidden');
+    }
+}, {
+    ATTRS: {
+        notification_text: { value: "" },
+        banner_icon: { value: "<span></span>" },
+        target: {
+            valueFn: function() {
+                return Y.one('#maincontent');
+            }
+        },
+        banner_template: {
+            valueFn: function() {
+                return [
+                    '<div class="global-notification transparent', 
+                        '{{#hidden}}',
+                            ' hidden',
+                        '{{/hidden}}">',
+                            '{{{ badge }}}',
+                            '{{ text }}',
+                    "</div>"].join('')
+            } 
+        },
+        hidden: { value: true },
+    },
+});
+
+}, '0.1', {'requires': ['base', 'node', 'anim', 'lp.mustache']});

=== renamed file 'lib/lp/app/javascript/beta-notification.js' => 'lib/lp/app/javascript/banners/beta-notification.js'
=== renamed file 'lib/lp/app/javascript/privacy.js' => 'lib/lp/app/javascript/banners/privacy.js'
=== added directory 'lib/lp/app/javascript/banners/tests'
=== added file 'lib/lp/app/javascript/banners/tests/test_banner.html'
--- lib/lp/app/javascript/banners/tests/test_banner.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/banners/tests/test_banner.html	2012-05-07 22:05:21 +0000
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<!--
+Copyright 2012 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+  <head>
+      <title>lp.app.banner Tests</title>
+
+      <!-- YUI and test setup -->
+      <script type="text/javascript"
+              src="../../../../../../build/js/yui/yui/yui.js">
+      </script>
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/console/assets/console-core.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+      <script type="text/javascript"
+              src="../../../../../../build/js/lp/app/testing/testrunner.js"></script>
+
+      <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
+
+      <!-- Dependencies -->
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/mustache.js"></script>
+
+      <!-- The module under test. -->
+      <script type="text/javascript" src="../banner.js"></script>
+
+      <!-- Placeholder for any css asset for this module. -->
+      <!-- <link rel="stylesheet" href="../assets/${LIBRARY}-core.css" /> -->
+
+      <!-- The test suite -->
+      <script type="text/javascript" src="test_banner.js"></script>
+
+    </head>
+    <body class="yui3-skin-sam">
+        <ul id="suites">
+            <li>lp.app.banner.test</li>
+        </ul>
+
+        <!-- The example markup required by the script to run. -->
+        <div id="maincontent"></div>
+    </body>
+</html>

=== added file 'lib/lp/app/javascript/banners/tests/test_banner.js'
--- lib/lp/app/javascript/banners/tests/test_banner.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/banners/tests/test_banner.js	2012-05-07 22:05:21 +0000
@@ -0,0 +1,115 @@
+/* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.app.banner.test', function (Y) {
+
+    var tests = Y.namespace('lp.app.banner.test');
+    tests.suite = new Y.Test.Suite('lp.app.banner Tests');
+
+    tests.suite.add(new Y.Test.Case({
+        name: 'banner_tests',
+
+        setUp: function () {
+            var main = Y.one('#maincontent');
+            var login_logout = Y.Node.create('<div></div>')
+                .addClass('login-logout');
+            main.appendChild(login_logout);
+        },
+
+        tearDown: function () {
+            var body = Y.one(document.body);
+            var main = Y.one('#maincontent');
+            main.remove(true);
+            main = Y.Node.create('<div></div>')
+                .set('id', 'maincontent');
+            body.appendChild(main);
+            var login_logout = Y.Node.create('<div></div>')
+                .addClass('login-logout');
+            main.appendChild(login_logout);
+        },
+
+        test_library_exists: function () {
+            Y.Assert.isObject(Y.lp.app.banner,
+                "Could not locate the lp.app.banner module");
+        },
+
+        test_init_without_config: function () {
+            var banner = new Y.lp.app.banner.Banner(); 
+            Y.Assert.areEqual("", banner.get('notification_text'));
+            Y.Assert.areEqual("<span></span>", banner.get('banner_icon'));
+        },
+
+        test_init_with_config: function () {
+            var cfg = {
+                notification_text: "Some text.",
+                banner_icon: '<span class="sprite"></span>'
+            };
+            var banner = new Y.lp.app.banner.Banner(cfg); 
+            Y.Assert.areEqual(
+                cfg.notification_text,
+                banner.get('notification_text'));
+            Y.Assert.areEqual(cfg.banner_icon, banner.get('banner_icon'));
+        },
+
+        test_render_no_config: function () {
+            var banner = new Y.lp.app.banner.Banner(); 
+            banner.render();
+            var main = Y.one("#maincontent");
+            var banner_node = main.one(".global-notification")
+            Y.Assert.isObject(banner_node);
+            Y.Assert.isTrue(banner_node.hasClass('hidden'));
+        },
+        
+        test_render_with_config: function () {
+            var cfg = {
+                notification_text: "Some text.",
+                banner_icon: '<span class="sprite"></span>',
+                hidden: false
+            };
+            var banner = new Y.lp.app.banner.Banner(cfg); 
+            banner.render();
+            var main = Y.one("#maincontent");
+            var banner_node = main.one(".global-notification");
+            var badge = banner_node.get('children').pop();
+            Y.Assert.isObject(banner_node);
+            Y.Assert.isFalse(banner_node.hasClass('hidden'));
+            Y.Assert.areEqual(cfg.notification_text, banner_node.get('text'));
+            Y.Assert.isTrue(badge.hasClass('sprite'));
+        },
+
+        test_hide: function() {
+            var cfg = { hidden: false };
+            var banner = new Y.lp.app.banner.Banner(cfg); 
+            banner.render();
+            var main = Y.one("#maincontent");
+            var banner_node = main.one(".global-notification");
+            Y.Assert.isFalse(banner_node.hasClass('hidden'));
+
+            var wait_for_hide_animation = 320;
+            var check = function () { 
+                Y.Assert.isTrue(banner_node.hasClass('hidden'));
+            }
+
+            banner.hide();
+            this.wait(check, wait_for_hide_animation);
+        },
+
+        test_show: function() {
+            var cfg = { hidden: true };
+            var banner = new Y.lp.app.banner.Banner(cfg); 
+            banner.render();
+            var main = Y.one("#maincontent");
+            var banner_node = main.one(".global-notification");
+            Y.Assert.isTrue(banner_node.hasClass('hidden'));
+
+            var wait_for_hide_animation = 320;
+            var check = function () { 
+                Y.Assert.isFalse(banner_node.hasClass('hidden'));
+            }
+            banner.show();
+            this.wait(check, wait_for_hide_animation);
+        }
+
+
+    }));
+
+}, '0.1', {'requires': ['test', 'console', 'lp.app.banner']});

=== renamed file 'lib/lp/app/javascript/tests/test_beta_notification.html' => 'lib/lp/app/javascript/banners/tests/test_beta_notification.html'
--- lib/lp/app/javascript/tests/test_beta_notification.html	2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/banners/tests/test_beta_notification.html	2012-05-07 22:05:21 +0000
@@ -10,23 +10,23 @@
 
       <!-- YUI and test setup -->
       <script type="text/javascript"
-              src="../../../../../build/js/yui/yui/yui.js">
+              src="../../../../../../build/js/yui/yui/yui.js">
       </script>
       <link rel="stylesheet"
-      href="../../../../../build/js/yui/console/assets/console-core.css" />
-      <link rel="stylesheet"
-      href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
-      <link rel="stylesheet"
-      href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+      href="../../../../../../build/js/yui/console/assets/console-core.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/test/assets/skins/sam/test.css" />
 
       <script type="text/javascript"
-              src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+              src="../../../../../../build/js/lp/app/testing/testrunner.js"></script>
 
-      <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+      <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
 
       <!-- Dependencies -->
       <script type="text/javascript"
-          src="../../../../../build/js/lp/app/mustache.js"></script>
+          src="../../../../../../build/js/lp/app/mustache.js"></script>
 
       <!-- The module under test. -->
       <script type="text/javascript" src="../beta-notification.js"></script>

=== renamed file 'lib/lp/app/javascript/tests/test_beta_notification.js' => 'lib/lp/app/javascript/banners/tests/test_beta_notification.js'
=== renamed file 'lib/lp/app/javascript/tests/test_privacy.html' => 'lib/lp/app/javascript/banners/tests/test_privacy.html'
--- lib/lp/app/javascript/tests/test_privacy.html	2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/banners/tests/test_privacy.html	2012-05-07 22:05:21 +0000
@@ -10,25 +10,25 @@
 
       <!-- YUI and test setup -->
       <script type="text/javascript"
-              src="../../../../../build/js/yui/yui/yui.js">
+              src="../../../../../../build/js/yui/yui/yui.js">
       </script>
       <link rel="stylesheet"
-      href="../../../../../build/js/yui/console/assets/console-core.css" />
-      <link rel="stylesheet"
-      href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
-      <link rel="stylesheet"
-      href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+      href="../../../../../../build/js/yui/console/assets/console-core.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/test/assets/skins/sam/test.css" />
 
       <script type="text/javascript"
-              src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+              src="../../../../../../build/js/lp/app/testing/testrunner.js"></script>
 
-      <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+      <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
 
       <!-- Dependencies -->
       <script type="text/javascript"
-          src="../../../../../build/js/lp/app/client.js"></script>
+          src="../../../../../../build/js/lp/app/client.js"></script>
       <script type="text/javascript"
-          src="../../../../../build/js/lp/app/lp.js"></script>
+          src="../../../../../../build/js/lp/app/lp.js"></script>
 
       <!-- The module under test. -->
       <script type="text/javascript" src="../privacy.js"></script>

=== renamed file 'lib/lp/app/javascript/tests/test_privacy.js' => 'lib/lp/app/javascript/banners/tests/test_privacy.js'

Follow ups