[cr-action-menu] Allow configurable anchors.

ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js

Index: ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
diff --git a/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js b/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
index 9a94cfa6204e750ccfec46c635eadf1e7ce077f2..2a5e00c76b7e28ef830d6bb018bd703352272416 100644
--- a/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
+++ b/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
@@ -151,7 +151,9 @@ Polymer({
     // Removing 'resize' and 'popstate' listeners when dialog is closed.
     this.removeListeners_();
     HTMLDialogElement.prototype.close.call(this);
-    this.anchorElement_.focus();
+    if (this.anchorElement_)
+      this.anchorElement_.focus();
+
     this.anchorElement_ = null;
   },
 
@@ -161,6 +163,54 @@ Polymer({
    */
   showAt: function(anchorElement) {
     this.anchorElement_ = anchorElement;
+    var rect = this.anchorElement_.getBoundingClientRect();
+
+    this.showAtPosition({
+      top: rect.top,
+      left: rect.left,
+      height: rect.height,
+      width: rect.width,
+      anchorPositionX: -1,

[dpapad, 2017/04/12 17:12:32]

Please explain this with a comment.

[calamity, 2017/04/13 04:34:24]

Done.

+    });
+  },
+
+  /**
+   * Shows the menu anchored to the given box.
+   * @param {{
+   *   top: number,
+   *   left: number,
+   *   bottom: (number| undefined),

[dpapad, 2017/04/12 17:12:32]

Where is config.bottom and config.right used?

[calamity, 2017/04/13 04:34:22]

Oops, these were supposed to change to width/height. Fixed.

+   *   right: (number| undefined),
+   *   anchorPositionX: (number| undefined),
+   *   anchorPositionY: (number| undefined),
+   *   minX: (number| undefined),
+   *   minY: (number| undefined),
+   *   maxX: (number| undefined),
+   *   maxY: (number| undefined),
+   * }} config
+   */

[dpapad, 2017/04/12 17:12:32]

Can we make a typedef for this? Such that clients of showAtPosition() don't need
to repeat this entire type when they document their code?

[calamity, 2017/04/13 04:34:23]

Done.

+  showAtPosition: function(config) {
+    /**
+     * @param {number|undefined} value
+     * @param {number} defaultValue
+     * @return {number}
+     */
+    var defaultIfUndefined =
+        function(value, defaultValue) {

[dpapad, 2017/04/12 17:12:32]

Can fit in previous line?

[calamity, 2017/04/13 04:34:23]

Done.

+      return value == undefined ? defaultValue : value;
+    }
+
+    var top = config.top;
+    var left = config.left;
+    var bottom = top + defaultIfUndefined(config.height, 0);
+    var right = left + defaultIfUndefined(config.width, 0);
+    var anchorPositionX = defaultIfUndefined(config.anchorPositionX, 1);
+    var anchorPositionY = defaultIfUndefined(config.anchorPositionY, 1);
+    var minX = defaultIfUndefined(config.minX, 0);
+    var maxX = defaultIfUndefined(config.maxX, window.innerWidth);
+    var minY = defaultIfUndefined(config.minY, 0);
+    var maxY = defaultIfUndefined(config.maxY, window.innerHeight);
+
     this.boundClose_ = this.boundClose_ || function() {
       if (this.open)
         this.close();
@@ -175,22 +225,34 @@ Polymer({
 
     this.showModal();
 
-    var rect = this.anchorElement_.getBoundingClientRect();
-    if (getComputedStyle(this.anchorElement_).direction == 'rtl') {
-      var right = window.innerWidth - rect.left - this.offsetWidth;
-      this.style.right = right + 'px';
-    } else {
-      var left = rect.right - this.offsetWidth;
-      this.style.left = left + 'px';
+    function getStartPointWithAnchor(

[dpapad, 2017/04/12 17:12:32]

@param annotations missing for this function. Also can we move it outside of
showAtPosition? showAtPosition is now fairly long, anything to make it shorter
will help readability.

[calamity, 2017/04/13 04:34:22]

Done.

+        start, end, length, anchorPosition, min, max) {
+      var startPoint = (start + end - length) / 2 +
+          (start - end + length) * anchorPosition / 2;
+      if (startPoint + length > max)
+        startPoint = end - length;
+      if (startPoint < min)
+        startPoint = start;
+      return startPoint;
     }
 
-    // Attempt to show the menu starting from the top of the rectangle and

[dpapad, 2017/04/12 17:12:32]

Has this logic been preserved in the new code? It is not obvious, can you point
out where this is happening and also preserve this comment?

[calamity, 2017/04/13 04:34:22]

Yes, this behavior is what getStartPointWithAnchor genericizes. This comment has
somewhat been added to the getStartPointWithAnchor method.

-    // extending downwards. If that does not fit within the window, fallback to
-    // starting from the bottom and extending upwards.
-    var top = rect.top + this.offsetHeight <= window.innerHeight ? rect.top :
-                                                                   rect.bottom -
-            this.offsetHeight - Math.max(rect.bottom - window.innerHeight, 0);
+    // Flip the X anchor in RTL.
+    var rtl = getComputedStyle(this).direction == 'rtl';
+    if (rtl)
+      anchorPositionX *= -1;
+
+    var menuLeft = getStartPointWithAnchor(
+        left, right, this.offsetWidth, anchorPositionX, minX, maxX);
+
+    if (rtl) {
+      var menuRight = window.innerWidth - menuLeft - this.offsetWidth;
+      this.style.right = menuRight + 'px';
+    } else {
+      this.style.left = menuLeft + 'px';
+    }
 
-    this.style.top = top + 'px';
+    var menuTop = getStartPointWithAnchor(
+        top, bottom, this.offsetHeight, anchorPositionY, minY, maxY);
+    this.style.top = menuTop + 'px';
   },
 });