launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/launchpad/bug-779538 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #779538 in Launchpad itself: "notifications disappear when you click *any* link in the text"
  https://bugs.launchpad.net/launchpad/+bug/779538

For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-779538/+merge/60552

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 limiting the click-to-close
behavior to the message box itself, excluding any <a> sub-elements.  

The make lint report is clean.
-- 
https://code.launchpad.net/~benji/launchpad/bug-779538/+merge/60552
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-779538 into lp:launchpad.

=== 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-10 20:31:32 +0000
@@ -128,7 +128,11 @@
         // Hook up an event hanlder that will close any message boxes when they
         // are clicked.
         Y.delegate('click', function(e) {
-            this.setStyle('display', 'none');
+            // If the thing being clicked is not a link (which has it's own
+            // on-click behavior), hide the message box.
+            if(e.target.get('nodeName') != 'A') {
+                this.setStyle('display', 'none');
+            }
         }, 'body', '.message');
 
         Y.on('lp:context:web_link:changed', function(e) {