Update the notification bubble.

chrome/browser/resources/shared/js/cr/ui/expandable_bubble.js

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// require: event_tracker.js
+
+cr.define('cr.ui', function() {
+  'use strict';
+
+  /**
+   * ExpandableBubble is a free-floating compact informational bubble with an
+   * arrow that points at a place of interest on the page. When clicked, the
+   * bubble expands to show more of its content. Width of the bubble is the
+   * width of the node it is overlapping when unexpanded. Expanded, it is of a
+   * fixed width, but variable height. Currently the arrow is always positioned
+   * at the bottom right and points down.
+   * @constructor
+   * @extends {cr.ui.div}
+   */
+  var ExpandableBubble = cr.ui.define('div');
+
+  ExpandableBubble.prototype = {
+    __proto__: HTMLDivElement.prototype,
+
+    /** @inheritDoc */
+    decorate: function() {
+      this.className = 'ex-bubble';
+      this.innerHTML =
+          '<div class=\"ex-bubble-contents\">' +

[arv (Not doing code reviews), 2011/10/14 22:20:55]

Replace \" with "

+            '<div class=\"ex-bubble-title\"></div>' +
+            '<div class=\"ex-bubble-main\" hidden=\"true\"></div>' +

[arv (Not doing code reviews), 2011/10/14 22:20:55]

It is hidden, not hidden="..."

'<div class="ex-bubble-main" hidden></div>' +

+          '</div>' +
+          '<div class=\"ex-bubble-close\" hidden=\"true\"></div>' +
+          '<div class=\"ex-bubble-shadow\"></div>' +
+          '<div class=\"ex-bubble-arrow\"></div>';
+
+      this.hidden = true;
+      this.handleCloseEvent = this.hide;
+      this.dismissOnBlur_ = true;

[Evan Stade, 2011/10/14 18:22:59]

this is always false, right? for now I wouldn't make this a toggleable option

[arv (Not doing code reviews), 2011/10/14 22:20:55]

This can be moved to the prototype.

+    },
+
+    /**
+     * Sets the title of the bubble. The title is always visible when the
+     * bubble is visible.
+     * @param {node} An HTML element.

[arv (Not doing code reviews), 2011/10/14 22:20:55]

@param {Node} node An HTML element.

or 

@type {Node}

since this is a setter.

+     */
+    set contentTitle(node) {
+      var bubbleTitle = this.querySelector('.ex-bubble-title');
+      bubbleTitle.innerHTML = '';

[arv (Not doing code reviews), 2011/10/14 22:20:55]

textContent = ''

But it doesn't really matter

+      bubbleTitle.appendChild(node);
+    },
+
+    /**
+     * Sets the content node of the bubble. The content node is only visible
+     * when the bubble is expanded.
+     * @param {node} An HTML element.
+     */
+    set content(node) {
+      var bubbleMain = this.querySelector('.ex-bubble-main');
+      bubbleMain.innerHTML = '';
+      bubbleMain.appendChild(node);
+    },
+
+    /**
+     * Sets the anchor node, i.e. the node that this bubble points at and
+     * partially overlaps.
+     * @param {HTMLElement} node The new anchor node.
+     */
+    set anchorNode(node) {
+      this.anchorNode_ = node;
+
+      if (!this.hidden)
+        this.resizeAndReposition();
+    },
+
+    /**
+    * Sets whether the bubble will dismiss on blur. By default, it does.

[arv (Not doing code reviews), 2011/10/14 22:20:55]

Please fix formatting of the comment

+    * @param {boolean} value Whether to dismiss on blur.
+    **/
+    set dismissOnBlur(value) {

[arv (Not doing code reviews), 2011/10/14 22:20:55]

This does not need a setter. since it has no side effect. I would just go with

/**
 * Whether the bubble will dismiss on blur. By default, it does.
 * @type {boolean}
 **/
dismissOnBlur: true,

+      this.dismissOnBlur_ = value;
+    },
+
+    /**
+     * Handles the close event which is triggered when the close button
+     * is clicked. By default is set to this.hide.
+     * @param {function} A function with no parameters
+     */
+    set handleCloseEvent(func) {

[arv (Not doing code reviews), 2011/10/14 22:20:55]

no need for setter

+      this.handleCloseEvent_ = func;
+    },
+
+    /**
+     * Updates the position of the bubble.
+     * @private
+     */
+    reposition: function() {
+      var clientRect = this.anchorNode_.getBoundingClientRect();
+      this.style.left = this.style.right = clientRect.left + 'px';
+
+      if (this.classList.contains('.ex-bubble-contents-expanded')) {
+        this.style.top = (clientRect.top - this.offsetHeight +

[arv (Not doing code reviews), 2011/10/14 22:20:55]

Maybe a variable for top to reduce the code size

+                                           this.unexpandedHeight - 1) + 'px';
+      } else {
+        this.style.top = (clientRect.top - 1) + 'px';
+      }
+    },
+
+    /**
+     * Resizes the bubble and then repositions it.
+     * @private
+     */
+    resizeAndReposition: function() {

[arv (Not doing code reviews), 2011/10/14 22:20:55]

This should have a trailing underscore since it is private.

+      var clientRect = this.anchorNode_.getBoundingClientRect();
+      var width = clientRect.width;
+      if (this.classList.contains('.ex-bubble-contents-expanded')) {
+        var expandedWidth = 250;
+        this.style.marginLeft = (width - expandedWidth) + 'px';
+        width = expandedWidth;
+      } else {
+        this.style.marginLeft = '0';
+      }
+
+      // Width is dynamic (when not expanded) based on the width of the anchor
+      // node, and the title and shadow need to follow suit.
+      this.style.width = width + 'px';
+      var bubbleTitle = this.querySelector('.ex-bubble-title');
+      bubbleTitle.style.width = (width - 2) + 'px';

[arv (Not doing code reviews), 2011/10/14 22:20:55]

I feel like the parentheses on expression like these does not help readability.
Also, the Google JS Style has my back here and says that parens should only be
used where required.

[Evan Stade, 2011/10/17 23:15:14]

I believe he got this style from copying my code. The reason I always do this is
because you're mixing two operations,  addition and string concatenation.
Although evaluation order saves us in this particular case, if you are not
careful this can lead to unexpected results, for example:

'foo' + 1 + 2 === 'foo12'

+      var bubbleContent = this.querySelector('.ex-bubble-main');
+      bubbleContent.style.width = (width - 12) + 'px';
+      var bubbleShadow = this.querySelector('.ex-bubble-shadow');
+      bubbleShadow.style.width = (width + 2) + 'px';
+
+      // Also reposition the bubble -- dimensions have potentially changed.
+      this.reposition();
+    },
+
+    /*
+     * Expand the bubble (bringing the full content into view).
+     * @private
+     */
+    expandBubble_: function() {
+      this.querySelector('.ex-bubble-main').hidden = false;
+      this.querySelector('.ex-bubble-close').hidden = false;
+      var title = this.querySelector('.ex-bubble-title');
+      title.classList.add('ex-bubble-title-bubble-expanded');

[arv (Not doing code reviews), 2011/10/14 22:20:55]

This could probably be done by adding and removing an expanded attribute on the
bubble element and then use css attribute selectors for the sub elements. Maybe
the elements are not in the same tree?

[Finnur, 2011/10/18 12:39:10]

Indeed. And I like it a lot more, actually. Done.

+      this.classList.add('.ex-bubble-contents-expanded');
+      this.resizeAndReposition();
+    },
+
+    /**
+     * Collapse the bubble, hiding the main content and the close button.
+     * This is automatically called when the window is resized.
+     * @private
+     */
+    collapseBubble_: function() {
+      this.querySelector('.ex-bubble-main').hidden = true;
+      this.querySelector('.ex-bubble-close').hidden = true;
+      var title = this.querySelector('.ex-bubble-title');
+      title.classList.remove('ex-bubble-title-bubble-expanded');
+      this.classList.remove('.ex-bubble-contents-expanded');
+      this.resizeAndReposition();
+    },
+
+    /**
+     * The onclick handler for the notification (expands the bubble).
+     * @param {Event} e The event.
+     * @private
+     */
+    onNotificationClick_ : function(e) {
+      if (!this.contains(e.target))
+        return;
+
+      if (!this.classList.contains('.ex-bubble-contents-expanded')) {
+        // Save the height of the unexpanded bubble, so we can make sure to
+        // position it correctly (arrow points in the same location) after
+        // we expand it.
+        this.unexpandedHeight = this.offsetHeight;
+      }
+
+      this.expandBubble_();
+    },
+
+    /**
+     * Starts showing the bubble. The bubble will grab input and show until the
+     * user clicks away.
+     */
+    show: function() {
+      if (!this.hidden)
+        return;
+
+      document.body.appendChild(this);
+      this.hidden = false;
+      this.resizeAndReposition();
+
+      this.eventTracker_ = new EventTracker;
+      this.eventTracker_.add(window,
+                             'load', this.resizeAndReposition.bind(this));
+      this.eventTracker_.add(window,
+                             'resize', this.collapseBubble_.bind(this));
+      this.eventTracker_.add(window,
+                             'click', this.onNotificationClick_.bind(this));
+
+      var doc = this.ownerDocument;
+      this.eventTracker_.add(doc, 'keydown', this, true);
+      this.eventTracker_.add(doc, 'mousedown', this, true);
+    },
+
+    /**
+     * Hides the bubble from view.
+     */
+    hide: function() {
+      this.hidden = true;
+      this.eventTracker_.removeAll();
+      this.parentNode.removeChild(this);
+    },
+
+    /**
+     * Handles keydown and mousedown events, dismissing the bubble if
+     * necessary.
+     * @param {Event} e The event.
+     * @private
+     */
+    handleEvent: function(e) {
+      switch (e.type) {
+        case 'keydown':
+          if (e.keyCode == 27)  // Esc.
+            this.hide();
+          break;
+
+        case 'mousedown':
+          if (e.target == this.querySelector('.ex-bubble-close')) {
+            this.handleCloseEvent_();
+          } else if (!this.contains(e.target)) {
+            // A click outside the bubble dismisses the bubble, unless that has
+            // been disabled. If dismissOnBlur has been disabled, it should
+            // collapse if expanded.
+            if (this.dismissOnBlur_)
+              this.hide();
+            else if (this.classList.contains('.ex-bubble-contents-expanded'))
+              this.collapseBubble_();
+          }
+          break;
+      }
+
+      e.stopPropagation();

[arv (Not doing code reviews), 2011/10/14 22:20:55]

Please add comments about both of these to explain why they are needed. It is a
common source of bugs to call stopPropagation and preventDefault too
aggressively.

[Finnur, 2011/10/18 12:39:10]

I don't know if they are needed. This is from the bubble.js class, which this
class is based on.

Evan, do you know?

On 2011/10/14 22:20:55, arv wrote:

[Evan Stade, 2011/10/18 16:34:15]

yea they are needed, this is a hacked up way of implementing a focus grab as I
don't think focus grabs are officially possible in js

+      e.preventDefault();
+      return;
+    },
+  };
+
+  return {
+    ExpandableBubble: ExpandableBubble
+  };
+});