Make pop-up menus more easily dismissable.

Previously, menus were dismissed by adding a click handler to the <body>, but that doesn't intercept clicks on the plugin element and complicates the logic somewhat because a second click on the button while the menu is visible then triggers both the show and hide handlers by default. This CL adds an explicit <div> covering the entire window, but behind the menu in the z-order. This <div> doesn't prevent window dragging, despite being on top of the title-bar, so I've also made that conditional on there being no menu visible. BUG=416307 Review URL: https://codereview.chromium.org/592793003 Cr-Commit-Position: refs/heads/master@{#296631}

Make pop-up menus more easily dismissable.
Previously, menus were dismissed by adding a click handler to the <body>, but that doesn't intercept clicks on the plugin element and complicates the logic somewhat because a second click on the button while the menu is visible then triggers both the show and hide handlers by default. This CL adds an explicit <div> covering the entire window, but behind the menu in the z-order. This <div> doesn't prevent window dragging, despite being on top of the title-bar, so I've also made that conditional on there being no menu visible. BUG=416307 Review URL: https://codereview.chromium.org/592793003 Cr-Commit-Position: refs/heads/master@{#296631}
36f3bce5 · jamiewalch · Commit bot · 119c6fa8 · 36f3bce5 · 36f3bce5
Commit 36f3bce5 authored Sep 24, 2014 by jamiewalch Committed by Commit bot Sep 25, 2014
4 changed files
--- a/remoting/webapp/menu_button.css
+++ b/remoting/webapp/menu_button.css
@@ -49,7 +49,7 @@ button.menu-button-activator.active {
  outline: 1px solid rgba(0, 0, 0, 0.2);
  padding: 0 0 6px;
  box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.2);
-  z-index: 1;
+  z-index: 2;
 }
 .menu-button li {
@@ -89,3 +89,12 @@ button.menu-button-activator.active {
  margin-left: 8px;
  margin-top: 4px;
 }
+.menu-button-click-trap {
+  position: fixed;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+  z-index: 1;
+}
\ No newline at end of file
--- a/remoting/webapp/menu_button.js
+++ b/remoting/webapp/menu_button.js
@@ -47,35 +47,41 @@ remoting.MenuButton = function(container, opt_onShow, opt_onHide) {
   */
  this.onHide_ = opt_onHide;
-  /** @type {remoting.MenuButton} */
-  var that = this;
  /**
-   * @type {function(Event):void}
+   * Create a "click-trap" div covering the entire document, but below the
+   * menu in the z-order. This ensures the the menu can be closed by clicking
+   * anywhere. Note that adding this event handler to <body> is not enough,
+   * because elements can prevent event propagation; specifically, the client
+   * plugin element does this.
+   *
+   * @type {HTMLElement}
   * @private
   */
-  var closeHandler = function(event) {
+  this.clickTrap_ = /** @type {HTMLElement} */ (document.createElement('div'));
+  this.clickTrap_.classList.add('menu-button-click-trap');
+  /** @type {remoting.MenuButton} */
+  var that = this;
+  var closeHandler = function() {
    that.button_.classList.remove(remoting.MenuButton.BUTTON_ACTIVE_CLASS_);
-    document.body.removeEventListener('click', closeHandler, false);
+    container.removeChild(that.clickTrap_);
    if (that.onHide_) {
      that.onHide_();
    }
  };
-  /**
+  var onClick = function() {
-   * @type {function(Event):void}
-   * @private
-   */
-  var onClick = function(event) {
    if (that.onShow_) {
      that.onShow_();
    }
    that.button_.classList.add(remoting.MenuButton.BUTTON_ACTIVE_CLASS_);
-    document.body.addEventListener('click', closeHandler, false);
+    container.appendChild(that.clickTrap_);
-    event.stopPropagation();
  };
  this.button_.addEventListener('click', onClick, false);
+  this.clickTrap_.addEventListener('click', closeHandler, false);
+  this.menu_.addEventListener('click', closeHandler, false);
 };
 /**

--- a/remoting/webapp/unittests/menu_button_unittest.js
+++ b/remoting/webapp/unittests/menu_button_unittest.js
@@ -40,22 +40,23 @@ test('should display on click', function() {
  ok(menu.offsetWidth != 0 && menu.offsetHeight != 0);
 });
-test('should dismiss when <body> is clicked', function() {
+test('should dismiss when the menu is clicked', function() {
  var menu = menuButton.menu();
  menuButton.button().click();
-  document.body.click();
+  menu.click();
  ok(menu.offsetWidth == 0 && menu.offsetHeight == 0);
 });
-/*
+test('should dismiss when anything outside the menu is clicked', function() {
-TODO(jamiewalch): Reinstate this once MenuButton is fixed properly.
-test('should dismiss when button is clicked', function() {
  var menu = menuButton.menu();
  menuButton.button().click();
-  menuButton.button().click();
+  var x = menu.offsetRight + 1;
+  var y = menu.offsetBottom + 1;
+  var notMenu = document.elementFromPoint(x, y);
+  base.debug.assert(notMenu != menu);
+  notMenu.click();
  ok(menu.offsetWidth == 0 && menu.offsetHeight == 0);
 });
-*/
 test('should dismiss when menu item is clicked', function() {
  var menu = menuButton.menu();
@@ -70,7 +71,7 @@ test('should invoke callbacks', function() {
  menuButton.button().click();
  ok(onShow.called);
  ok(!onHide.called);
-  document.body.click();
+  menuButton.menu().click();
  ok(onHide.called);
 });

--- a/remoting/webapp/window_frame.css
+++ b/remoting/webapp/window_frame.css
@@ -36,10 +36,13 @@ html.apps-v2 body:not(.fullscreen) {
  padding-__MSG_@@bidi_start_edge__: 12px;
  width: 100%;
  display: inline-block;
-  -webkit-app-region: drag;
  flex: 1;
 }
+.title-bar:not(.menu-opened) .window-title {
+  -webkit-app-region: drag;
+}
 .window-control {
  width: 32px;
  height: 32px;