rebaseline_server: cleanup of HTML/Javascript

gm/rebaseline_server/static/loader.js

Index: gm/rebaseline_server/static/loader.js
===================================================================
--- gm/rebaseline_server/static/loader.js	(revision 11924)
+++ gm/rebaseline_server/static/loader.js	(working copy)
@@ -8,6 +8,7 @@
     []
 );
 
+
 // TODO(epoger): Combine ALL of our filtering operations (including
 // truncation) into this one filter, so that runs most efficiently?
 // (We would have to make sure truncation still took place after
@@ -20,6 +21,8 @@
       var filteredItems = [];
       for (var i = 0; i < unfilteredItems.length; i++) {
         var item = unfilteredItems[i];
+	// For performance, we examine the "set" objects directly rather
+	// than calling $scope.isValueInSet().
         if (!(true == hiddenResultTypes[item.resultType]) &&
             !(true == hiddenConfigs[item.config]) &&
             (viewingTab == item.tab)) {
@@ -31,6 +34,7 @@
   }
 );
 
+
 Loader.controller(
   'Loader.Controller',
     function($scope, $http, $filter, $location) {
@@ -39,6 +43,11 @@
     $scope.loadingMessage = "Loading results of type '" + resultsToLoad +
         "', please wait...";
 
+    /**
+     * On initial page load, load a full dictionary of results.
+     * Once the dictionary is loaded, unhide the page elements so they can
+     * render the data.
+     */
     $http.get("/results/" + resultsToLoad).success(
       function(data, status, header, config) {
         $scope.loadingMessage = "Processing data, please wait...";
@@ -74,13 +83,16 @@
           $scope.testData[i].tab = $scope.defaultTab;
         }
 
+	// Arrays within which the user can toggle individual elements.
+        $scope.selectedItems = [];
+
+	// Sets within which the user can toggle individual elements.
         $scope.hiddenResultTypes = {
           'failure-ignored': true,
           'no-comparison': true,
           'succeeded': true,
         };
         $scope.hiddenConfigs = {};
-        $scope.selectedItems = [];
 
         $scope.updateResults();
         $scope.loadingMessage = "";
@@ -94,59 +106,21 @@
       }
     );
 
-    $scope.isItemSelected = function(index) {
-      return (-1 != $scope.selectedItems.indexOf(index));
-    }
-    $scope.toggleItemSelected = function(index) {
-      var i = $scope.selectedItems.indexOf(index);
-      if (-1 == i) {
-        $scope.selectedItems.push(index);
-      } else {
-        $scope.selectedItems.splice(i, 1);
-      }
-      // unlike other toggle methods below, does not set
-      // $scope.areUpdatesPending = true;
-    }
 
-    $scope.isHiddenResultType = function(thisResultType) {
-      return (true == $scope.hiddenResultTypes[thisResultType]);
-    }
-    $scope.toggleHiddenResultType = function(thisResultType) {
-      if (true == $scope.hiddenResultTypes[thisResultType]) {
-        delete $scope.hiddenResultTypes[thisResultType];
-      } else {
-        $scope.hiddenResultTypes[thisResultType] = true;
-      }
-      $scope.areUpdatesPending = true;
-    }
+    //
+    // Tab operations.
+    //
 
-    // TODO(epoger): Rather than maintaining these as hard-coded

[epoger, 2013/10/23 20:10:31]

Combined these functions into generic versions and moved them near the bottom of
this file.

-    // variants of isHiddenResultType and toggleHiddenResultType, we
-    // should create general-purpose functions that can work with ANY
-    // category.
-    // But for now, I wanted to see this working. :-)
-    $scope.isHiddenConfig = function(thisConfig) {
-      return (true == $scope.hiddenConfigs[thisConfig]);
-    }
-    $scope.toggleHiddenConfig = function(thisConfig) {
-      if (true == $scope.hiddenConfigs[thisConfig]) {
-        delete $scope.hiddenConfigs[thisConfig];
-      } else {
-        $scope.hiddenConfigs[thisConfig] = true;
-      }
-      $scope.areUpdatesPending = true;
-    }
-
+    /**
+     * Change the selected tab.
+     *
+     * @param tab (string): name of the tab to select
+     */
     $scope.setViewingTab = function(tab) {
       $scope.viewingTab = tab;
       $scope.updateResults();
     }
 
-    $scope.localTimeString = function(secondsPastEpoch) {
-      var d = new Date(secondsPastEpoch * 1000);
-      return d.toString();
-    }
-
     /**
      * Move the items in $scope.selectedItems to a different tab,
      * and then clear $scope.selectedItems.
@@ -177,6 +151,24 @@
       $scope.numResultsPerTab[newTab] += numItems;
     }
 
+
+    //
+    // updateResults() and friends.
+    //
+
+    /**
+     * Set $scope.areUpdatesPending (to enable/disable the Update Results
+     * button).
+     *
+     * @param val boolean value to set $scope.areUpdatesPending to
+     */
+    $scope.setUpdatesPending = function(val) {
+      $scope.areUpdatesPending = val;
+    }

[borenet, 2013/10/23 20:31:46]

Why is this helper function necessary?

[epoger, 2013/10/24 15:37:17]

Great question.  I didn't know why, but by asking you forced me to actually
learn something.  Added a TODO explaining why.

[borenet, 2013/10/28 12:57:09]

Thanks for explaining. I don't think this is a TODO though, unless you have a
workaround in mind. I think it's fine to leave as-is.

+
+    /**
+     * Update the displayed results, based on filters/settings.
+     */
     $scope.updateResults = function() {
       $scope.displayLimit = $scope.displayLimitPending;
       // TODO(epoger): Every time we apply a filter, AngularJS creates
@@ -209,14 +201,24 @@
             $scope.filteredTestData, $scope.displayLimit);
       }
       $scope.imageSize = $scope.imageSizePending;
