launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07850
lp:~jcsackett/launchpad/use-banner-to-cleanup-beta-notifications into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/use-banner-to-cleanup-beta-notifications into lp:launchpad with lp:~jcsackett/launchpad/use-banner-to-cleanup-privacy as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/use-banner-to-cleanup-beta-notifications/+merge/105732
Summary
=======
This branch uses the new banner code to update the beta banner.
Implementation
==============
A new object, BetaBanner, extends lp.app.banner.Banner.
It sets up a close button unlike the other versions, which fires the widget's
hide event.
Because the banner should only be displayed if there are beta features, a new
wrapper is created that can be called in base-layout.
Tests and callsites are updated.
Tests
=====
bin/test -vvct beta --layer=YUI
QA
==
Find a beta feature; the banner should be displayed appropriately.
Hit the close button, the banner should go away.
LoC
===
This branch removes code.
Lint
====
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/javascript/banners/beta-notification.js
lib/lp/app/javascript/banners/tests/test_banner.js
lib/lp/app/javascript/banners/tests/test_beta_notification.js
lib/lp/app/javascript/banners/banner.js
lib/lp/bugs/templates/bugtarget-macros-filebug.pt
lib/lp/app/javascript/banners/privacy.js
lib/lp/app/javascript/banners/tests/test_privacy.js
lib/lp/bugs/javascript/tests/test_information_type_choice.js
lib/lp/app/templates/base-layout-macros.pt
lib/lp/app/javascript/banners/tests/test_beta_notification.html
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/bugs/javascript/tests/test_bugtask_delete.html
lib/lp/app/javascript/banners/tests/test_privacy.html
lib/lp/bugs/javascript/tests/test_information_type_choice.html
lib/lp/bugs/javascript/information_type_choice.js
--
https://code.launchpad.net/~jcsackett/launchpad/use-banner-to-cleanup-beta-notifications/+merge/105732
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/use-banner-to-cleanup-beta-notifications into lp:launchpad.
=== modified file 'lib/lp/app/javascript/banners/beta-notification.js'
--- lib/lp/app/javascript/banners/beta-notification.js 2012-05-01 17:01:21 +0000
+++ lib/lp/app/javascript/banners/beta-notification.js 2012-05-14 22:24:20 +0000
@@ -1,81 +1,85 @@
-YUI.add('lp.app.beta_features', function(Y) {
-
-var namespace = Y.namespace('lp.app.beta_features');
-
-var beta_notification_node = null;
-
-/**
- * For unit tests - we need to reset the notification setup to run more than
- one test.
- */
-namespace._reset_beta_notification = function () {
- notification_node = null;
-};
-
-/*
- * Display beta feature notifications.
- *
- * This should be called after the page has loaded e.g. on 'domready'.
- */
-function display_beta_notification() {
- var notifications = Y.lp.mustache.to_html([
- '{{#features}}{{#is_beta}}',
- '<span class="beta-feature"> {{title}}',
- '{{#url}}',
- ' (<a href="{{url}}" class="info-link">read more</a>)',
- '{{/url}}',
- '</span>',
- '{{/is_beta}}{{/features}}'].join(''),
- {features: Y.Object.values(LP.cache.related_features)});
- if (notifications.length === 0) {
- return;
- }
- var body = Y.one('body');
- body.addClass('global-notification-visible');
- var main = Y.one('#maincontent');
- beta_notification_node = Y.Node.create('<div></div>')
- .addClass('beta-banner');
- main.appendChild(beta_notification_node);
- var beta_warning = Y.Node.create(
- '<span class="beta-warning">BETA!</span>');
- beta_notification_node.appendChild(beta_warning);
- var close_box = Y.Node.create(
- '<a href="#" class="global-notification-close">Hide' +
- '<span class="notification-close sprite" /></a>');
- beta_notification_node.appendChild(close_box);
- beta_notification_node.append('Some parts of this page are in beta: ');
- beta_notification_node.append(notifications);
- close_box.on('click', function(e) {
- e.halt();
- var fade_out = new Y.Anim({
- node: '.beta-banner',
- 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();
- });
+YUI.add('lp.app.banner.beta', function(Y) {
+
+var ns = Y.namespace('lp.app.banner.beta');
+var baseBanner = Y.lp.app.banner.Banner;
+
+ns.BetaBanner = Y.Base.create('betaBanner', baseBanner, [], {
+
+ _createBanner: function () {
+ var banner_data = {
+ badge: this.get('banner_icon'),
+ text: this.get('notification_text'),
+ features: this.get('features'),
+ banner_id: this.get('banner_id')
+ };
+ return Y.lp.mustache.to_html(
+ this.get('banner_template'),
+ banner_data);
+ },
+
+ renderUI: function () {
+ baseBanner.prototype.renderUI.apply(this, arguments);
+ var banner_node = Y.one('.beta-banner');
+ var close_box = Y.Node.create(
+ '<a href="#" class="global-notification-close">Hide' +
+ '<span class="notification-close sprite" /></a>');
+ banner_node.appendChild(close_box);
+ },
+
+ bindUI: function () {
+ baseBanner.prototype.bindUI.apply(this, arguments);
+ var close_box = Y.one('.global-notification-close');
+ var that = this;
+ close_box.on('click', function(e) {
+ e.halt();
+ that.hide();
+ })
+ },
+
+}, {
+ ATTRS: {
+ banner_id: { value: "beta-banner" },
+ notification_text: { value: "Some parts of this page are in beta: " },
+ banner_icon: { value: '<span class="beta-warning">BETA!</span>' },
+ banner_template: {
+ valueFn: function() {
+ return [
+ '<div id="{{ banner_id }}"',
+ 'class="global-notification beta-banner ',
+ 'transparent hidden">',
+ '{{{ badge }}}',
+ '<span class="banner-text">',
+ '{{ text }}{{{ features }}}',
+ '</span>',
+ "</div>"].join('')
+ }
+ },
+ features: {
+ valueFn: function () {
+ var features_template = [
+ '{{#features}}{{#is_beta}}',
+ '<span class="beta-feature"> {{title}}',
+ '{{#url}}',
+ ' (<a href="{{url}}" class="info-link">read more</a>)',
+ '{{/url}}',
+ '</span>',
+ '{{/is_beta}}{{/features}}'].join('');
+ var feature_data = {
+ features: Y.Object.values(LP.cache.related_features)
+ }
+ return Y.lp.mustache.to_html(features_template, feature_data);
+ }
+ },
+ }
+});
+
+ns.show_beta_if_needed = function () {
+ var banner = new ns.BetaBanner();
+ if (banner.get('features').length !== 0) {
+ banner.render();
+ banner.show();
+ }
}
-namespace.display_beta_notification = display_beta_notification;
-}, '0.1', {'requires': ['base', 'node', 'anim', 'lp.mustache']});
+}, '0.1', {'requires': ['base', 'node', 'anim', 'lp.mustache',
+ 'lp.app.banner']});
=== modified file 'lib/lp/app/javascript/banners/tests/test_beta_notification.html'
--- lib/lp/app/javascript/banners/tests/test_beta_notification.html 2012-05-01 17:01:21 +0000
+++ lib/lp/app/javascript/banners/tests/test_beta_notification.html 2012-05-14 22:24:20 +0000
@@ -26,6 +26,8 @@
<!-- Dependencies -->
<script type="text/javascript"
+ src="../banner.js"></script>
+ <script type="text/javascript"
src="../../../../../../build/js/lp/app/mustache.js"></script>
<!-- The module under test. -->
@@ -41,7 +43,7 @@
<body class="yui3-skin-sam">
<ul id="suites">
<!-- <li>lp.large_indicator.test</li> -->
- <li>lp.app.beta_notification.test</li>
+ <li>lp.app.banner.beta.test</li>
</ul>
<!-- The example markup required by the script to run. -->
<div id="maincontent"></div>
=== modified file 'lib/lp/app/javascript/banners/tests/test_beta_notification.js'
--- lib/lp/app/javascript/banners/tests/test_beta_notification.js 2012-05-01 17:01:21 +0000
+++ lib/lp/app/javascript/banners/tests/test_beta_notification.js 2012-05-14 22:24:20 +0000
@@ -4,42 +4,39 @@
// This must be a global variable for the code being tested to work.
var privacy_notification_enabled = true;
-YUI.add('lp.app.beta_notification.test', function (Y) {
+YUI.add('lp.app.banner.beta.test', function (Y) {
- var tests = Y.namespace('lp.app.beta_notification.test');
- tests.suite = new Y.Test.Suite('beta_notification Tests');
+ var tests = Y.namespace('lp.app.banner.beta.test');
+ tests.suite = new Y.Test.Suite('lp.app.banner.beta Tests');
tests.suite.add(new Y.Test.Case({
name: 'beta-notification',
- _reset_container: function () {
- Y.lp.app.beta_features._reset_beta_notification();
- var body = Y.one(document.body);
-
- // Replace the container.
- var container = Y.one('#maincontent');
- container.remove(true);
- container = Y.Node.create('<div></div>')
- .set('id', 'maincontent');
- body.appendChild(container);
- body.removeClass('global-notification-visible');
- return container;
- },
-
setUp: function () {
- // Create the global notification html.
- var container = this._reset_container();
+ var main = Y.one('#maincontent');
var login_logout = Y.Node.create('<div></div>')
.addClass('login-logout');
- container.appendChild(login_logout);
+ main.appendChild(login_logout);
window.LP = {
cache: {}
};
},
+ 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.beta_features,
- "Could not locate the lp.app.beta_features module");
+ Y.Assert.isObject(Y.lp.app.banner.beta,
+ "Could not locate the lp.app.banner.beta module");
},
test_beta_banner_one_beta_feature: function() {
@@ -49,20 +46,19 @@
title: 'A beta feature',
url: 'http://lp.dev/LEP/one'
}};
- Y.lp.app.beta_features.display_beta_notification();
+ var betabanner = new Y.lp.app.banner.beta.BetaBanner();
+ betabanner.render();
+ betabanner.show();
var body = Y.one('body');
// The <body> node has the class global-notification-visible,
// so that the element has enough padding for the beta banner.
Y.Assert.isTrue(body.hasClass('global-notification-visible'));
- var banner = Y.one('.beta-banner');
- var sub_nodes = banner.get('children');
-
+ feature_info = Y.one('.beta-feature');
// The message about a beta feature consists of the feature
// title and a link to a page with more information about
// the feature.
- feature_info = sub_nodes.filter('.beta-feature').item(0);
Y.Assert.areEqual(
' A beta feature (read more)', feature_info.get('text'));
info_link = feature_info.get('children').item(0);
@@ -81,16 +77,15 @@
title: 'Beta feature 2',
url: ''
}};
- Y.lp.app.beta_features.display_beta_notification();
+ var betabanner = new Y.lp.app.banner.beta.BetaBanner();
+ betabanner.render();
+ betabanner.show();
var body = Y.one('body');
Y.Assert.isTrue(body.hasClass('global-notification-visible'));
- var banner = Y.one('.beta-banner');
- var sub_nodes = banner.get('children');
-
// Notifications about several features can be displayed.
- feature_info = sub_nodes.filter('.beta-feature').item(0);
+ feature_info = Y.all('.beta-feature').item(0);
Y.Assert.areEqual(
' Beta feature 1 (read more)', feature_info.get('text'));
info_link = feature_info.get('children').item(0);
@@ -98,7 +93,7 @@
// If an entry in LP.cache.related_features does not provide a
// "read more" link, the corrsponding node is not added.
- feature_info = sub_nodes.filter('.beta-feature').item(1);
+ feature_info = Y.all('.beta-feature').item(1);
Y.Assert.areEqual(
' Beta feature 2', feature_info.get('text'));
Y.Assert.isNull(feature_info.get('children').item(0));
@@ -111,11 +106,7 @@
title: 'Non-beta feature',
url: 'http://example.org'
}};
- Y.lp.app.beta_features.display_beta_notification();
-
- var body = Y.one('body');
- Y.Assert.isFalse(body.hasClass('global-notification-visible'));
-
+ Y.lp.app.banner.beta.show_beta_if_needed();
Y.Assert.isNull(Y.one('.beta-banner'));
},
@@ -126,27 +117,26 @@
title: 'A beta feature',
url: 'http://lp.dev/LEP/one'
}};
- Y.lp.app.beta_features.display_beta_notification();
+ var betabanner = new Y.lp.app.banner.beta.BetaBanner();
+ betabanner.render();
+ betabanner.show();
var body = Y.one('body');
var banner = Y.one('.beta-banner');
Y.Assert.isFalse(banner.hasClass('hidden'));
+ // We must wait until the fade out animation finishes.
+ var check = function() {
+ Y.Assert.isTrue(banner.hasClass('hidden'));
+ Y.Assert.isFalse(
+ body.hasClass('global-notification-visible'));
+ }
close_link = Y.one('.global-notification-close');
close_link.simulate('click');
- // We must wait until the fade out animation finishes.
- this.wait(
- function() {
- Y.Assert.isTrue(banner.hasClass('hidden'));
- Y.Assert.isFalse(
- body.hasClass('global-notification-visible'));
- },
- 400);
+ var wait_time = 400;
+ this.wait(check, wait_time);
}
-
-
}));
-}, '0.1', {
- 'requires': ['test', 'console', 'lp.beta_notification', 'node',
- 'lp.app.beta_features', 'node-event-simulate']
+}, '0.1', { 'requires': ['test', 'console', 'node', 'lp.app.banner.beta',
+ 'node-event-simulate']
});
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2012-05-14 22:24:19 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2012-05-14 22:24:20 +0000
@@ -164,7 +164,7 @@
banner = Y.lp.app.banner.privacy.getPrivacyBanner();
banner.show();
}
- Y.lp.app.beta_features.display_beta_notification();
+ Y.lp.app.banner.beta.show_beta_if_needed();
Y.lp.app.sorttable.SortTable.init();
Y.lp.app.inlinehelp.init_help();
Y.lp.activate_collapsibles();
Follow ups
