Merge lp:~teknico/juju-gui/charm-panel-docs into lp:juju-gui/experimental

[Nicola Larosa]

Better charm panel docs.

Improve the charm panel comment blocks.

https://codereview.appspot.com/7307063/

[Nicola Larosa (teknico)]

Reviewers: mp+147174_code.launchpad.net,

Message:
Please take a look.

Description:
Better charm panel docs.

Improve the charm panel comment blocks.

https://code.launchpad.net/~teknico/juju-gui/charm-panel-docs/+merge/147174

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7307063/

Affected files:
   A [revision details]
   M app/views/charm-panel.js
   M undocumented

[Gary Poster (gary)]

Land as is.

Thank you.

Gary

https://codereview.appspot.com/7307063/

[Madison Scott-Clary (makyo)]

Land as is. Thanks for the docs!

https://codereview.appspot.com/7307063/

[Nicola Larosa (teknico)]

*** Submitted:

Better charm panel docs.

Improve the charm panel comment blocks.

R=gary.poster, matthew.scott
CC=
https://codereview.appspot.com/7307063

https://codereview.appspot.com/7307063/

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2013-01-30 12:38:03 +0000
+++ app/views/charm-panel.js	2013-02-07 17:04:21 +0000
@@ -1,9 +1,10 @@
 'use strict';
 
 /**
- * The charm panel view(s).
+ * The charm panel views implement the various things shown in the right panel
+ * when clicking on the "Charms" label in the title bar, or running a search.
  *
- * @module views
+ * @module charm-panel
  */
 
 YUI.add('juju-charm-panel', function(Y) {
@@ -16,6 +17,7 @@
       subscriptions = [],
       // Singleton
       _instance,
+
       // See https://github.com/yui/yuidoc/issues/25 for issue tracking
       // missing @function tag.
       /**
@@ -46,6 +48,7 @@
           icon.replaceClass('chevron_up', 'chevron_down');
         }
       },
+
       /**
        * Given a container node and a total height available, set the height of
        * a '.charm-panel' node to fill the remaining height available to it
@@ -68,6 +71,7 @@
           scrollContainer.setStyle('height', scrollHeight + 'px');
         }
       },
+
       /**
        * Given a set of entries as returned by the charm store "find"
        * method (charms grouped by series), return the list filtered
@@ -84,6 +88,7 @@
        */
       filterEntries = function(entries, filter, services) {
         var deployedCharms;
+
         /**
          * Filter to determine if a charm is a subordinate.
          *
@@ -94,6 +99,7 @@
         function isSubFilter(charm) {
           return !!charm.get('is_subordinate');
         }
+
         /**
          * Filter to determine if a charm is the same as any
          * deployed services.
@@ -105,6 +111,7 @@
         function isDeployedFilter(charm) {
               return deployedCharms.indexOf(charm.get('id')) !== -1;
         }
+
         var filter_fcn;
 
         if (filter === 'all') {
@@ -157,6 +164,7 @@
               };
             });
       },
+
       /**
        * Given an array of interface data as stored in a charm's "required"
        * and "provided" attributes, return an array of interface names.
@@ -179,7 +187,8 @@
       };
 
   /**
-   * Display a unit.
+   * Charm collection view. Show a list of charms, each clickable through
+   * for a description, or deployed directly with a "Deploy" button.
    *
    * @class CharmCollectionView
    * @namespace views
@@ -190,12 +199,25 @@
       'a.charm-detail': {click: 'showDetails'},
       '.charm-entry .btn.deploy': {click: 'showConfiguration'},
       '.charm-entry': {
+
+        /**
+         * Show the charm deploy button on mouse pointer enter.
+         *
+         * @method CharmCollectionView.events.mouseenter
+         */
         mouseenter: function(ev) {
           ev.currentTarget.all('.btn').transition({opacity: 1, duration: 0.25});
         },
+
+        /**
+         * Hide the charm deploy button on mouse pointer leave.
+         *
+         * @method CharmCollectionView.events.mouseleave
+         */
         mouseleave: function(ev) {
           ev.currentTarget.all('.btn').transition({opacity: 0, duration: 0.25});
         }
+
       },
       '.charm-filter-picker .picker-button': {
         click: 'showCharmFilterPicker'
@@ -204,9 +226,15 @@
         click: 'hideCharmFilterPicker'
       }
     },
-    // Set searchText to cause the results to be found and rendered.
-    // Set defaultSeries to cause all the results for the default series to be
-    // found and rendered.
+
+    /**
+     * Set searchText to cause the results to be found and rendered.
+     *
+     * Set defaultSeries to cause all the results for the default series
+     * to be found and rendered.
+     *
+     * @method CharmCollectionView.initializer
+     */
     initializer: function() {
       var self = this;
       this.set('filter', 'all');
@@ -224,6 +252,7 @@
               });
         }
       });
