launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04435
[Merge] lp:~jcsackett/launchpad/reusable-notification-setup into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/reusable-notification-setup into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/reusable-notification-setup/+merge/69867
Summary
=======
The new privacy ribbon was initially setup with some extensive HTMl added to any template which used the ribbon.
To easily extend it to any private context on LP, we need something that's a bit less repetitive. A javascript function we can just add in on domready, for example.
Pre-implementation notes
========================
None, this is continuation of established work discussed with Curtis Hovey.
Implementation details
======================
lib/lp/app/javascript/privacy.js
--------------------------------
A new function that does the setup for the ribbon was created in the privacy javascript file.
lib/lp/app/javascript/tests/test_privacy.js
lib/lp/app/javascript/tests/test_privacy.html
---------------------------------------------
Added a test for setup.
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/bugs/templates/bugcomment-index.pt
-----------------------------------------
Removed the existing template code, and added a call to the setup function.
Tests
=====
firefox lib/lp/app/javascript/tests/test_privacy.html
QA
==
Make sure both the bugcomment and bugtask index pages show the privacy ribbon properly.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/templates/bugcomment-index.pt
lib/lp/bugs/browser/bugcomment.py
lib/lp/app/javascript/privacy.js
lib/lp/app/javascript/tests/test_privacy.js
lib/lp/app/templates/base-layout-macros.pt
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/app/javascript/tests/test_privacy.html
lib/lp/bugs/templates/bugtask-index.pt
--
https://code.launchpad.net/~jcsackett/launchpad/reusable-notification-setup/+merge/69867
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/reusable-notification-setup into lp:launchpad.
=== modified file 'lib/lp/app/javascript/privacy.js'
--- lib/lp/app/javascript/privacy.js 2011-07-28 19:28:08 +0000
+++ lib/lp/app/javascript/privacy.js 2011-07-29 22:34:32 +0000
@@ -6,6 +6,50 @@
*
* This should be called after the page has loaded e.g. on 'domready'.
*/
+
+function setup_privacy_notification(config) {
+ var notification_text = 'The information on this page is private';
+ var hidden = true;
+ var target_id = "maincontent";
+ if (config !== undefined) {
+ if (config.notification_text !== undefined) {
+ notification_text = config.notification_text;
+ }
+ if (config.hidden !== undefined) {
+ hidden = config.hidden;
+ }
+ if (config.target_id !== undefined) {
+ target_id = config.target_id;
+ }
+ }
+ var id_selector = "#" + target_id;
+ var main = Y.one(id_selector);
+ var notification = Y.Node.create('<div></div>')
+ .addClass('global-notification');
+ if (hidden) {
+ notification.addClass('hidden');
+ }
+ var notification_span = Y.Node.create('<span></span>')
+ .addClass('sprite')
+ .addClass('notification-private');
+ var close_link = Y.Node.create('<a></a>')
+ .addClass('global-notification-close')
+ .set('href', '#');
+ var close_span = Y.Node.create('<span></span>')
+ .addClass('sprite')
+ .addClass('notification-close');
+
+ notification.set('text', notification_text);
+ close_link.set('text', "Hide");
+
+ main.appendChild(notification);
+ notification.appendChild(notification_span);
+ notification.appendChild(close_link);
+ close_link.appendChild(close_span);
+
+}
+namespace.setup_privacy_notification = setup_privacy_notification;
+
function display_privacy_notification(highlight_portlet_on_close) {
/* Check if the feature flag is set for this notification. */
var highlight = true;
@@ -69,7 +113,7 @@
var fade_out = new Y.Anim({
node: '.global-notification',
to: {opacity: 0},
- duration: 0.3
+ duration: 0.3
});
var body_space = new Y.Anim({
node: 'body',
=== modified file 'lib/lp/app/javascript/tests/test_privacy.js'
--- lib/lp/app/javascript/tests/test_privacy.js 2011-07-29 14:32:29 +0000
+++ lib/lp/app/javascript/tests/test_privacy.js 2011-07-29 22:34:32 +0000
@@ -14,17 +14,21 @@
suite.add(new Y.Test.Case({
name: 'privacy',
- setUp: function () {
- //create the global notification html
+ _reset_container: function () {
var body = Y.one(document.body);
-
+
//replace the container
var container = Y.one('#notification-area');
container.remove(true);
container = Y.Node.create('<div></div>')
.set('id', 'notification-area');
body.appendChild(container);
+ return container;
+ },
+ setUp: function () {
+ //create the global notification html
+ var container = this._reset_container();
var login_logout = Y.Node.create('<div></div>')
.addClass('login-logout');
var notification = Y.Node.create('<div></div>')
@@ -32,14 +36,14 @@
.addClass('hidden');
var notification_span = Y.Node.create('<span></span>')
.addClass('sprite')
- .addClass('notification-private')
+ .addClass('notification-private');
var close_link = Y.Node.create('<a></a>')
.addClass('global-notification-close')
.set('href', '#');
var close_span = Y.Node.create('<span></span>')
.addClass('sprite')
- .addClass('notification-close')
-
+ .addClass('notification-close');
+
notification.set('text', "This bug is private");
close_link.set('text', "Hide");
@@ -50,10 +54,27 @@
close_link.appendChild(close_span);
},
+ test_setup: function() {
+ //undo the setup
+ var container = this._reset_container();
+ var config = {
+ notification_text: "stuff is private",
+ hidden: true,
+ target_id: container.get('id')
+ };
+ Y.lp.app.privacy.setup_privacy_notification(config);
+ var ribbon = Y.one('.global-notification');
+ Y.Assert.isTrue(ribbon.hasClass('hidden'));
+ //
+ // Text is both the passed in config and the "Hide" button text.
+ var expected_text = 'stuff is private' + 'Hide';
+ Y.Assert.areEqual(expected_text, ribbon.get('text'));
+ },
+
test_display_shows_ribbon: function () {
- var ribbon = Y.one('.global-notification');
+ var ribbon = Y.one('.global-notification');
Y.Assert.isTrue(ribbon.hasClass('hidden'));
- Y.lp.app.privacy.display_privacy_notification()
+ Y.lp.app.privacy.display_privacy_notification();
Y.Assert.isFalse(ribbon.hasClass('hidden.'));
},
@@ -62,11 +83,11 @@
Y.lp.app.privacy.display_privacy_notification();
Y.Assert.isFalse(ribbon.hasClass('hidden'));
Y.lp.app.privacy.hide_privacy_notification(false);
-
+
// We wait for the privacy ribbon fadeout to complete.
// It takes 300ms, but we have to pad that out to avoid a race and
// intermittent failures.
- var wait_for_anim = 320;
+ var wait_for_anim = 320;
this.wait(
function () {
Y.Assert.isTrue(ribbon.hasClass('hidden'));
=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py 2011-07-29 14:51:06 +0000
+++ lib/lp/bugs/browser/bugcomment.py 2011-07-29 22:34:32 +0000
@@ -299,20 +299,20 @@
# Don't cache for long if we are waiting for synchronization.
elif self.bugwatch and not self.synchronized:
- return 5*60
+ return 5 * 60
# For the rest of the first day, the rendering changes every
# hour. '4 hours ago'. Expire in 15 minutes so the timestamp
# is at most 15 minutes out of date.
elif self.datecreated > now - timedelta(days=1):
- return 15*60
+ return 15 * 60
# Otherwise, cache away. Lets cache for 6 hours. We don't want
# to cache for too long as there are still things that can
# become stale - eg. if a bug attachment has been deleted we
# should stop rendering the link.
else:
- return 6*60*60
+ return 6 * 60 * 60
class BugCommentView(LaunchpadView):
@@ -328,7 +328,7 @@
@property
def show_spam_controls(self):
return self.comment.show_spam_controls
-
+
def page_title(self):
return 'Comment %d for bug %d' % (
self.comment.index, self.context.bug.id)
@@ -347,12 +347,12 @@
@property
def show_spam_controls(self):
if hasattr(self.context, 'show_spam_controls'):
- return self.context.show_spam_controls
+ return self.context.show_spam_controls
elif (hasattr(self, 'comment') and
- hasattr(self.comment, 'show_spam_controls')):
- return self.comment.show_spam_controls
+ hasattr(self.comment, 'show_spam_controls')):
+ return self.comment.show_spam_controls
else:
- return False
+ return False
def proxiedUrlOfLibraryFileAlias(self, attachment):
"""Return the proxied URL for the Librarian file of the attachment."""
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-07-29 14:32:45 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-07-29 22:34:32 +0000
@@ -41,16 +41,6 @@
namespace.setup_bugtask_index = function() {
/*
- * Display the privacy notification if the bug is private
- */
-
- Y.on("domready", function () {
- if (Y.one(document.body).hasClass('private')) {
- Y.lp.app.privacy.display_privacy_notification();
- }
- });
-
- /*
* Check the page for links related to overlay forms and request the HTML
* for these forms.
*/
=== modified file 'lib/lp/bugs/templates/bugcomment-index.pt'
--- lib/lp/bugs/templates/bugcomment-index.pt 2011-07-25 20:48:59 +0000
+++ lib/lp/bugs/templates/bugcomment-index.pt 2011-07-29 22:34:32 +0000
@@ -8,27 +8,24 @@
>
<body>
<metal:block fill-slot="head_epilogue">
+ <tal:if condition="request/features/bugs.private_notification.enabled">
<script>
LPS.use('base', 'node', 'lp.app.privacy', function(Y) {
Y.on("domready", function () {
if (Y.one(document.body).hasClass('private')) {
+ var config = {
+ notification_text: 'This comment is on a private bug',
+ hidden: true
+ };
+ Y.lp.app.privacy.setup_privacy_notification(config);
Y.lp.app.privacy.display_privacy_notification(false);
}
});
});
</script>
+ </tal:if>
</metal:block>
<div metal:fill-slot="main" tal:define="comment view/comment">
- <tal:if condition="request/features/bugs.private_notification.enabled">
- <div tal:attributes="class string: global-notification ${view/privacy_notice_classes}">
- <span class="sprite notification-private"></span>
- This comment is on a private bug
- <a href="#" class="global-notification-close">
- <span class="sprite notification-close"></span>
- Hide
- </a>
- </div>
- </tal:if>
<h1 tal:content="view/page_title">Foo doesn't work</h1>
<tal:comment replace="structure comment/@@+box-expanded-reply">
</tal:comment>
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2011-07-27 13:54:24 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2011-07-29 22:34:32 +0000
@@ -34,6 +34,28 @@
});
});
</script>
+ <tal:if condition="request/features/bugs.private_notification.enabled">
+ <script type="text/javascript">
+ LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bugtask_index',
+ 'lp.bugs.subscribers',
+ 'lp.code.branchmergeproposal.diff', 'lp.comments.hide',
+ function(Y) {
+ /*
+ * Display the privacy notification if the bug is private
+ */
+ Y.on("domready", function () {
+ if (Y.one(document.body).hasClass('private')) {
+ var config = {
+ notification_text: 'This bug is private',
+ hidden: true
+ };
+ Y.lp.app.privacy.setup_privacy_notification(config);
+ Y.lp.app.privacy.display_privacy_notification();
+ }
+ });
+ });
+ </script>
+ </tal:if>
<style type="text/css">
/* Align the 'add comment' link to the right of the comment box. */
#add-comment-form textarea { width: 100%; }
@@ -89,16 +111,6 @@
</metal:heading>
<div metal:fill-slot="main" tal:define="context_menu context/menu:context">
- <tal:if condition="request/features/bugs.private_notification.enabled">
- <div tal:attributes="class string: global-notification ${view/privacy_notice_classes}">
- <span class="sprite notification-private"></span>
- This bug is private
- <a href="#" class="global-notification-close">
- <span class="sprite notification-close"></span>
- Hide
- </a>
- </div>
- </tal:if>
<div id="tags-autocomplete">
<div id="tags-autocomplete-content"></div>
</div>
