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..17e00f503381482560a7b61fd61cff035e67517a 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';
+  if (isLastPage) {
+    document.getElementById('loadingIndicator').className ='hidden';
+    updateLoadingIndicator(0);
+  }
+  else {

[robliao, 2014/08/07 17:58:07]

} else {

[sunangel, 2014/08/07 20:57:57]

Done.

+    updateLoadingIndicator(1);
+    document.getElementById('loadingIndicator').className ='visible';
+  }
 }
 
 // Maps JS theme to CSS class and then changes body class name.
@@ -26,3 +32,28 @@ function useTheme(theme) {
   }
   document.body.className = cssClass;
 }
+
+function updateLoadingIndicator(isNotLastPage) {

[robliao, 2014/08/07 17:58:07]

isNotLastPage should be treated as true or false.

[sunangel, 2014/08/07 20:57:57]

Done.

+  if (isNotLastPage == 1 && this.alreadyRunning == 0) {

[robliao, 2014/08/07 17:58:07]

No need to compare to == 1 or == 0.

http://www.ecma-international.org/ecma-262/5.1/#sec-9.2

[sunangel, 2014/08/07 20:57:57]

Done.

+    var colorIndex = -1;

[robliao, 2014/08/07 17:58:07]

Move this declaration into the if (loader) block.

[sunangel, 2014/08/07 20:57:57]

Done.

+    var colors = ["red", "yellow", "green", "blue"];

[robliao, 2014/08/07 17:58:07]

Put this in a private scope.

var updateLoadingIndicator = function() {
  var colors = ["red", "yellow", "green", "blue"];
  return function(isNotLastPage) {
    if (isNotLastPage ...)
  };
}();

[sunangel, 2014/08/07 20:57:57]

Wow did not know you could do scoping in javascript.
Thanks!
On 2014/08/07 17:58:07, robliao wrote:

+    var loader = document.getElementById("loader");
+    if(loader) {

[robliao, 2014/08/07 17:58:07]

if (loader)

[sunangel, 2014/08/07 20:57:57]

Done.

+      this.alreadyRunning = 1;
+      this.colorShuffle = setInterval(function() {
+        colorIndex = (colorIndex + 1) % colors.length;
+        loader.className = colors[colorIndex];
+      }, 600);
+    }
+  }
+  else if (isNotLastPage == 0 && this.alreadyRunning == 1) {
+      clearInterval(this.colorShuffle);

[robliao, 2014/08/07 17:58:07]

2 spaces for tabs in JS.

[sunangel, 2014/08/07 20:57:57]

Done.

+  }
+}
+
+// Sets initial state of loading indicator to be not already running.
+function init() {
+  this.alreadyRunning = 0;

[robliao, 2014/08/07 17:58:07]

Instead of alreadyRunning, you can use the existence of colorShuffle to
determine if the interval is running.

[sunangel, 2014/08/07 20:57:57]

Done.

+}
+
+window.onload = init;