+
       this.after('defaultSeriesChange', function(ev) {
         this.set('defaultEntries', null);
         if (ev.newVal) {
@@ -248,6 +277,10 @@
       });
       this.after('heightChange', this._setScroll);
     },
+
+    /**
+     * @method CharmCollectionView.render
+     */
     render: function() {
       var container = this.get('container'),
           searchText = this.get('searchText'),
@@ -299,9 +332,10 @@
       this._setScroll();
       return this;
     },
+
     /**
-     * When the view's "height" attribute is set, adjust the internal scrollable
-     * div to have the appropriate height.
+     * When the view's "height" attribute is set, adjust the internal
+     * scrollable div to have the appropriate height.
      *
      * @method _setScroll
      * @protected
@@ -315,6 +349,7 @@
         scrollContainer.setStyle('height', height - 1 + 'px');
       }
     },
+
     /**
      * Fire an event indicating that the charm panel should switch to the
      * "description" for a given charm.
@@ -330,6 +365,15 @@
           { name: 'description',
             charmId: ev.target.getAttribute('href') });
     },
+
+    /**
+     * Fire an event indicating that the charm panel should switch to the
+     * "configuration" for a given charm.
+     *
+     * @method showConfiguration
+     * @param {Object} ev An event object (with a `halt` method).
+     * @return {undefined} Sends a signal only.
+     */
     showConfiguration: function(ev) {
       // Without the ev.halt the 'outside' click handler is getting
       // called which immediately closes the panel.
@@ -339,8 +383,9 @@
           { name: 'configuration',
             charmId: ev.currentTarget.getData('url')});
     },
+
     /**
-     * Create a data structure friendly to the view
+     * Create a data structure friendly to the view.
      *
      * @method normalizeCharms.
      */
@@ -389,6 +434,7 @@
         return {series: name, charms: hash[name]};
       });
     },
