Merge lp:~jcsackett/launchpad/diff-overlay-close into lp:launchpad

[j.c.sackett]

[r=bac][bug=720122] Adds a close event handler to the diff overlay so the close button actually works.

Summary
=======

The diff overlay for merge proposals (as seen on the branch index or bug index page) has a close button that doesn't do anything. This just adds a simple close function to the overlay bound to the close button's click event.

Preimplementation
=================

Spoke with Deryck Hodge about the reason for the breakage and ways to fix it.

Implementation
==============
lib/lp/code/javascript/branchmergeproposal.diff.js
--------------------------------------------------
A click handler is bound to the close button that dismisses the diff overlay.

Tests
=====

There's not really a good test for this that I can see; we're testing a lot of interaction between things. Similarly, windmill tests are bad, because they suck, and we're not testing data back and forth between page and server. I'm at a loss. If my assumptions are wrong, call me out and I'll add tests.

QA
==

The diff overlay in the example in the bug should be dismissable via the close button.

[Brad Crittenden (bac)]

Hi Jonathan,

Thanks for talking me through the semantics of Y.after. Totally unintuitive.

As we discussed, I think _dismissDiff is poorly named as it isn't doing the dismissal but is instead setting up the click handler to do the dismissing at the appropriate time.

I do note the dismiss button (X) is oddly located. Is that a function of PrettyOverlay?

I've never used the diff overlay before so I'm happy to have seen this branch and that you're making it work properly.

review: Approve (code)

=== modified file 'lib/lp/code/javascript/branchmergeproposal.diff.js'
--- lib/lp/code/javascript/branchmergeproposal.diff.js	2011-02-24 00:23:04 +0000
+++ lib/lp/code/javascript/branchmergeproposal.diff.js	2011-03-18 21:26:13 +0000
@@ -24,12 +24,22 @@
  */
 var DiffOverlay = function() {
     DiffOverlay.superclass.constructor.apply(this, arguments);
+    Y.after(this._setupDismissDiff, this, 'syncUI')
 };
 
 Y.extend(DiffOverlay, Y.lazr.PrettyOverlay, {
         bindUI: function() {
             // call PrettyOverlay's bindUI
             this.constructor.superclass.bindUI.call(this);
+        },
+
+        _setupDismissDiff: function() {
+            var cancel_icon = Y.one('.close-button');
+
+            // Setup a click handler to dismiss the diff overlay.
+            cancel_icon.on('click', function(e) {
+                Y.one('.yui3-diff-overlay').setStyle('display', 'none');
+            });
         }
     });