Merge lp:~huwshimi/launchpad/private-objects-298152 into lp:launchpad

[Huw Wilkins]

[r=gmb][bug=298152] Changed private bug notifications to be more obvious. This is only available under a feature flag.

The current style for notifying private bugs is not obvious enough. After some discussion I created a new style that overlays a banner at the top of the page that scrolls with it.

The banner can be closed, and when closed the privacy portlet should fade to red.

To test the notification you'll need a bug that has been marked private and you'll need to set the flag with 'bugs.private_notification.enabled default 1 on' on https://launchpad.dev/+feature-rules

Here is a screenshot of this change: http://launchpadlibrarian.net/66331334/privacy_red.png

[Martin Pool (mbp)]

I don't want to bikeshed so make of these what you will:

1- stackoverflow etc show the ribbon above (y-axis) the page content,
rather than overlaid on it (z-axis), so it does not obscure any
content.

2- The current hashmarks have the advantage that you can still see the
bug is private when you have scrolled down the page, and it's very
common for bugs to be more than one screen long.

Martin

[Huw Wilkins (huwshimi)]

> 1- stackoverflow etc show the ribbon above (y-axis) the page content,
> rather than overlaid on it (z-axis), so it does not obscure any
> content.

That screenshot was probably not the best. This banner works exactly like the banner on Stackoverflow. That screenshot shows the page scrolled down a bit but it does push the content down when you're at the top of the page.

>
> 2- The current hashmarks have the advantage that you can still see the
> bug is private when you have scrolled down the page, and it's very
> common for bugs to be more than one screen long.

Again, like Stackoverflow the banner stays at the top of the visible area.

[Martin Pool (mbp)]

Thanks for clarifying.

There is a bit of a difference which is that the stackoverflow
notifications are point-in-time and once they are dismissed the user
will never see that particular notification again. Privacy on the
other hand is a persistent attribute of the bug state and will for
some users occur on nearly every bug they look at. In that case
needing to dismiss it for every window, or having it always overlay
the window contents might get tedious.

(But perhaps the users who complained privacy was insufficiently
prominent will be happy to have it just always hanging around.)

[Graham Binns (gmb)]

Hi Huw,

Thanks for this change. My only request is that we move the JS you've added to bugtask-index.pt into lib/lp/bugs/javascript/bugtask_index.js so that we can keep the JS neat(ish) and in one place.

Otherwise this is r=me.

review: Approve (code)

[Huw Wilkins (huwshimi)]

> Thanks for this change. My only request is that we move the JS you've added to
> bugtask-index.pt into lib/lp/bugs/javascript/bugtask_index.js so that we can
> keep the JS neat(ish) and in one place.

Thanks, that's been done.

I also found a bug when changing the bug between private to public where it would not show/hide the banner. This has been fixed, but required some big changes to the code.

[Graham Binns (gmb)]

Hi Huw,

I've taken a look at your changes and they look fine. r=me.

review: Approve (code)

[Brad Crittenden (bac)]

Huw this branch deleted the file icon-sprites which seems to have been a mistake. I am in the process of restoring the file and landing it. If you meant for that file to really go away please provide an explanation. Thanks.

[Brad Crittenden (bac)]

Huw, never mind. I see that the file is generated and therefore should not be in our tree. Unfortunately we didn't have rules in our makefile to generate it automatically. I've just landed a fix to do just that.

=== removed file 'lib/canonical/launchpad/icing/icon-sprites'
Binary files lib/canonical/launchpad/icing/icon-sprites	2011-02-16 04:32:34 +0000 and lib/canonical/launchpad/icing/icon-sprites	1970-01-01 00:00:00 +0000 differ
=== modified file 'lib/canonical/launchpad/icing/icon-sprites.positioning'
--- lib/canonical/launchpad/icing/icon-sprites.positioning	2011-02-16 04:32:34 +0000
+++ lib/canonical/launchpad/icing/icon-sprites.positioning	2011-03-23 01:08:27 +0000
@@ -441,6 +441,10 @@
         0, 
         -4096
     ], 
+    "../images/notification-close.png": [
+        0, 
+        -20725
+    ], 
     "../images/merge-proposal-large.png": [
         0, 
         -17868
@@ -453,6 +457,10 @@
         0, 
         -1968
     ], 
+    "../images/notification-private.png": [
+        0, 
+        -20554
+    ], 
     "../images/bug.png": [
         0, 
         -4260

=== modified file 'lib/canonical/launchpad/icing/sprite.css.in'
--- lib/canonical/launchpad/icing/sprite.css.in	2011-03-18 22:47:24 +0000
+++ lib/canonical/launchpad/icing/sprite.css.in	2011-03-23 01:08:27 +0000
@@ -492,3 +492,11 @@
     background-image: url(/@@/trash-logo.png); /* sprite-ref: icon-sprites */
     background-repeat: no-repeat;
     }
