Merge lp:~benji/launchpad/bug-779538 into lp:launchpad

[Benji York]

Bug 779538 describes a regression caused by my recent change to add a
"Hide X" link to notification boxes so the user can dismiss them when
they're no longer needed (especially useful on AJAX-heavy pages).
Unfortunately that behavior extended to links within the notification
box.

When a user would click on a link in a notification box the box would
disappear. This is primarily a problem when control- or shift-clicking
on a link to open it in another browser tab or window.

This branch rectifies that problem by using a different approach to
implement the click-to-close behavior. We now use setInterval to
periodically add real (as opposed to the previous approach of using CSS)
hide links and attaching click handlers to them. This preserves all the
expected behavior of message boxes (control-/shift-click on links
doesn't close them, highlighting text works correctly, etc.).

The setInterval period is so high (500 ms), and the function does so
little that it should introduce only negligible CPU load. Memory leaks
are also a concern with approaches like this: I can't see that any
objects are allocated in the "idle" case, so I believe there are no
leaks.

The make lint report is clean.

Unmerged revisions

13019. By Benji York on 2011-05-11

settled on final approach

13018. By Benji York on 2011-05-10

even better

13017. By Benji York on 2011-05-10

make the comment a little clearer

13016. By Benji York on 2011-05-10

a much nicer way to handle this

13015. By Benji York on 2011-05-10

works, but it's ugly

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-05-04 14:33:48 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-05-11 15:48:43 +0000
@@ -1017,23 +1017,13 @@
 .warning.message::before {
     content: url(/@@/warning-large);
     }
-.error.message::after, .warning.message::after,
-.informational.message::after {
-    /* Add an affordance that suggests the ability to hide the message. */
-    /* The NBSP (\00A0) is needed because of the margin trick below. */
-    content: 'Hide\00A0\2715';
+.error.message .hider, .warning.message .hider,
+.informational.message .hider {
+    color: #333;
     float: right;
     display: block;
     margin-left: 999em; /* Assure the message is always "bumped" down. */
     }
-.error.message:hover::after, .warning.message:hover::after,
-.informational.message:hover::after {
-    text-decoration: underline;
-    }
-.error.message:hover, .warning.message:hover,
-.informational.message:hover {
-    cursor: pointer;
-}
 .informational {
     /* Informational messages are blue-to-grey, alerts have an info icon. */
 

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2011-05-03 17:53:50 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2011-05-11 15:48:43 +0000
@@ -125,11 +125,29 @@
         // anywhere outside of it.
         Y.on('click', handleClickOnPage, window);
 
-        // Hook up an event hanlder that will close any message boxes when they
-        // are clicked.
-        Y.delegate('click', function(e) {
-            this.setStyle('display', 'none');
-        }, 'body', '.message');
+        // Add a "Hide" link to a message box.
+        var add_hide_to_message = function(message) {
+            if (Y.Lang.isValue(message.one('.hider'))) {
+                // We've already done this one.
+                return;
+            }
+            var hider = message.appendChild(Y.Node.create('<a/>')
+                .set('href', '#')
+                .set('text', 'Hide\u00A0\u2715')
+                .addClass('hider'));
+
+            Y.on('click', function(e) {
+                message.setStyle('display', 'none');
+            }, hider);
+        }
+
+        // Periodically check to see if there are any new message boxes on the
+        // page that need a hide link.
+        setInterval(function() {
+            Y.all('.warning.message').each(add_hide_to_message);
+            Y.all('.informational.message').each(add_hide_to_message);
+            Y.all('.error.message').each(add_hide_to_message);
+        }, 500);
 
         Y.on('lp:context:web_link:changed', function(e) {
             window.location = e.new_value;