NTP5: Reuse insertion/removal animations for Apps.

chrome/browser/resources/ntp_search/tile_page.js

Index: chrome/browser/resources/ntp_search/tile_page.js
diff --git a/chrome/browser/resources/ntp_search/tile_page.js b/chrome/browser/resources/ntp_search/tile_page.js
index a55d612a4a0345ef52d5c706995104da9c04df33..48b71b6012a2bbb813703c45cf0625c80a874bdb 100644
--- a/chrome/browser/resources/ntp_search/tile_page.js
+++ b/chrome/browser/resources/ntp_search/tile_page.js
@@ -480,10 +480,15 @@ cr.define('ntp', function() {
      * number of tiles has changed, and finally will call |updateTiles_| which
      * will in turn render the individual tiles.
      * @param {Array} dataList The array of data.
+     * @param {boolean=} opt_create Whether to create tiles.
      */
-    setDataList: function(dataList) {
+    setDataList: function(dataList, opt_create) {
       this.dataList_ = dataList.slice(0, this.config.maxTileCount);
 
+      opt_create = typeof opt_create == 'undefined' ? true : opt_create;
+      if (!opt_create)

[Dan Beam, 2012/12/05 18:40:44]

why isn't this simply 2 methods? `set dataList` and createTiles()?

[pedro (no code reviews), 2012/12/05 20:03:55]

For simplicity reasons, I'm trying to share as much code as possible between
Most Visited and Apps, and the Most Visited logic requires the tiles to be
created any time dataList has changed. Apps Page, on the other hand does not
require that.

Please bear in mind that the NTP5 is going to be rewritten from scratch, and all
the code you're seeing now is going to be deleted. I wouldn't be too much
worried about the NTP5 code right now, because this is just for experiment
purposes.

[Dan Beam, 2012/12/05 20:05:54]

If only I had a $1 for every time I heard that ;)

+        return;
+
       var dataListLength = this.dataList_.length;
       var tileCount = this.tileCount;
       // Create or remove tiles if necessary.
@@ -846,7 +851,10 @@ cr.define('ntp', function() {
       var tileBeingRestored = createTile(this, restoredData);
       tileCells[index].appendChild(tileBeingRestored);
 
-      var extraTile = extraCell.firstChild;
+      if (this.config.scrollable)
+        this.content_.scrollTop = tileCells[index].offsetTop;
+
+      var extraTile = extraCell && extraCell.firstChild;

[Dan Beam, 2012/12/05 18:40:44]

IMO, these are slightly easier to understand for C++ devs (or anybody unaware of
|| or && returning the results of an expression in JS):

  var extraTile = extraCell ? extraCell.firstChild : null;

or

  var extraTile;
  if (extraCell && extraCell.firstChild)
    extraTile = extraCell.firstChild;

[pedro (no code reviews), 2012/12/05 20:03:55]

Done.

 
       this.executeRepositioningAnimation_(tileBeingRestored, extraTile,
           repositioningStartIndex, repositioningEndIndex, false);
@@ -947,7 +955,8 @@ cr.define('ntp', function() {
 
       // Alternate the visualization of the target and extra tiles.
       fadeTile(targetTile, !isRemoving);
-      fadeTile(extraTile, isRemoving);
+      if (extraTile)
+        fadeTile(extraTile, isRemoving);
 
       // Move tiles to the new position.
       var tiles = this.tiles_;
@@ -982,7 +991,9 @@ cr.define('ntp', function() {
         var tile = tiles[i];
         this.resetTilePosition_(tile);
         tile.style.zIndex = '';
-        tileCells[i + positionDiff].assign(tile);
+        var tileCell = tileCells[i + positionDiff];
+        if (tileCell)
+          tileCell.assign(tile);
       }
     },