+.notification-private {
+    background-image: url(/@@/notification-private.png); /* sprite-ref: icon-sprites */
+    background-repeat: no-repeat;
+    }
+.notification-close {
+    background-image: url(/@@/notification-close.png); /* sprite-ref: icon-sprites */
+    background-repeat: no-repeat;
+    }

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-03-21 21:30:07 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-03-23 01:08:27 +0000
@@ -24,6 +24,7 @@
       * Header, footer, side, help
       * Colors
       * Sprites
+      * Global notifications
       * Application or page specifc styles that do not belong in this file.
 */
 
@@ -132,6 +133,10 @@
     /* It must be obvious to the user that the context is private */
     background: url("/@@/private-y-bg") top left repeat-y;
     }
+/* Override for when the feature flag is active */
+body.feature-flag-bugs-private-notification-enabled.private {
+    background-image: none;
+    }
 * html body {
     /* stops floats dropping in IE 5.5/6 */
     word-wrap: break-word;
@@ -1102,7 +1107,7 @@
     font-size: 12px;
     text-decoration: underline;
     }
-.unseen {
+.unseen, .hidden {
     display: none;
     }
 .invisible-link {
@@ -1112,6 +1117,11 @@
     left:-9999em;
     display:block;
     }
+.transparent {
+  opacity: 0;
+  filter: alpha(opacity=0);
+  -ms-filter:"progid:DXImageTransform.Microsoft.Alpha(Opacity=0)";
+}
 .rss-right {
     background: url(/@@/rss.png) right center no-repeat;
     }
@@ -1314,8 +1324,14 @@
     background: #fbfbfb;
     }
 #privacy.private {
-      background: url(/@@/private-bg) top left repeat-x; /* 8px high */
-      padding-top: 12px; /* = 8px + the usual 4px top padding */
+    background: url(/@@/private-bg) top left repeat-x; /* 8px high */
+    padding-top: 12px; /* = 8px + the usual 4px top padding */
+    }
+/* Override for when the feature flag is active */
+.feature-flag-bugs-private-notification-enabled #privacy.private {
+    background-image: none;
+    background-color: #FBFBFB;
+    padding-top: 0.5em;
     }
 .downloads li {
     margin: 0;
@@ -1876,6 +1892,68 @@
     padding:2px 0 0 18px;
     }
 
+
+/* ====================
+   Global notifications
+*/
+/* Move the content down so the notification banner doesn't hide any content. */
+body.global-notification-visible {
+    padding-top: 40px;
+    }
+body.global-notification-visible .login-logout {
+    top: 45px;
+    }
+.global-notification {
+    position: fixed;
+    z-index: 10;
+    top: 0;
+    left: 0;
+    right: 0;
+    padding: 8px 20px;
+    /* Define colour for browsers that don't support transparency */
+    background-color: #8d1f1f;
+    /* Set transparent background for browsers that support it */
+    background-color: rgba(125,0,0,0.9);
+    color: #fff;
+    text-shadow: 0 -1px 0 #631616;
+    font-size: 14px;
+    line-height: 21px;
+    font-weight: bold;
+    -moz-box-shadow: 0 0 5px #333;
+    -webkit-box-shadow: 0 0 5px #333;
+    box-shadow: 0 0 5px #333;
+    }
+.global-notification .sprite.notification-private {
+    float: left;
+    display: inline-block;
+    height: 21px;
+    width: 20px;
+    margin-right: 10px;
+    padding: 0;
+    }
+.global-notification-close, .global-notification-close:active,
+.global-notification-close:visited {
+    color: #e47a7a;
+    }
+.global-notification-close {
+    display: block;
+    position: absolute;
+    top: 11px;
+    right: 20px;
+    font-size: 12px;
+    font-weight: normal;
+    line-height: 14px;
+    }
+.global-notification-close .sprite.notification-close {
+    float: right;
+    display: block;
+    height: 9px;
+    width: 8px;
+    margin: 3px 0 0 7px;
+    padding: 0;
+    }
+
+
 /*
  * YOU HAVE REACHED THE END OF THIS FILE. IF YOU SEE ANYTHING BELOW SPRITES
  * YOU SHOULD WORK TO GET IT OUT OF THIS GLOBAL STYLE SHEET.

=== added file 'lib/canonical/launchpad/images/notification-close.png'
Binary files lib/canonical/launchpad/images/notification-close.png	1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/notification-close.png	2011-03-23 01:08:27 +0000 differ
=== added file 'lib/canonical/launchpad/images/notification-private.png'
Binary files lib/canonical/launchpad/images/notification-private.png	1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/notification-private.png	2011-03-23 01:08:27 +0000 differ
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-03-21 21:36:55 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-03-23 01:08:27 +0000
@@ -974,6 +974,13 @@
         else:
             return bugtask_heat_html(self.context)
 
+    @property
+    def privacy_notice_classes(self):
+        if not self.context.bug.private:
+            return 'hidden'
+        else:
+            return ''
+
 
 def calculate_heat_display(heat, max_bug_heat):
     """Calculate the number of heat 'flames' to display."""

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-02-24 00:23:04 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-03-23 01:08:27 +0000
@@ -43,6 +43,15 @@
     Y.lp.bugs.bugtask_index.portlets.setup_portlet_handlers();
 
     /*
+     * Display the privacy notification if the bug is private
+     */
+    if (bug_private) {
+        Y.on("domready", function () {
+            display_privacy_notification();
+        });
+    }
+    
+    /*
      * Check the page for links related to overlay forms and request the HTML
      * for these forms.
      */