+
     /**
      * Find charms that match a query.
      *
@@ -419,6 +465,12 @@
             );
           }}});
     },
+
+    /**
+     * Show errors on both console and notifications.
+     *
+     * @method _showErrors
+     */
     _showErrors: function(e) {
       console.error(e.error);
       this.get('db').notifications.add(
@@ -463,7 +515,8 @@
   views.CharmCollectionView = CharmCollectionView;
 
   /**
-   * Display a unit.
+   * Charm description view. It describes a charm's features in detail,
+   * together with a "Deploy" button.
    *
    * @class CharmDescriptionView
    * @namespace views
@@ -478,10 +531,16 @@
           '.charm-section h4': {click: toggleSectionVisibility},
           'a.charm-detail': {click: 'showDetails'}
         },
+        /**
+         * @method CharmDescriptionView.initializer
+         */
         initializer: function() {
           this.bindModelView(this.get('model'));
           this.after('heightChange', this._setScroll);
         },
+        /**
+         * @method CharmDescriptionView.render
+         */
         render: function() {
           var container = this.get('container'),
               charm = this.get('model');
@@ -647,7 +706,9 @@
   views.CharmDescriptionView = CharmDescriptionView;
 
   /**
-   * Display a charms configuration panel.
+   * Display a charm's configuration panel. It shows editable fields for
+   * the charm's configuration parameters, together with a "Cancel" and
+   * a "Confirm" button for deployment.
    *
    * @class CharmConfigurationView
    * @namespace views
@@ -657,10 +718,18 @@
         template: views.Templates['charm-pre-configuration'],
         tooltip: null,
         configFileContent: null,
+
+        /**
+         * @method CharmConfigurationView.initializer
+         */
         initializer: function() {
           this.bindModelView(this.get('model'));
           this.after('heightChange', this._setScroll);
         },
+
+        /**
+         * @method CharmConfigurationView.render
+         */
         render: function() {
           var container = this.get('container'),
               charm = this.get('model'),
@@ -684,6 +753,7 @@
           this._setScroll();
           return this;
         },
+
         /**
          * When the view's "height" attribute is set, adjust the internal
          * scrollable div to have the appropriate height.
@@ -695,6 +765,7 @@
         _setScroll: function() {
           setScroll(this.get('container'), this.get('height'));
         },
+
         events: {
           '.btn.cancel': {click: 'goBack'},
           '.btn.deploy': {click: 'onCharmDeployClicked'},
@@ -706,8 +777,9 @@
           'input.config-field[type=checkbox]':
               {click: function(evt) {evt.target.focus();}}
         },
+
         /**
-         * Determines the Y coordinate that would center a tooltip on a field.
+         * Determine the Y coordinate that would center a tooltip on a field.
          *
          * @static
          * @param {Number} fieldY The current Y position of the tooltip.
@@ -720,8 +792,9 @@
           var y_offset = (tooltipHeight - fieldHeight) / 2;
           return fieldY - y_offset;
         },
+
         /**
-         * Determines the X coordinate that would place a tooltip next to a
+         * Determine the X coordinate that would place a tooltip next to a
          * field.
          *
          * @static
@@ -733,6 +806,12 @@
         _calculateTooltipX: function(fieldX, tooltipWidth) {
           return fieldX - tooltipWidth - 15;
         },
+
+        /**
+         * Move a tooltip to its predefined position.
+         *
+         * @method _moveTooltip
+         */
         _moveTooltip: function() {
           if (this.tooltip.field &&
               Y.DOM.inRegion(
@@ -759,6 +838,12 @@
             this.tooltip.hide();
           }
         },
+
+        /**
+         * Show the charm's description.
+         *
+         * @method showDescription
+         */
         showDescription: function(evt) {
           var controlGroup = evt.target.ancestor('.control-group'),
               node = controlGroup.one('.control-description'),
@@ -772,10 +857,17 @@
               this.tooltip.panel.getDOMNode());
           this._moveTooltip();
         },
+
+        /**
+         * Hide the charm's description.
+         *
+         * @method hideDescription
+         */
         hideDescription: function(evt) {
           this.tooltip.hide();
           delete this.tooltip.field;
         },
+
         /**
          * Pass clicks on the overlay on to the correct recipient.
          * The recipient can be the upload widget or the file remove one.
@@ -792,6 +884,7 @@
             container.one('.config-file-upload-widget').getDOMNode().click();
           }
         },
+
         /**
          * Handle the file upload click event.
          * Call onFileLoaded or onFileError if an error occurs during upload.
@@ -813,6 +906,7 @@
           container.one('.config-file-upload-overlay')
             .setContent('Remove file');
         },
+
         /**
          * Handle the file remove click event.
          * Restore the file upload widget on click.
@@ -837,6 +931,7 @@
           overlay.ancestor('.collapsible')
             .show('sizeIn', {duration: 0.25, width: null});
         },
+
         /**
          * Callback called when a file is correctly uploaded.
          * Hide the charm configuration section.
@@ -849,7 +944,7 @@
           this.configFileContent = evt.target.result;
 
           if (!this.configFileContent) {
-            // Some file read errors don't go through the error handler as
+            // Some file read errors do not go through the error handler as
             // expected but instead return an empty string.  Warn the user if
             // this happens.
             var db = this.get('db');
@@ -863,6 +958,7 @@
           }
           this.get('container').one('.charm-settings').hide();
         },
+
         /**
          * Callback called when an error occurs during file upload.
          * Hide the charm configuration section.
@@ -897,9 +993,11 @@
           }
           return;
         },
+
         /**
          * Fires an event indicating that the charm panel should switch to the
-         * "charms" search result view.
+         * "charms" search result view. Called upon clicking the "Cancel"
+         * button.
          *
          * @method goBack
          * @param {Object} ev An event object (with a "halt" method).
@@ -909,6 +1007,14 @@
           ev.halt();
           this.fire('changePanel', { name: 'charms' });
         },
+
+        /**
+         * Called upon clicking the "Confirm" button.
+         *
+         * @method onCharmDeployClicked
+         * @param {Object} ev An event object (with a "halt" method).
+         * @return {undefined} Sends a signal only.
+         */
         onCharmDeployClicked: function(evt) {
           var container = this.get('container'),
               db = this.get('db'),
@@ -972,6 +1078,14 @@
               });
           this.goBack(evt);
         },
+
+        /**
+         * Setup the panel overlay.
+         *
+         * @method setupOverlay
+         * @param {Object} container The container element.
+         * @return {undefined} Side effects only.
+         */
         setupOverlay: function(container) {
           var self = this;
           container.appendChild(Y.Node.create('<div/>'))
@@ -983,7 +1097,11 @@
       });
   views.CharmConfigurationView = CharmConfigurationView;
 
