rebaseline_server: cleanup of HTML/Javascript

No behavioral change at all.
(SkipBuildbotRuns)

R=borenet@google.com

Review URL: https://codereview.chromium.org/31583007

git-svn-id: http://skia.googlecode.com/svn/trunk@11946 2bbb7eff-a529-9590-31e7-b0007b416f81

3 files changed, 187 insertions, 65 deletions

diff --git a/gm/rebaseline_server/static/loader.js b/gm/rebaseline_server/static/loader.js
index 09b66d5546..5af0e24c2b 100644
--- a/gm/rebaseline_server/static/loader.js
+++ b/gm/rebaseline_server/static/loader.js
@@ -8,6 +8,7 @@ var Loader = angular.module(
     []
 );
 
+
 // 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 @@ Loader.filter(
       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.filter(
   }
 );
 
+
 Loader.controller(
   'Loader.Controller',
     function($scope, $http, $filter, $location) {
@@ -39,6 +43,11 @@ Loader.controller(
     $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 @@ Loader.controller(
           $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 @@ Loader.controller(
       }
     );
 
-    $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;
-    }
 
-    // TODO(epoger): Rather than maintaining these as hard-coded
-    // 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;
-    }
+    //
+    // Tab operations.
+    //
 
+    /**
+     * 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,31 @@ Loader.controller(
       $scope.numResultsPerTab[newTab] += numItems;
     }
 
+
+    //
+    // updateResults() and friends.
+    //
+
+    /**
+     * Set $scope.areUpdatesPending (to enable/disable the Update Results
+     * button).
+     *
+     * TODO(epoger): We could reduce the amount of code by just setting the
+     * variable directly (from, e.g., a button's ng-click handler).  But when
+     * I tried that, the HTML elements depending on the variable did not get
+     * updated.
+     * It turns out that this is due to variable scoping within an ng-repeat
+     * element; see http://stackoverflow.com/questions/15388344/behavior-of-assignment-expression-invoked-by-ng-click-within-ng-repeat
+     *
+     * @param val boolean value to set $scope.areUpdatesPending to
+     */
+    $scope.setUpdatesPending = function(val) {
+      $scope.areUpdatesPending = val;
+    }
+
+    /**
+     * 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 +208,24 @@ Loader.controller(
             $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 +275,90 @@ Loader.controller(
         $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();
+    }
+
   }
 );
diff --git a/gm/rebaseline_server/static/view.css b/gm/rebaseline_server/static/view.css
index e4eeb0f5a3..f848220325 100644
--- a/gm/rebaseline_server/static/view.css
+++ b/gm/rebaseline_server/static/view.css
@@ -1,6 +1,32 @@
+/* Special alert areas at the top of the page. */
+.todo-div {
+    background-color: #bbffbb;
+}
+.warning-div {
+    background-color: #ffbb00;
+}
+
+/* Tab which has been selected. */
 .tab-true {
     background-color: #ccccff;
+    display: inline-block;
+    font-size: 20px;
 }
+/* All other tabs. */
 .tab-false {
     background-color: #8888ff;
+    display: inline-block;
+    font-size: 20px;
+}
+/* Spacers between tabs. */
+.tab-spacer {
+    display: inline-block;
+}
+/* The main working area (connected to the selected tab). */
+.tab-main {
+    background-color: #ccccff;
+}
+
+.update-results-button {
+    font-size: 30px;
 }
diff --git a/gm/rebaseline_server/static/view.html b/gm/rebaseline_server/static/view.html
index 9bb01fc0c5..292386b190 100644
--- a/gm/rebaseline_server/static/view.html
+++ b/gm/rebaseline_server/static/view.html
@@ -16,13 +16,13 @@
 
   <div ng-hide="!categories"><!-- everything: hide until data is loaded -->
 
-    <div ng-hide="!(header.isEditable && header.isExported)"
-         style="background-color:#ffbb00">
+    <div class="warning-div"
+         ng-hide="!(header.isEditable && header.isExported)">
       WARNING!  These results are editable and exported, so any user
       who can connect to this server over the network can modify them.
     </div>
 
-    <div style="background-color:#bbffbb"><!-- TODOs -->
+    <div class="todo-div"><!-- TODOs -->
       <p>
       TODO(epoger):
       <input type="checkbox" name="showTodosCheckbox" value="true"
@@ -59,23 +59,22 @@
       Results current as of {{localTimeString(header.timeUpdated)}}
     </div>
 
-    <div style="font-size:20px"><!-- tabs -->
-      <div ng-repeat="tab in tabs"
-           style="display:inline-block">
+    <div><!-- tabs -->
+      <div class="tab-spacer" ng-repeat="tab in tabs">
         <div class="tab-{{tab == viewingTab}}"
-             style="display:inline-block"
              ng-click="setViewingTab(tab)">
           &nbsp;{{tab}} ({{numResultsPerTab[tab]}})&nbsp;
         </div>
-        <div style="display:inline-block">
+        <div class="tab-spacer">
           &nbsp;
         </div>
       </div>
     </div><!-- tabs -->
 
-    <div class="tab-true"><!-- display of current tab -->
+    <div class="tab-main"><!-- main display area of selected tab -->
 
     <br>
+    <!-- We only show the filters/settings table on the Unfiled tab. -->
     <table ng-hide="viewingTab != defaultTab" border="1">
     <tr>
       <th colspan="2">
@@ -92,8 +91,8 @@
           <input type="checkbox"
                  name="resultTypes"
                  value="{{resultType}}"
-                 ng-checked="!isHiddenResultType(resultType)"
-                 ng-click="toggleHiddenResultType(resultType)">
+                 ng-checked="!isValueInSet(resultType, hiddenResultTypes)"
+                 ng-click="toggleValueInSet(resultType, hiddenResultTypes); setUpdatesPending(true)">
           {{resultType}} ({{count}})<br>
         </label>
       </td>
@@ -103,8 +102,8 @@
           <input type="checkbox"
                  name="configs"
                  value="{{config}}"
-                 ng-checked="!isHiddenConfig(config)"
-                 ng-click="toggleHiddenConfig(config)">
+                 ng-checked="!isValueInSet(config, hiddenConfigs)"
+                 ng-click="toggleValueInSet(config, hiddenConfigs); setUpdatesPending(true)">
           {{config}} ({{count}})<br>
         </label>
       </td>
@@ -124,7 +123,7 @@
                  maxlength="4"/>
         </td></tr>
         <tr><td>
-          <button style="font-size:30px"
+          <button class="update-results-button"
                   ng-click="updateResults()"
                   ng-disabled="!areUpdatesPending">
             Update Results
@@ -136,6 +135,7 @@
 
       <p>
 
+      <!-- Submission UI that we only show in the Pending Approval tab. -->
       <div ng-hide="'Pending Approval' != viewingTab">
         <div style="display:inline-block">
           <button style="font-size:20px"
@@ -182,6 +182,7 @@
 
       <table border="1">
         <tr>
+          <!-- Most column headers are displayed in a common fashion... -->
           <th ng-repeat="categoryName in ['resultType', 'builder', 'test', 'config']">
             <input type="radio"
                    name="sortColumnRadio"
@@ -190,6 +191,7 @@
                    ng-click="sortResultsBy(categoryName)">
             {{categoryName}}
           </th>
+          <!-- ... but there are a few columns where we display things differently. -->
           <th>
             <input type="radio"
                    name="sortColumnRadio"
@@ -229,11 +231,11 @@
             <input type="checkbox"
                    name="rowSelect"
                    value="{{result.index}}"
-                   ng-checked="isItemSelected(result.index)"
-                   ng-click="toggleItemSelected(result.index)">
+                   ng-checked="isValueInArray(result.index, selectedItems)"
+                   ng-click="toggleValueInArray(result.index, selectedItems)">
         </tr>
       </table>
-  </div><!-- display of current tab -->
+  </div><!-- main display area of selected tab -->
   </div><!-- everything: hide until data is loaded -->
 
   <!-- TODO(epoger): Can we get the base URLs (commondatastorage and