rebaseline-server: sorting of columns (asc/desc) in frontend.

Rebaseline-server: sorting of columns in asc/desc order in frontend.

NOTRY=true

BUG=skia:1907
Committed: https://skia.googlesource.com/skia/+/d9bf7dbf0931a626d3f685163fb12913c58f48b8

[stephana, 2014-08-07 13:47:02]

The CQ bit was checked by stephana@google.com

[commit-bot: I haz the power, 2014-08-07 13:47:21]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/stephana@google.com/449843002/1

[epoger, 2014-08-07 13:58:29]

Seems pretty good, a few comments.  Grab me if you'd like to discuss live.

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
File gm/rebaseline_server/static/loader.js (right):

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
gm/rebaseline_server/static/loader.js:670: if ((subdict ===
$scope.sortColumnSubdict) && ($scope.sortColumnKey === key)) {
Maybe add comment, like...

// If we are already sorting by this column, toggle the sort order.

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
gm/rebaseline_server/static/loader.js:712: };
Nit: I see that we don't have semicolons at the end of other function
declarations.  Does it matter one way or the other?

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
File gm/rebaseline_server/static/view.css (right):

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
gm/rebaseline_server/static/view.css:93: 
Is there a reason for all the whitespace?  If so, please add a comment
indicating that reason.

[epoger, 2014-08-07 14:04:08]

The CQ bit was unchecked by epoger@google.com

[epoger, 2014-08-07 14:04:35]

On 2014/08/07 14:04:08, epoger wrote:
> The CQ bit was unchecked by mailto:epoger@google.com

I unchecked the box... for now, we should wait until CLs are approved before
checking the box.

[stephana, 2014-08-07 15:50:30]

On 2014/08/07 14:04:35, epoger wrote:
> On 2014/08/07 14:04:08, epoger wrote:
> > The CQ bit was unchecked by mailto:epoger@google.com
> 
> I unchecked the box... for now, we should wait until CLs are approved before
> checking the box.

Do I also need to add these changes into live-* files ?

[stephana, 2014-08-07 15:52:22]

Addressed issues raised in CR.

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
File gm/rebaseline_server/static/loader.js (right):

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
gm/rebaseline_server/static/loader.js:670: if ((subdict ===
$scope.sortColumnSubdict) && ($scope.sortColumnKey === key)) {
On 2014/08/07 13:58:29, epoger wrote:
> Maybe add comment, like...
> 
> // If we are already sorting by this column, toggle the sort order.

Done.

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
gm/rebaseline_server/static/loader.js:712: };
On 2014/08/07 13:58:29, epoger wrote:
> Nit: I see that we don't have semicolons at the end of other function
> declarations.  Does it matter one way or the other?

Well they should be at the other functions as well. 
In this particular case you are actually making an assignment (versus declaring
a function). Assignments should end with a semicolon. 
(see
http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#Semicolons)

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
File gm/rebaseline_server/static/view.css (right):

https://codereview.chromium.org/449843002/diff/1/gm/rebaseline_server/static/...
gm/rebaseline_server/static/view.css:93: 
On 2014/08/07 13:58:29, epoger wrote:
> Is there a reason for all the whitespace?  If so, please add a comment
> indicating that reason.

Done.

[epoger, 2014-08-07 22:09:20]

Hey Stephan-

Is this ready for me to review again as of patchset 3?

(I don't get notified when you upload new patchsets to the CL... I only get
notified when you "Publish+Mail Comments".)

[stephana, 2014-08-08 13:27:03]

On 2014/08/07 22:09:20, epoger wrote:
> Hey Stephan-
> 
> Is this ready for me to review again as of patchset 3?
> 
> (I don't get notified when you upload new patchsets to the CL... I only get
> notified when you "Publish+Mail Comments".)

Yes please review.

[epoger, 2014-08-08 13:53:57]

lgtm

[stephana, 2014-08-08 14:00:39]

The CQ bit was checked by stephana@google.com

[commit-bot: I haz the power, 2014-08-08 14:00:46]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/stephana@google.com/449843002/40001

[commit-bot: I haz the power, 2014-08-08 14:00:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-08 14:00:55]

Failed to apply patch for gm/rebaseline_server/static/live-loader.js:
While running git apply --index -p1;
  <stdin>:10: trailing whitespace.
      var currSortAsc = true; 
  error: patch failed: gm/rebaseline_server/static/live-loader.js:131
  error: gm/rebaseline_server/static/live-loader.js: patch does not apply

Patch:       gm/rebaseline_server/static/live-loader.js
Index: gm/rebaseline_server/static/live-loader.js
diff --git a/gm/rebaseline_server/static/live-loader.js
b/gm/rebaseline_server/static/live-loader.js
index
f171ff0fe91331c200c6264e791acc6505cb4759..e744c975dd2335389c84e96d27422942cbace3c6
100644
--- a/gm/rebaseline_server/static/live-loader.js
+++ b/gm/rebaseline_server/static/live-loader.js
@@ -131,6 +131,9 @@ Loader.controller(
     $scope.setBDir = $location.search().setBDir;
     $scope.loadingMessage = "please wait...";
 
+    var currSortAsc = true; 
+
+
     /**
      * On initial page load, load a full dictionary of results.
      * Once the dictionary is loaded, unhide the page elements so they can
@@ -171,8 +174,11 @@ Loader.controller(
           $scope.orderedColumnNames =
data[constants.KEY__ROOT__EXTRACOLUMNORDER];
           $scope.imagePairs = data[constants.KEY__ROOT__IMAGEPAIRS];
           $scope.imageSets = data[constants.KEY__ROOT__IMAGESETS];
+
+          // set the default sort column and make it ascending.
           $scope.sortColumnSubdict = constants.KEY__IMAGEPAIRS__DIFFERENCES;
           $scope.sortColumnKey = constants.KEY__DIFFERENCES__PERCEPTUAL_DIFF;
+          currSortAsc = true;
 
           $scope.showSubmitAdvancedSettings = false;
           $scope.submitAdvancedSettings = {};
@@ -604,14 +610,7 @@ Loader.controller(
       // array copies?  (For better performance.)
 
       if ($scope.viewingTab == $scope.defaultTab) {
-
-        // TODO(epoger): Until we allow the user to reverse sort order,
-        // there are certain columns we want to sort in a different order.
-        var doReverse = (
-            ($scope.sortColumnKey ==
-             constants.KEY__DIFFERENCES__PERCENT_DIFF_PIXELS) ||
-            ($scope.sortColumnKey ==
-             constants.KEY__DIFFERENCES__PERCEPTUAL_DIFF));
+        var doReverse = !currSortAsc;
 
         $scope.filteredImagePairs =
             $filter("orderBy")(

[stephana, 2014-08-08 14:20:23]

The CQ bit was checked by stephana@google.com

[commit-bot: I haz the power, 2014-08-08 14:20:54]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/stephana@google.com/449843002/60001

[commit-bot: I haz the power, 2014-08-08 14:21:07]

Change committed as d9bf7dbf0931a626d3f685163fb12913c58f48b8