Embedded Extension Options: Improve the overlay popup UI

chrome/browser/resources/extensions/extension_options_overlay.js

 // Copyright 2014 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.
 
 cr.define('extensions', function() {
   'use strict';
 
   /**
    * The ExtensionOptionsOverlay will show an extension's options page using
    * an <extensionoptions> element.
@@ skipping 32 matching lines @@
     /**
      * Handles a click on the close button.
      * @param {Event} e The click event.
      * @private
      */
     handleDismiss_: function(event) {
       this.setVisible_(false);
       var extensionoptions = document.querySelector('extensionoptions');
       if (extensionoptions)
         $('extension-options-overlay-guest').removeChild(extensionoptions);
+
+      var icon = $('extension-options-overlay-icon').querySelector('img');
+      if (icon)
+        $('extension-options-overlay-icon').removeChild(icon);

[not at google - send to devlin, 2014/08/22 22:31:00]

Should we basically be removing #extension-options-overlay from the DOM? Or at
least all its children?

[ericzeng, 2014/08/23 00:14:50]

No, every overlay's nodes remain in the DOM (not just this one), but the parent
overlay div's visibility is toggled. 

     },
 
     /**
      * Associate an extension with the overlay and display it.
      * @param {string} extensionId The id of the extension whose options page
      *     should be displayed in the overlay.
      * @param {string} extensionName The name of the extension, which is used
      *     as the header of the overlay.
+     * @param {string} extensionIcon The URL of the extension's icon.
      */
-    setExtensionAndShowOverlay: function(extensionId, extensionName) {
+    setExtensionAndShowOverlay: function(extensionId,
+                                         extensionName,
+                                         extensionIcon) {
       $('extension-options-overlay-title').textContent = extensionName;
 
+      var icon = document.createElement('img');
+      icon.setAttribute('src', extensionIcon);
+      $('extension-options-overlay-icon').appendChild(icon);

[not at google - send to devlin, 2014/08/22 22:31:00]

Why not always have the img in the DOM, then set the src when it's shown? Also,
make sure it has empty alt text.

[ericzeng, 2014/08/23 00:14:50]

Done

+
+      this.setVisible_(true);
+
       var extensionoptions = new ExtensionOptions();
       extensionoptions.extension = extensionId;
       extensionoptions.autosize = 'on';
 
       // The <extensionoptions> content's size needs to be restricted to the
       // bounds of the overlay window. The overlay gives a min width and
       // max height, but the maxheight does not include our header height
       // (title and close button), so we need to subtract that to get the
       // max height for the extension options.
-      var headerHeight = $('extension-options-overlay-title').offsetHeight;
+      var headerHeight = $('extension-options-overlay-header').offsetHeight;
       var overlayMaxHeight =
           parseInt($('extension-options-overlay').style.maxHeight);
       extensionoptions.maxheight = overlayMaxHeight - headerHeight;
 
       extensionoptions.minwidth =
           parseInt(window.getComputedStyle($('extension-options-overlay'))
               .minWidth);
 
       // Resize the overlay if the <extensionoptions> changes size.
       extensionoptions.onsizechanged = function(evt) {
@@ skipping 30 matching lines @@
       // Don't allow the <extensionoptions> to autosize until the overlay
       // animation is complete.
       extensionoptions.setDeferAutoSize(true);
 
       // Move the <extensionoptions> off screen until the overlay is ready
       $('extension-options-overlay-guest').style.position = 'fixed';
       $('extension-options-overlay-guest').style.left =
           window.outerWidth + 'px';
 
       $('extension-options-overlay-guest').appendChild(extensionoptions);
-      this.setVisible_(true);
     },
 
     /**
      * Toggles the visibility of the ExtensionOptionsOverlay.
      * @param {boolean} isVisible Whether the overlay should be visible.
      * @private
      */
     setVisible_: function(isVisible) {
       this.showOverlay_(isVisible ? $('extension-options-overlay') : null);
     }
   };
 
   // Export
   return {
     ExtensionOptionsOverlay: ExtensionOptionsOverlay
   };
 });