-  // Creates the "_instance" object
+  /**
+   * Create the "_instance" object.
+   *
+   * @method createInstance
+   */
   function createInstance(config) {
 
     var charmStore = config.charm_store,
@@ -1030,6 +1148,11 @@
     Y.one(document.body).append(container);
     container.hide();
 
+    /**
+     * Setup the panel data.
+     *
+     * @method setPanel
+     */
     function setPanel(config) {
       var newPanel = panels[config.name];
       if (!Y.Lang.isValue(newPanel)) {
@@ -1105,7 +1228,7 @@
     subscriptions.push(Y.on('afterPageSizeRecalculation', function() {
       if (isPanelVisible) {
         // We need to do this both in windowresize and here because
-        // windowresize can only be fired with "on," and so we can't know
+        // windowresize can only be fired with "on," and so we cannot know
         // which handler will be fired first.
         updatePanelPosition();
       }
@@ -1159,12 +1282,18 @@
       }
     }
 
+    /**
+     * Update the panel position.
+     *
+     * This should only be called when the popup is supposed to be visible.
+     * We need to hide the popup before we calculate positions, so that it
+     * does not cause scrollbars to appear while we are calculating
+     * positions.  The temporary scrollbars can cause the calculations to
+     * be incorrect.
+     *
+     * @method updatePanelPosition
+     */
     function updatePanelPosition() {
-      // This should only be called when the popup is supposed to be visible.
-      // We need to hide the popup before we calculate positions so that it
-      // does not cause scrollbars to appear while we are calculating
-      // positions.  The temporary scrollbars can cause the calculations to be
-      // incorrect.
       container.setStyle('display', 'none');
       var pos = calculatePanelPosition();
       container.setStyle('display', 'block');
@@ -1176,6 +1305,11 @@
       }
     }
 
+    /**
+     * Calculate the panel position.
+     *
+     * @method calculatePanelPosition
+     */
     function calculatePanelPosition() {
       var headerBox = Y.one('#charm-search-trigger-container'),
           dimensions = utils.getEffectiveViewportSize();
@@ -1200,12 +1334,18 @@
       subscriptions.push(searchField.on('keydown', handleKeyDown));
     }
 
-    // The public methods
+    // The public methods.
     return {
       hide: hide,
       toggle: toggle,
       show: show,
       node: container,
+
+      /**
+       * Set the default charm series in the search and description panels.
+       *
+       * @method setDefaultSeries
+       */
       setDefaultSeries: function(series) {
         charmsSearchPanel.set('defaultSeries', series);
         descriptionPanel.set('defaultSeries', series);
@@ -1213,14 +1353,26 @@
     };
   }
 
-  // The public methods
+  // The public methods.
   views.CharmPanel = {
+
+    /**
+     * Get the instance, creating it if it does not yet exist.
+     *
+     * @method getInstance
+     */
     getInstance: function(config) {
       if (!_instance) {
         _instance = createInstance(config);
       }
       return _instance;
     },
+
+    /**
+     * Destroy the instance and its node, detaching all subscriptions.
+     *
+     * @method getInstance
+     */
     killInstance: function() {
       while (subscriptions.length) {
         var sub = subscriptions.pop();

=== modified file 'undocumented'
--- undocumented	2013-01-31 00:02:54 +0000
+++ undocumented	2013-02-07 17:04:21 +0000
@@ -66,28 +66,6 @@
 app/views/utils.js:132 "native"
 app/views/utils.js:795 "value"
 app/views/environment.js:24 "initializer"
-app/views/charm-panel.js:1168 "calculatePanelPosition"
-app/views/charm-panel.js:476 "initializer"
-app/views/charm-panel.js:250 "render"
-app/views/charm-panel.js:901 "onCharmDeployClicked"
-app/views/charm-panel.js:976 "createInstance"
-app/views/charm-panel.js:764 "hideDescription"
-app/views/charm-panel.js:1022 "setPanel"
-app/views/charm-panel.js:332 "showConfiguration"
-app/views/charm-panel.js:1151 "updatePanelPosition"
-app/views/charm-panel.js:1213 "killInstance"
-app/views/charm-panel.js:655 "render"
-app/views/charm-panel.js:1198 "setDefaultSeries"
-app/views/charm-panel.js:195 "mouseleave"
-app/views/charm-panel.js:417 "_showErrors"
-app/views/charm-panel.js:651 "initializer"
-app/views/charm-panel.js:480 "render"
-app/views/charm-panel.js:751 "showDescription"
-app/views/charm-panel.js:725 "_moveTooltip"
-app/views/charm-panel.js:209 "initializer"
-app/views/charm-panel.js:964 "setupOverlay"
-app/views/charm-panel.js:192 "mouseenter"
-app/views/charm-panel.js:1207 "getInstance"
 app/views/charm.js:32 "render"
 app/views/charm.js:161 "on_results_change"
 app/views/charm.js:118 "render"