Exiting menu mode when content preview is clicked

chrome/browser/resources/vr_shell/vr_shell_ui.js

Index: chrome/browser/resources/vr_shell/vr_shell_ui.js
diff --git a/chrome/browser/resources/vr_shell/vr_shell_ui.js b/chrome/browser/resources/vr_shell/vr_shell_ui.js
index b9fa59cf5b614eaf25918659e87c96b4d7af1735..a46d53d50c522cab0d3d877797bfaf041a06671a 100644
--- a/chrome/browser/resources/vr_shell/vr_shell_ui.js
+++ b/chrome/browser/resources/vr_shell/vr_shell_ui.js
@@ -39,6 +39,8 @@ var vrShellUi = (function() {
       /** @const */ this.MENU_MODE_SCREEN_HEIGHT = 0.8;
       /** @const */ this.MENU_MODE_SCREEN_ELEVATION = 0.2;
       /** @const */ this.BACKGROUND_DISTANCE_MULTIPLIER = 1.414;
+      /** @const */ this.DOM_INTERCEPTOR_SELECTOR = '#content-interceptor';
+      /** @const */ this.DOM_INTERCEPTOR_ELEVATION = 0.1;

[cjgrant, 2017/03/09 20:03:00]

10 cm seems big, since this quad is like a skin over the content.  Possibly 0.01
or 0.001?  I think we could visually see the cursor hover at 10 cm.

[acondor_, 2017/03/09 20:40:37]

Done.

 
       this.menuMode = false;
       this.fullscreen = false;
@@ -61,6 +63,24 @@ var vrShellUi = (function() {
       backPlane.setFill(new api.NoFill());
       ui.addElement(backPlane);
 
+      // Place invisible plane on top of content quad, to intercept the clicks
+      // while on menu mode in order to exit it.

[cjgrant, 2017/03/09 20:03:00]

optional: 'in order to exit it' - you could leave this out, as it's the kind of
detail that gets stale easily, and is clear in the code below.

[acondor_, 2017/03/09 20:40:38]

Done.

+      this.interceptor = new DomUiElement(this.DOM_INTERCEPTOR_SELECTOR);
+      let interceptorUpdate = new api.UiElementUpdate();

[cjgrant, 2017/03/09 20:03:00]

'let update' = is sufficient, or you could reuse a previous 'update' variable if
one exists.

[acondor_, 2017/03/09 20:40:37]

Done.

+      interceptorUpdate.setTranslation(0, 0, this.DOM_INTERCEPTOR_ELEVATION);
+      interceptorUpdate.setVisible(false);
+      interceptorUpdate.setParentId(this.elementId);
+      interceptorUpdate.setSize(
+          this.MENU_MODE_SCREEN_HEIGHT * this.SCREEN_RATIO,
+          this.MENU_MODE_SCREEN_HEIGHT);
+      ui.updateElement(this.interceptor.id, interceptorUpdate);
+      let interceptorButton =
+          document.querySelector(this.DOM_INTERCEPTOR_SELECTOR);
+      interceptorButton.addEventListener('click', function() {
+        uiManager.exitMenuMode();
+        ui.flush();
+      });
+
       this.updateState();
     }
 
@@ -106,6 +126,11 @@ var vrShellUi = (function() {
         distance = this.FULLSCREEN_DISTANCE;
       }
 
+      // show/hide the interceptor.

[cjgrant, 2017/03/09 20:03:00]

Comments should always be full sentences, ie "// Show or hide the interceptor."
However, I don't think this comment is needed.  :)

[acondor_, 2017/03/09 20:40:38]

Done.

+      let interceptorUpdate = new api.UiElementUpdate();

[cjgrant, 2017/03/09 20:03:00]

'let update' =  or  'update =' is cool here too (makes the code easier to read,
as we almost always call these objects 'update'.

[acondor_, 2017/03/09 20:40:37]

Done.

+      interceptorUpdate.setVisible(this.menuMode);
+      ui.updateElement(this.interceptor.id, interceptorUpdate);
+
       let anim;
       anim = new api.Animation(this.elementId, ANIM_DURATION);
       anim.setTranslation(0, y, -distance);
@@ -938,6 +963,13 @@ var vrShellUi = (function() {
       this.updateState();
     }
 
+    exitMenuMode() {
+      if (this.menuMode) {
+        this.menuMode = false;
+        this.updateState();
+      }
+    }
+
     updateState() {
       /** @const */ var URL_INDICATOR_VISIBILITY_TIMEOUT_MS = 5000;