NTP5: Apps regression fix.

chrome/browser/resources/ntp_search/tile_page.js

 // Copyright (c) 2012 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.
 
 cr.define('ntp', function() {
   'use strict';
 
   /**
    * The maximum gap from the edge of the scrolling area which will display
    * the shadow with transparency. After this point the shadow will become
@@ skipping 108 matching lines @@
    * place and to animate them individually. Each TileCell is associated to
    * one Tile at a time (or none if it is a filler object), and that association
    * might change when the grid is resized. When that happens, the grid is
    * updated and the Tiles are moved to the proper TileCell. We cannot move the
    * the TileCell itself during the resize because this transition is animated
    * with CSS and there's no way to stop CSS animations, and we really want to
    * animate with CSS to take advantage of hardware acceleration.
    * @constructor
    * @extends {HTMLDivElement}
    * @param {HTMLElement} tile Tile element that will be associated to the cell.
-   * @param {Object} config TilePage configuration object.
    */
-  function TileCell(tile, config) {
+  function TileCell(tile) {
     var tileCell = cr.doc.createElement('div');
     tileCell.__proto__ = TileCell.prototype;
-    tileCell.initialize(tile, config);
+    tileCell.initialize(tile);
 
     return tileCell;
   }
 
   TileCell.prototype = {
     __proto__: HTMLDivElement.prototype,
 
     /**
      * Initializes a TileCell.
      * @param {Tile} tile The Tile that will be assigned to this TileCell.
-     * @param {Object} config TilePage configuration object.
      */
-    initialize: function(tile, config) {
+    initialize: function(tile) {
       this.className = 'tile-cell';
       this.assign(tile);
     },
 
     /**
      * The index of the TileCell.
      * @type {number}
      */
     get index() {
       return Array.prototype.indexOf.call(this.tilePage.tiles_,
@@ skipping 286 matching lines @@
       this.tiles_.splice(index, 0, tile);
       this.fireAddedEvent(tile, index, false);
       this.renderGrid();
     },
 
     /**
      * Create a blank tile.
      * @protected
      */
     createTile_: function() {
-      return new this.TileClass(this.config);
+      return new this.TileClass();
     },
 
     /**
      * Create blank tiles.
      * @param {number} count The desired number of Tiles to be created. If this
      *   value the maximum value defined in |config.maxTileCount|, the maximum
      *   value will be used instead.
      * @protected
      */
     createTiles_: function(count) {
@@ skipping 164 matching lines @@
         }
 
         // For each column in the current row.
         for (var col = 0; col < colCount; col++, tile++) {
           var tileCell;
           var tileElement;
           if (tileRowTiles[col]) {
             tileCell = tileRowTiles[col];
           } else {
             var span = cr.doc.createElement('span');
-            tileCell = new TileCell(span, this.config);
+            tileCell = new TileCell(span);
           }
 
           // Render Tiles.
           if (tile < tileCount) {
             tileCell.classList.remove('filler');
             tileElement = tiles[tile];
             if (!tileCell.tile)
               tileCell.appendChild(tileElement);
             else if (tileElement != tileCell.tile)
               tileCell.replaceChild(tileElement, tileCell.tile);
@@ skipping 194 matching lines @@
      */
     animateTileRestoration: function(index, newDataList) {
       var tiles = this.tiles_;
       var tileCount = tiles.length;
 
       var tileCells = this.querySelectorAll('.tile-cell');
       var extraTileIndex = Math.min(tileCount, this.config.maxTileCount - 1);
       var extraCell = tileCells[extraTileIndex];
       var extraTileData = newDataList[extraTileIndex + 1];
 
+      var restoredData = newDataList[index];
+      var tileBeingRestored = createTile(this, restoredData);
+
+      // If the tile being restored/added is on the last position, and the
+      // tile before that is on the last column of a particular row, then we
+      // need to handle it separately once there will be no cell to put the
+      // restored tile temporarily and there will be no rearrangement of tiles.

[Dan Beam, 2012/12/14 18:28:13]

I appreciate this comment but I'm still not quite sure what it means?  Is there
a way to sum this up in less words and better for a new reader of the code?

[pedro (no code reviews), 2012/12/14 18:54:10]

I've rewritten this comment, let me know if it is better now. It's kinda hard to
describe because it is a very visual thing. I guess it makes more sense once you
see what the animation is doing (one tile is appearing with a "expand/scale"
animation while the others are being rearranged in order to give room for that
tile. But there's a corner case when the tile being added is on the very last
position of the grid, on a row that is not yet rendered.

+      if (!tileCells[index]) {
+        this.appendTile(tileBeingRestored);
+        tileBeingRestored.classList.add('target-tile');
+        fadeTile(tileBeingRestored, true);
+        return;
+      }
+
       var repositioningStartIndex = index;
       var repositioningEndIndex = tileCount - (extraTileData ? 1 : 0);
 
       this.initializeRepositioningAnimation_(index, repositioningEndIndex);
 
-      var restoredData = newDataList[index];
-      var tileBeingRestored = createTile(this, restoredData);
       tileCells[index].appendChild(tileBeingRestored);
 
       if (this.config.scrollable)
         this.content_.scrollTop = tileCells[index].offsetTop;
 
       var extraTile;
       if (extraCell)
         extraTile = extraCell.tile;
 
       this.executeRepositioningAnimation_(tileBeingRestored, extraTile,
@@ skipping 206 matching lines @@
    * Creates a new tile given a particular data. If there's no data, then
    * a tile filler will be created.
    * @param {TilePage} tilePage A TilePage.
    * @param {Object=} opt_data The data that will be used to create the tile.
    * @return {Tile} The new tile.
    */
   function createTile(tilePage, opt_data) {
     var tile;
     if (opt_data) {
       // If there's data, the new tile will be a real one (not a filler).
-      tile = new tilePage.TileClass(tilePage.config);
-      tile.data = opt_data;
+      tile = new tilePage.TileClass(opt_data);
     } else {
       // Otherwise, it will be a fake filler tile.
       tile = cr.doc.createElement('span');
       tile.className = 'tile';
     }
     return tile;
   }
 
   /**
    * Fades a tile.
    * @param {Tile} tile A Tile.
    * @param {boolean} isFadeIn Whether to fade-in the tile. If |isFadeIn| is
    * false, then the tile is going to fade-out.
    */
   function fadeTile(tile, isFadeIn) {
     var className = 'animate-hide-tile';
     tile.classList.add(className);
     if (isFadeIn) {
       // Forces a reflow to ensure that the fade-out animation will work.
       tile.scrollTop;
       tile.classList.remove(className);
     }
   }
 
   return {
     Tile: Tile,
     TilePage: TilePage,
   };
 });