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

chrome/browser/resources/local_ntp/most_visited_single.js

 /* Copyright 2015 The Chromium Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file. */
 
  // Single iframe for NTP tiles.
 (function() {
 'use strict';
 
 
 /**
@@ skipping 240 matching lines @@
   // Store the tiles on the current closure.
   var cur = tiles;
 
   // Create empty tiles until we have NUMBER_OF_TILES.
   while (cur.childNodes.length < NUMBER_OF_TILES) {
     addTile({});
   }
 
   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;
     cur.addEventListener('webkitTransitionEnd', function(ev) {
       if (ev.target === cur) {
         removeAllOldTiles();
       }
     });
   }
 
   // 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');
 
   if (impressionUrl) {
     navigator.sendBeacon(impressionUrl);
     impressionUrl = null;
   }
 };
 
@@ skipping 319 matching lines @@
     var html = document.querySelector('html');
     html.dir = 'rtl';
   }
 
   window.addEventListener('message', handlePostMessage);
 };
 
 
 window.addEventListener('DOMContentLoaded', init);
 })();