Loading Indicator for Distilled Pages

components/dom_distiller/content/resources/dom_distiller_viewer.js

Index: components/dom_distiller/content/resources/dom_distiller_viewer.js
diff --git a/components/dom_distiller/content/resources/dom_distiller_viewer.js b/components/dom_distiller/content/resources/dom_distiller_viewer.js
index 94299467ef3df0b6ffa129f1ee149d008e58eec8..4b12896c0ef81301627d460df56bcfb65b39a303 100644
--- a/components/dom_distiller/content/resources/dom_distiller_viewer.js
+++ b/components/dom_distiller/content/resources/dom_distiller_viewer.js
@@ -9,8 +9,14 @@ function addToPage(html) {
 }
 
 function showLoadingIndicator(isLastPage) {
-  document.getElementById('loadingIndicator').className =
-      isLastPage ? 'hidden' : 'visible';
+  var loadingIndicatorShuffle = updateLoadingIndicator();
+  if (isLastPage) {
+    document.getElementById('loadingIndicator').className ='hidden';
+    loadingIndicatorShuffle(false);
+  } else {
+    loadingIndicatorShuffle(true);
+    document.getElementById('loadingIndicator').className ='visible';
+  }
 }
 
 // Maps JS theme to CSS class and then changes body class name.
@@ -26,3 +32,21 @@ function useTheme(theme) {
   }
   document.body.className = cssClass;
 }
+
+var updateLoadingIndicator = function() {
+  var colors = ["red", "yellow", "green", "blue"];
+  return function(isNotLastPage) {

[robliao, 2014/08/07 21:53:04]

If you inverted this to isLastPage, you can pass the argument of
showLoadingIndicator directly to here instead of a direct call of true/false.

[sunangel, 2014/08/08 00:45:30]

inverted it in function call above instead...is that okay? 
On 2014/08/07 21:53:04, robliao wrote:

[robliao, 2014/08/08 17:22:30]

Right. The point here is that this function can also accept isLastPage, making
this consistent with the above.

Then your calls above become loadingIndicatorShuffle(isLastPage) and then you
can invert your logic in this function.

[sunangel, 2014/08/08 21:25:04]

Wait hm okay sorry. The change I thought I made didn't show up -__-. I
reuploaded it.
On 2014/08/08 17:22:30, robliao wrote:

+    if (isNotLastPage && typeof this.colorShuffle == "undefined") {
+      var loader = document.getElementById("loader");
+      if (loader) {
+        var colorIndex = -1;
+        this.colorShuffle = setInterval(function() {
+        colorIndex = (colorIndex + 1) % colors.length;
+        loader.className = colors[colorIndex];
+        }, 600);
+      }
+    } else if (!isNotLastPage && typeof this.colorShuffle != "undefined") {
+      clearInterval(this.colorShuffle);
+    }
+  };
+};

[robliao, 2014/08/07 21:53:04]

Don't forget to invoke it (note the () at the end of the comment before)! Right
now updateLoading indicator points to the anonymous function with no args. We
want the one inside, so we have to call this anonymous function first.

[sunangel, 2014/08/08 00:45:30]

Done.