-      $scope.areUpdatesPending = false;
+      $scope.setUpdatesPending(false);
     }
 
+    /**
+     * Re-sort the displayed results.
+     *
+     * @param sortColumn (string): name of the column to sort on
+     */
     $scope.sortResultsBy = function(sortColumn) {
       $scope.sortColumn = sortColumn;
       $scope.updateResults();
     }
 
+
+    //
+    // Operations for sending info back to the server.
+    //
+
     /**
      * Tell the server that the actual results of these particular tests
      * are acceptable.
@@ -266,5 +268,90 @@
         $scope.submitPending = false;
       });
     }
+
+
+    //
+    // Operations we use to mimic Set semantics, in such a way that
+    // checking for presence within the Set is as fast as possible.
+    // But getting a list of all values within the Set is not necessarily
+    // possible.
+    // TODO(epoger): move into a separate .js file?
+    //
+
+    /**
+     * Returns true if value "value" is present within set "set".
+     *
+     * @param value a value of any type
+     * @param set an Object which we use to mimic set semantics
+     *        (this should make isValueInSet faster than if we used an Array)
+     */
+    $scope.isValueInSet = function(value, set) {
+      return (true == set[value]);
+    }
+
+    /**
+     * If value "value" is already in set "set", remove it; otherwise, add it.
+     *
+     * @param value a value of any type
+     * @param set an Object which we use to mimic set semantics
+     */
+    $scope.toggleValueInSet = function(value, set) {
+      if (true == set[value]) {
+        delete set[value];
+      } else {
+        set[value] = true;
+      }
+    }
+
+
+    //
+    // Array operations; similar to our Set operations, but operate on a
+    // Javascript Array so we *can* easily get a list of all values in the Set.
+    // TODO(epoger): move into a separate .js file?
+    //
+
+    /**
+     * Returns true if value "value" is present within array "array".
+     *
+     * @param value a value of any type
+     * @param array a Javascript Array
+     */
+    $scope.isValueInArray = function(value, array) {
+      return (-1 != array.indexOf(value));
+    }
+
+    /**
+     * If value "value" is already in array "array", remove it; otherwise,
+     * add it.
+     *
+     * @param value a value of any type
+     * @param array a Javascript Array
+     */
+    $scope.toggleValueInArray = function(value, array) {
+      var i = array.indexOf(value);
+      if (-1 == i) {
+        array.push(value);
+      } else {
+        array.splice(i, 1);
+      }
+    }
+
+
+    //
+    // Miscellaneous utility functions.
+    // TODO(epoger): move into a separate .js file?
+    //
+
+    /**
+     * Returns a human-readable (in local time zone) time string for a
+     * particular moment in time.
+     *
+     * @param secondsPastEpoch (numeric): seconds past epoch in UTC
+     */
+    $scope.localTimeString = function(secondsPastEpoch) {
+      var d = new Date(secondsPastEpoch * 1000);
+      return d.toString();
+    }
+
   }
 );