launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12504
[Merge] lp:~rharding/launchpad/info_type_events into lp:launchpad
Richard Harding has proposed merging lp:~rharding/launchpad/info_type_events into lp:launchpad.
Commit message:
Add events to information_type and privacy banner to drive usage.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rharding/launchpad/info_type_events/+merge/126317
= Summary =
This is the first of a series of branches to update the PrivacyBanner to be
event driven. The end goal is that the information type changes at the widget
level will fire global events that will update the banner appropriately.
== Pre Implementation ==
Talked with JC about how things currently worked.
== Implementation Notes ==
This branch includes some general cleanup and linting. Sorting ATTRS into
alphabetical order, moving functions up above the class definitions.
It also includes one drive by import warning around #636 of the diff.
This adds published events to the information_type module. Currently, these
are not fired in real code, but tests are added that you can fire an event
that the information type has changed, and it'll determine if the new value is
public or private and fire a secondary event with that info.
It also adds logic and events to the PrivacyBanner. It watches for changes to
the information type so that it can know if it should show/hide. PrivacyBanner
is also used for security notification so it needs manual events so that when
something is a security issue, it can be told to show, but with a custom
message and nothing to do with information type itself.
== Tests ==
test_information_type.html
test_privacy.html
--
https://code.launchpad.net/~rharding/launchpad/info_type_events/+merge/126317
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/info_type_events into lp:launchpad.
=== modified file 'lib/lp/app/javascript/banners/banner.js'
--- lib/lp/app/javascript/banners/banner.js 2012-05-25 14:38:42 +0000
+++ lib/lp/app/javascript/banners/banner.js 2012-09-26 13:33:25 +0000
@@ -1,4 +1,5 @@
-/* Copyright 2012 Canonical Ltd. This software is licensed under the
+/*
+ * Copyright 2012 Canonical Ltd. This software is licensed under the
* GNU Affero General Public License version 3 (see the file LICENSE).
*
* Notification banner widget
@@ -29,10 +30,11 @@
_showBanner: function () {
var body = Y.one('body');
+ var global_notification = Y.one('.global-notification');
+ var anim_times = this._getAnimTimes();
+
body.addClass('global-notification-visible');
- var global_notification = Y.one('.global-notification');
global_notification.removeClass('hidden');
- var anim_times = this._getAnimTimes();
var fade_in = new Y.Anim({
node: global_notification,
@@ -59,10 +61,10 @@
_hideBanner: function () {
var body = Y.one('body');
var global_notification = Y.one('.global-notification');
+ var anim_times = this._getAnimTimes();
+
global_notification.addClass('transparent');
- var anim_times = this._getAnimTimes();
-
var fade_out = new Y.Anim({
node: global_notification,
to: {opacity: 0},
@@ -92,38 +94,37 @@
login_space.run();
},
+ bindUI: function() {
+ this.after('visibleChange', function() {
+ if (this.get('visible')) {
+ this._showBanner();
+ } else {
+ this._hideBanner();
+ }
+ });
+ },
+
renderUI: function () {
var banner_data = {
badge: this.get('banner_icon'),
text: this.get('notification_text')
};
- var banner_html = Y.lp.mustache.to_html(
+ var banner_html = Y.lp.mustache.to_html(
this.get('banner_template'),
banner_data);
- this.get('contentBox').append(banner_html);
- },
-
- bindUI: function() {
- this.after('visibleChange', function() {
- if (this.get('visible')) {
- this._showBanner();
- } else {
- this._hideBanner();
- }
- });
+ this.get('contentBox').append(banner_html);
},
updateText: function (new_text) {
+ var text_node = this.get('contentBox').one('.banner-text');
if (!Y.Lang.isValue(new_text)) {
new_text = this.get('notification_text');
}
- var text_node = this.get('contentBox').one('.banner-text');
text_node.set('text', new_text);
}
}, {
ATTRS: {
- notification_text: { value: "" },
banner_icon: { value: "<span></span>" },
banner_template: {
valueFn: function() {
@@ -134,9 +135,12 @@
"</div>"].join('');
}
},
+ notification_text: { value: "" },
skip_animation: { value: false },
visible: { value: false }
}
});
-}, '0.1', {'requires': ['base', 'node', 'anim', 'widget', 'lp.mustache']});
+}, '0.1', {
+ requires: ['base', 'node', 'anim', 'widget', 'lp.mustache']
+});
=== modified file 'lib/lp/app/javascript/banners/beta-notification.js'
--- lib/lp/app/javascript/banners/beta-notification.js 2012-05-23 21:28:36 +0000
+++ lib/lp/app/javascript/banners/beta-notification.js 2012-09-26 13:33:25 +0000
@@ -1,9 +1,49 @@
+/**
+ * Add a BetaBanner widget for use.
+ *
+ * @namespace lp.app.banner
+ * @module beta
+ *
+ */
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, [], {
+var Banner = Y.lp.app.banner.Banner;
+
+// For the beta banner to work, it needs to have one instance, and one
+// instance only.
+window._singleton_beta_banner = null;
+
+ns.show_beta_if_needed = function () {
+ if (window._singleton_beta_banner === null) {
+ var src = Y.one('.yui3-betabanner');
+ window._singleton_beta_banner = new ns.BetaBanner({ srcNode: src });
+ }
+ if (window._singleton_beta_banner.get('features').length !== 0) {
+ window._singleton_beta_banner.render();
+ window._singleton_beta_banner.show();
+ }
+};
+
+
+/**
+ * Banner to display for beta features.
+ *
+ * @class BetaBanner
+ * @extends Banner
+ *
+ */
+ns.BetaBanner = Y.Base.create('betaBanner', Banner, [], {
+
+ bindUI: function () {
+ Banner.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();
+ });
+ },
renderUI: function () {
var banner_data = {
@@ -20,35 +60,26 @@
'<a href="#" class="global-notification-close">Hide' +
'<span class="notification-close sprite" /></a>');
beta_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: {
- 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 class="global-notification transparent hidden">',
+ '<div class="global-notification transparent hidden">',
'{{{ badge }}}',
'<span class="banner-text">',
'{{ text }}{{{ features }}}',
'</span>',
- "</div>"].join('')
- }
+ "</div>"].join('');
+ }
},
+
features: {
- valueFn: function () {
+ valueFn: function () {
var features_template = [
'{{#features}}{{#is_beta}}',
'<span class="beta-feature"> {{title}}',
@@ -59,26 +90,15 @@
'{{/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);
}
},
+
+ notification_text: { value: "Some parts of this page are in beta: " }
}
});
-// For the beta banner to work, it needs to have one instance, and one
-// instance only.
-_singleton_beta_banner = null;
-ns.show_beta_if_needed = function () {
- if (_singleton_beta_banner === null) {
- var src = Y.one('.yui3-betabanner')
- _singleton_beta_banner = new ns.BetaBanner({ srcNode: src });
- }
- if (_singleton_beta_banner.get('features').length !== 0) {
- _singleton_beta_banner.render();
- _singleton_beta_banner.show();
- }
-}
}, '0.1', {'requires': ['base', 'node', 'anim', 'lp.mustache',
'lp.app.banner']});
=== modified file 'lib/lp/app/javascript/banners/privacy.js'
--- lib/lp/app/javascript/banners/privacy.js 2012-05-25 14:38:42 +0000
+++ lib/lp/app/javascript/banners/privacy.js 2012-09-26 13:33:25 +0000
@@ -1,33 +1,117 @@
+/**
+ * Add a PrivacyBanner widget for use.
+ *
+ * @namespace lp.app.banner
+ * @module privacy
+ *
+ */
YUI.add('lp.app.banner.privacy', function(Y) {
-
var ns = Y.namespace('lp.app.banner.privacy');
-var baseBanner = Y.lp.app.banner.Banner;
-
-ns.PrivacyBanner = Y.Base.create('privacyBanner', baseBanner, [], {}, {
- ATTRS: {
- notification_text: {
- value: "The information on this page is private."
- },
- banner_icon: {
- value: '<span class="sprite notification-private"></span>'
- }
- }
-});
+var Banner = Y.lp.app.banner.Banner;
+var info_type = Y.lp.app.information_type;
+
+ns.EV_SHOW = 'privacy_banner:show';
+ns.EV_HIDE = 'privacy_banner:hide';
+
+/**
+ * Allow for adjusting the global instance of the Privacy Banner via events.
+ *
+ * @event privacy_banner:show
+ * @param text The message to show
+ */
+Y.publish(ns.EV_SHOW, {
+ emitFacade: true
+});
+
+/**
+ * Hide the global instance of the banner via events.
+ *
+ * @event privacy_banner:hide
+ */
+Y.publish(ns.EV_HIDE, {
+ emitFacade: true
+});
+
// For the privacy banner to work, it needs to have one instance, and one
// instance only.
-var _singleton_privacy_banner = null;
+window._singleton_privacy_banner = null;
ns.getPrivacyBanner = function (banner_text, skip_animation) {
- if (_singleton_privacy_banner === null) {
+ if (window._singleton_privacy_banner === null) {
var src = Y.one('.yui3-privacybanner');
- _singleton_privacy_banner = new ns.PrivacyBanner(
+ window._singleton_privacy_banner = new ns.PrivacyBanner(
{ srcNode: src, skip_animation: skip_animation });
- _singleton_privacy_banner.render();
+ window._singleton_privacy_banner.render();
}
if (Y.Lang.isValue(banner_text)) {
- _singleton_privacy_banner.updateText(banner_text);
+ window._singleton_privacy_banner.updateText(banner_text);
}
- return _singleton_privacy_banner;
+ return window._singleton_privacy_banner;
};
-}, "0.1", {"requires": ["base", "node", "anim", "lp.app.banner"]});
+
+/**
+ * Banner to display when page contains private information.
+ *
+ * @class PrivacyBanner
+ * @extends Banner
+ *
+ */
+ns.PrivacyBanner = Y.Base.create('privacyBanner', Banner, [], {
+ _custom_message: function (ev) {
+ var body = Y.one('body');
+ body.replaceClass('public', 'private');
+ if (!ev.text) {
+ thow('A custom privacy banner must have a text attribute');
+ }
+ this.updateText(ev.text);
+ this.show();
+ },
+
+ _make_public: function (ev) {
+ body.replaceClass('private', 'public');
+ this.hide();
+ },
+
+ _make_private: function (ev) {
+ // Update the text in the banner before we show it.
+ debugger;
+ var banner_text;
+ var body = Y.one('body');
+ body.replaceClass('public', 'private');
+
+ if (ev.text) {
+ banner_text = ev.text;
+ } else {
+ banner_text = info_type.get_banner_text(ev.value);
+ }
+
+ this.updateText(banner_text);
+ this.show();
+ },
+
+ bindUI: function () {
+ that = this;
+ Banner.prototype.bindUI.apply(this, arguments);
+ Y.on(info_type.EV_ISPUBLIC, that._make_public, that);
+ Y.on(info_type.EV_ISPRIVATE, that._make_private, that);
+ Y.on(ns.EV_SHOW, that._custom_message, that);
+ Y.on(ns.EV_HIDE, function (ev) {
+ that.hide();
+ }, that);
+ },
+
+}, {
+ ATTRS: {
+ banner_icon: {
+ value: '<span class="sprite notification-private"></span>'
+ },
+ notification_text: {
+ value: "The information on this page is private."
+ }
+ }
+});
+
+}, "0.1", {
+ requires: ["base", "node", "anim", "lp.app.banner", "lp.app.information_type"]
+});
=== modified file 'lib/lp/app/javascript/banners/tests/test_privacy.html'
--- lib/lp/app/javascript/banners/tests/test_privacy.html 2012-05-15 14:39:36 +0000
+++ lib/lp/app/javascript/banners/tests/test_privacy.html 2012-09-26 13:33:25 +0000
@@ -29,6 +29,8 @@
src="../../../../../../build/js/lp/app/mustache.js"></script>
<script type="text/javascript"
src="../../../../../../build/js/lp/app/banners/banner.js"></script>
+ <script type="text/javascript"
+ src="../../../../../../build/js/lp/app/information_type.js"></script>
<!-- The module under test. -->
<script type="text/javascript" src="../privacy.js"></script>
@@ -41,8 +43,5 @@
<ul id="suites">
<li>lp.app.banner.privacy.test</li>
</ul>
-
- <!-- The example markup required by the script to run. -->
- <div id="maincontent"></div>
</body>
</html>
=== modified file 'lib/lp/app/javascript/banners/tests/test_privacy.js'
--- lib/lp/app/javascript/banners/tests/test_privacy.js 2012-06-25 17:39:07 +0000
+++ lib/lp/app/javascript/banners/tests/test_privacy.js 2012-09-26 13:33:25 +0000
@@ -15,19 +15,22 @@
var login_logout = Y.Node.create('<div></div>')
.addClass('login-logout');
main.appendChild(login_logout);
+
+ var banner_node = Y.Node.create('<div></div>')
+ .addClass('yui3-privacybanner');
+ main.appendChild(banner_node);
Y.one('body').appendChild(main);
},
tearDown: function () {
Y.one('#maincontent').remove(true);
- Y.all('.yui3-banner').remove(true);
+ window._singleton_privacy_banner = null;
},
test_library_exists: function () {
Y.Assert.isObject(Y.lp.app.banner.privacy,
"Could not locate the lp.app.banner.privacy module");
- },
-
+ },
test_init: function () {
var banner = new Y.lp.app.banner.privacy.PrivacyBanner();
Y.Assert.areEqual(
@@ -50,6 +53,32 @@
Y.Assert.areEqual(
new_text,
Y.one('.global-notification').get('text'));
+ },
+
+ test_banner_with_custom_text: function () {
+ var banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+ var new_text = 'New custom text';
+
+ Y.fire('privacy_banner:show', {
+ text: new_text
+ });
+ Y.Assert.areEqual(
+ new_text,
+ Y.one('.global-notification').get('text'));
+ Y.Assert.isTrue(banner.get('visible'),
+ 'Banner should be visible.');
+ },
+
+ test_banner_hide_event: function () {
+ var banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+ var new_text = 'New custom text';
+
+ Y.fire('privacy_banner:show', {
+ text: new_text
+ });
+ Y.fire('privacy_banner:hide');
+ Y.Assert.isFalse(banner.get('visible'),
+ 'Banner should not be visible.');
}
}));
=== modified file 'lib/lp/app/javascript/information_type.js'
--- lib/lp/app/javascript/information_type.js 2012-09-21 15:39:22 +0000
+++ lib/lp/app/javascript/information_type.js 2012-09-26 13:33:25 +0000
@@ -7,6 +7,69 @@
YUI.add('lp.app.information_type', function(Y) {
var ns = Y.namespace('lp.app.information_type');
+ns.EV_CHANGE = 'information_type:change';
+ns.EV_ISPUBLIC = 'information_type:is_public';
+ns.EV_ISPRIVATE = 'information_type:is_private';
+
+/**
+ * Any time the information type changes during a page we need an event we can
+ * watch out for that.
+ *
+ * @event information_type:change
+ * @param value The new information type value.
+ */
+Y.publish(ns.EV_CHANGE, {
+ emitFacade: true
+});
+
+/*
+ * We also want shortcuts we can use to fire specific events if the new
+ * information type is public or private (generally speaking).
+ *
+ * @event information_type:is_public
+ * @param value The new information type value.
+ */
+Y.publish(ns.EV_ISPUBLIC, {
+ emitFacade: true
+});
+
+/**
+ * The information type has been changed to a private type.
+ *
+ * @event information_type:is_private
+ * @param value The new information type value.
+ */
+Y.publish(ns.EV_ISPRIVATE, {
+ emitFacade: true
+});
+
+/**
+ * Wire up a helper event so that if someone changes the event, we take care
+ * of also firing any is_private/is_public event shortcuts others want to
+ * listen on.
+ *
+ * This enables us to keep the looking up of information type to ourselves
+ * instead of having each module needing to know the location in the LP.cache
+ * and such.
+ */
+Y.on(ns.EV_CHANGE, function (ev) {
+ if (!ev.value) {
+ throw('Information type change event without new value');
+ }
+
+ if (ns.get_cache_data_from_key(ev.value,
+ 'value',
+ 'is_private')) {
+ Y.fire(ns.EV_ISPRIVATE, {
+ value: ev.value
+ });
+ } else {
+ Y.fire(ns.EV_ISPUBLIC, {
+ value: ev.value
+ });
+ }
+});
+
// For testing.
var skip_animation = false;
=== modified file 'lib/lp/app/javascript/tests/test_information_type.js'
--- lib/lp/app/javascript/tests/test_information_type.js 2012-09-21 15:39:22 +0000
+++ lib/lp/app/javascript/tests/test_information_type.js 2012-09-26 13:33:25 +0000
@@ -9,6 +9,12 @@
tests.suite.add(new Y.Test.Case({
name: 'lp.app.information_type_tests',
+ _should: {
+ error: {
+ test_change_event_empty: true
+ }
+ },
+
setUp: function() {
window.LP = {
cache: {
@@ -315,6 +321,75 @@
Y.Assert.isTrue(summary.hasClass('public'));
// The error was displayed.
Y.Assert.isNotNull(Y.one('.yui3-lazr-formoverlay-errors'));
+ },
+
+ test_change_event_empty: function () {
+ // When firing a change event you must supply a value.
+ Y.fire('information_type:change');
+ },
+
+ test_change_event_fires: function () {
+ var called = false;
+ var change_ev = Y.on('information_type:change', function (ev) {
+ Y.Assert.areEqual('PUBLIC', ev.value);
+ called = true;
+ });
+
+ Y.fire('information_type:change', {
+ value: 'PUBLIC'
+ });
+
+ Y.Assert.isTrue(called, 'Did get a called event');
+
+ // clean up our event since it's global to our Y instance.
+ change_ev.detach();
+ },
+
+ test_change_public_event: function () {
+ // If the value is a public value then the is_public event fires.
+ var called = false;
+ var public_ev = Y.on('information_type:is_public', function (ev) {
+ Y.Assert.areEqual('PUBLIC', ev.value);
+ called = true;
+ });
+ // However is should not fire an is_private event.
+ var private_ev = Y.on('information_type:is_private', function (ev) {
+ called = false;
+ });
+
+ Y.fire('information_type:change', {
+ value: 'PUBLIC'
+ });
+
+ Y.Assert.isTrue(called, 'Did get a called event');
+
+ // Clean up our event since it's global to our Y instance.
+ public_ev.detach();
+ private_ev.detach();
+ },
+
+ test_change_private_event: function () {
+ // If the value is a private value then the is_private event fires.
+ var called = false;
+
+ // However is should not fire an is_private event.
+ var public_ev = Y.on('information_type:is_public', function (ev) {
+ called = false;
+ });
+ var private_ev = Y.on('information_type:is_private', function (ev) {
+ Y.Assert.areEqual('PROPRIETARY', ev.value);
+ called = true;
+ });
+
+ Y.fire('information_type:change', {
+ value: 'PROPRIETARY'
+ });
+
+ Y.Assert.isTrue(called, 'Did get a called event');
+
+ // Clean up our event since it's global to our Y instance.
+ public_ev.detach();
+ private_ev.detach();
}
}));
=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py 2012-09-25 18:57:52 +0000
+++ lib/lp/blueprints/browser/specification.py 2012-09-26 13:33:25 +0000
@@ -123,10 +123,8 @@
from lp.blueprints.interfaces.sprintspecification import ISprintSpecification
from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
from lp.registry.interfaces.distribution import IDistribution
-from lp.registry.interfaces.product import (
- IProduct,
- IProductSeries,
- )
+from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.productseries import IProductSeries
from lp.services.config import config
from lp.services.features import getFeatureFlag
from lp.services.fields import WorkItemsText
=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js 2012-09-21 15:39:22 +0000
+++ lib/lp/bugs/javascript/filebug.js 2012-09-26 13:33:25 +0000
@@ -97,8 +97,8 @@
Y.lp.app.choice.addPopupChoice(
'importance', LP.cache.bugtask_importance_data);
var cache = LP.cache.information_type_data;
- var information_helpers = Y.lp.app.information_type
- var choices = information_helpers.cache_to_choicesource(cache)
+ var information_helpers = Y.lp.app.information_type;
+ var choices = information_helpers.cache_to_choicesource(cache);
Y.lp.app.choice.addPopupChoiceForRadioButtons(
'information_type', choices, true);
Follow ups