@@ -372,11 +381,19 @@
                     privacy_text.set(
                         'innerHTML',
                         'This report is <strong>private</strong> ');
+                    display_privacy_notification();
                 } else {
+                    if (bugs_private_notification_enabled) {
+                        if (Y.one('.global-notification').hasClass('hidden')) {
+                            Y.one('.portlet.private').setStyle('color', '#333');
+                            Y.one('.portlet.private').setStyle('background-color', '#fbfbfb');
+                        }
+                    }
                     Y.one('body').replaceClass('private', 'public');
                     privacy_div.replaceClass('private', 'public');
                     privacy_text.set(
                         'innerHTML', 'This report is public ');
+                    hide_privacy_notification();
                 }
                 privacy_text.appendChild(privacy_link);
                 privacy_text.appendChild(privacy_spinner);
@@ -411,6 +428,108 @@
 };
 
 
+/*
+ * Display privacy notifications
+ *
+ * This should be called after the page has loaded e.g. on 'domready'.
+ */
+function display_privacy_notification() {
+    /* Check if the feature flag is set for this notification. */
+    if (bugs_private_notification_enabled) {
+        /* Set a temporary class on the body for the feature flag,
+         this is because we have no way to use feature flags in
+         css directly. This should be remove if the feature
+         is accepted. */
+        Y.one('body').addClass('feature-flag-bugs-private-notification-enabled');
+        /* Set the visible flag so that the content moves down. */
+        Y.one('body').addClass('global-notification-visible');
+        
+        if (Y.one('.global-notification').hasClass('hidden')) {
+            Y.one('.global-notification').addClass('transparent');
+            Y.one('.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: {'padding-top': '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.run();
+            body_space.run();
+            login_space.run();
+        }
+        
+        Y.on('click', function(e) {
+            hide_privacy_notification(true);
+            e.halt();
+        }, '.global-notification-close');
+    }
+}
+
+
+/*
+ * Hide privacy notifications
+ *
+ * This should be called after the page has loaded e.g. on 'domready'.
+ */
+function hide_privacy_notification(highlight_portlet) {
+    if (bugs_private_notification_enabled) {
+        if (!Y.one('.global-notification').hasClass('hidden')) {
+            var fade_out = new Y.Anim({
+                node: '.global-notification',
+                to: {opacity: 0},
+                duration: 0.3
+            });
+            var body_space = new Y.Anim({
+                node: 'body',
+                to: {'padding-top': 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();
+            
+            if (highlight_portlet) {
+                var portlet_colour = new Y.Anim({
+                    node: '.portlet.private',
+                    to: {
+                        color: '#fff',
+                        backgroundColor:'#8d1f1f'
+                    },
+                    duration: 0.4,
+                });
+                portlet_colour.run();
+            }
+        }
+    }
+}
+
+
 /**
  * Set up the link-a-related-branch picker.
  */

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-03-15 01:40:18 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-03-23 01:08:27 +0000
@@ -21,6 +21,26 @@
           use_advanced_subscriptions = false;
         </script>
       </tal:subscriptions-js>
+      <tal:if condition="context/bug/private">
+        <script type="text/javascript">
+          var bug_private = true;
+        </script>
+      </tal:if>
+      <tal:if condition="not:context/bug/private">
+        <script type="text/javascript">
+          var bug_private = false;
+        </script>
+      </tal:if>
+      <tal:if condition="request/features/bugs.private_notification.enabled">
+        <script type="text/javascript">
+          var bugs_private_notification_enabled = true;
+        </script>
+      </tal:if>
+      <tal:if condition="not:request/features/bugs.private_notification.enabled">
+        <script type="text/javascript">
+          var bugs_private_notification_enabled = false;
+        </script>
+      </tal:if>
       <script type="text/javascript">
         LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bugtask_index',
                   'lp.bugs.bugtask_index.portlets',
@@ -69,6 +89,16 @@
     </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>

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-03-11 12:27:31 +0000
+++ lib/lp/services/features/flags.py	2011-03-23 01:08:27 +0000
@@ -85,6 +85,10 @@
      'boolean',
      'Shows the server-side page render time in the login widget.',
      ''),
+    ('bugs.private_notification.enabled',
+     'boolean',
+     'Changes the appearance of notifications on private bugs.',
+     ''),
     ])
 
 # The set of all flag names that are documented.