NTP: fade-in the tiles only when they change, not when they are shown initially

chrome/browser/resources/local_ntp/most_visited_single.js

Index: chrome/browser/resources/local_ntp/most_visited_single.js
diff --git a/chrome/browser/resources/local_ntp/most_visited_single.js b/chrome/browser/resources/local_ntp/most_visited_single.js
index e2e0911eb43557e4178920adaf9de2185fcbb191..535c3fe624a4876b38edc0ddb171b8e394bddaf0 100644
--- a/chrome/browser/resources/local_ntp/most_visited_single.js
+++ b/chrome/browser/resources/local_ntp/most_visited_single.js
@@ -258,9 +258,12 @@ var showTiles = function() {
 
   var parent = document.querySelector('#most-visited');
 
-  // Mark old tile DIV for removal after the transition animation is done.
+  // Only fade in the new tiles if there were tiles before.
+  var fadeIn = false;
   var old = parent.querySelector('#mv-tiles');
   if (old) {

[jkrcal, 2016/09/09 08:13:13]

I am puzzled:
How come this is not true initially when you added the div#mv-tiles into the
html?

[Marc Treib, 2016/09/09 08:56:28]

When this runs for the first time, there is no div#mv-tiles (it used to be in
the html, but I removed it, since there's no reason to have it there initially
as far as I can tell).

[jkrcal, 2016/09/09 09:00:17]

Huh, I've swapped the direction :) Sure, this makes sense, thanks!

+    fadeIn = true;
+    // Mark old tile DIV for removal after the transition animation is done.
     old.removeAttribute('id');
     old.classList.add('mv-tiles-old');
     old.style.opacity = 0.0;
@@ -274,11 +277,12 @@ var showTiles = function() {
   // Add new tileset.
   cur.id = 'mv-tiles';
   parent.appendChild(cur);
-  // We want the CSS transition to trigger, so need to add to the DOM before
-  // setting the style.
-  setTimeout(function() {

[Marc Treib, 2016/09/08 18:17:48]

This was the root problem: When the timeout function runs, the original style
may or may not have been applied, hence the inconsistent behavior.

-    cur.style.opacity = 1.0;
-  }, 0);
+  // getComputedStyle causes the initial style (opacity 0) to be applied, so
+  // that when we then set it to 1, that triggers the CSS transition.
+  if (fadeIn) {
+    window.getComputedStyle(cur).opacity;

[Marc Treib, 2016/09/08 18:17:48]

From
https://timtaubert.de/blog/2012/09/css-transitions-for-dynamically-created-do...
:)

[jkrcal, 2016/09/09 08:13:13]

This looks hacky :) I haven't found any cleaner solution, though. So, okay...

[Marc Treib, 2016/09/09 08:56:28]

It's not pretty, but I'd argue it's less hacky than the previous setTimeout
hack. And at least, it seems to work consistently!

[jkrcal, 2016/09/09 09:00:17]

That's true, probably less hacky.

+  }
+  cur.style.opacity = 1.0;
 
   // Make sure the tiles variable contain the next tileset we may use.
   tiles = document.createElement('div');