[NTP4] Auto-deletion of empty apps panes.

chrome/browser/resources/ntp4/tile_page.js

 // Copyright (c) 2011 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('ntp4', function() {
   'use strict';
 
   // We can't pass the currently dragging tile via dataTransfer because of
   // http://crbug.com/31037
   var currentlyDraggingTile = null;
@@ skipping 272 matching lines @@
      */
     onDragCloneTransitionEnd_: function(e) {
       if (this.classList.contains('dragging') &&
           (e.propertyName == 'left' || e.propertyName == 'top' ||
            e.propertyName == '-webkit-transform')) {
         this.finalizeDrag_();
       }
     },
 
     /**
-     * Called when an app is removed from Chrome. Animates its disappearance.
+     * Called when a tile is removed from a tilePage. Animates its disappearance
+     * if |opt_animate| is truthy.
+     * @param {=boolean} opt_animate Whether the removal should be animated.
+     * @param {=boolean} opt_dontDelete Whether we should auto-delete an empty
+     *     apps page if the last tile is removed.
+     * @return {Element} The tile being removed.
      */
-    doRemove: function() {
-      this.firstChild.classList.add('removing-tile-contents');
+    remove: function(opt_animate, opt_dontDelete) {
+      var owningPage = this.tilePage;
+      if (opt_animate)
+        owningPage.classList.add('animating-tile-page');
+      this.parentNode.removeChild(this);
+      owningPage.calculateLayoutValues_();
+      owningPage.cleanupDrag();
+      if (owningPage instanceof ntp4.AppsPage && !opt_dontDelete)

[Evan Stade, 2011/12/02 00:53:24]

don't do this, use inheritance or whatever (same applies to all the other places
you used instanceof)

[Dan Beam, 2011/12/02 01:25:11]

So do something like .removeIfEmpty() on TilePage and override for just
AppsPage?  That's cool with me.

Does this mean that I should also fix the other occurrences of the use of this
operator, like:

  if (typeof ntp4.MostVisitedPage != 'undefined' &&
      page instanceof ntp4.MostVisitedPage) {
    assert(this.tilePages.length == 1,
           'MostVisitedPage should be added as first tile page');

or

  var animate = page instanceof ntp4.AppsPage &&
                page.classList.contains('temporary');

to be something like

  if (page.shouldBeFirst())
    assert(this.tilePages.length == 1);

and

  var animate = page.shouldAnimateInsertion();

?

[Evan Stade, 2011/12/02 02:39:49]

no, I just meant in this context. Superclass is not supposed to know about
subclasses.

[Dan Beam, 2011/12/05 18:07:13]

Done. (still ironing out the issues with this small re-design)

+        ntp4.removeAppsPageIfEmpty(owningPage, opt_animate);
+      return this;
     },
 
     /**
      * Callback for the webkitAnimationEnd event on the tile's contents.
      * @param {Event} e The event object.
      */
     onContentsAnimationEnd_: function(e) {
       if (this.firstChild.classList.contains('new-tile-contents'))
         this.firstChild.classList.remove('new-tile-contents');
       if (this.firstChild.classList.contains('removing-tile-contents'))
-        this.tilePage.removeTile(this, true);
+        this.remove(true);
     },
   };
 
   /**
    * Gives the proportion of the row width that is devoted to a single icon.
    * @param {number} rowTileCount The number of tiles in a row.
    * @param {number} tileSpacingFraction The proportion of the tile width which
    *     will be used as spacing between tiles.
    * @return {number} The ratio between icon width and row width.
    */
@@ skipping 173 matching lines @@
     /**
      * Returns any extra padding to insert to the bottom of a tile page.  By
      * default there is none, but subclasses can override.
      * @type {number}
      */
     get extraBottomPadding() {
       return 0;
     },
 
     /**
+     * Removes the tilePage from the DOM and cleans up event handlers.
+     */
+    remove: function() {
+      // This checks arguments.length as most remove functions have a boolean
+      // |opt_animate| argument, but that's not necesarilly applicable to
+      // removing a tilePage. Selecting a different card in an animated way and
+      // deleting the card afterward is probably a better choice.
+      assert(typeof arguments[0] != 'boolean',
+             'This function takes no |opt_animate| argument.');
+      assert(this.tileCount == 0, 'Only removal of an empty page is allowed.');
+      this.tearDown_();
+      this.parentNode.removeChild(this);
+    },
+
+    /**
      * Cleans up resources that are no longer needed after this TilePage
      * instance is removed from the DOM.
+     * @private
      */
-    tearDown: function() {
+    tearDown_: function() {
       this.eventTracker.removeAll();
     },
 
     /**
      * Appends a tile to the end of the tile grid.
      * @param {HTMLElement} tileElement The contents of the tile.
      * @param {?boolean} animate If true, the append will be animated.
      * @protected
      */
     appendTile: function(tileElement, animate) {
@@ skipping 19 matching lines @@
         this.tileGrid_.insertBefore(wrapperDiv,
                                     this.tileElements_[index]);
       }
       this.calculateLayoutValues_();
       this.heightChanged_();
 
       this.positionTile_(index);
     },
 
     /**
-     * Removes the given tile and animates the respositioning of the other
-     * tiles.
-     * @param {HTMLElement} tile The tile to remove from |tileGrid_|.
-     * @param {?boolean} animate If true, remaining tiles will animate.
-     */
-    removeTile: function(tile, animate) {
-      if (animate)
-        this.classList.add('animating-tile-page');
-      var index = tile.index;
-      tile.parentNode.removeChild(tile);
-      this.calculateLayoutValues_();
-      this.cleanupDrag();
-    },
-
-    /**
      * Removes all tiles from the page.
      */
     removeAllTiles: function() {
       this.tileGrid_.innerHTML = '';
     },
 
     /**
      * Called when the page is selected (in the card selector).
      * @param {Event} e A custom cardselected event.
      * @private
@@ skipping 542 matching lines @@
             (index > this.dragItemIndex_ ? 1 : 0);
         if (this.withinPageDrag_) {
           this.tileGrid_.insertBefore(
               currentlyDraggingTile,
               this.tileElements_[adjustedIndex]);
           this.tileMoved(currentlyDraggingTile, this.dragItemIndex_);
         } else {
           var originalPage = currentlyDraggingTile ?
               currentlyDraggingTile.tilePage : null;
           this.addDragData(e.dataTransfer, adjustedIndex);
-          if (originalPage)
+          if (originalPage) {
             originalPage.cleanupDrag();
+            if (originalPage instanceof ntp4.AppsPage)
+              ntp4.removeAppsPageIfEmpty(originalPage, true);
+          }
         }
 
         // Dropping the icon may cause topMargin to change, but changing it
         // now would cause everything to move (annoying), so we leave it
         // alone. The top margin will be re-calculated next time the window is
         // resized or the page is selected.
       }
 
       this.classList.remove('animating-tile-page');
       this.cleanupDrag();
     },
 
     /**
      * Appends the currently dragged tile to the end of the page. Called
      * from outside the page, e.g. when dropping on a nav dot.
      */
     appendDraggingTile: function() {
       var originalPage = currentlyDraggingTile.tilePage;
       if (originalPage == this)
         return;
 
       this.addDragData(null, this.tileElements_.length);
-      originalPage.cleanupDrag();
+      if (originalPage instanceof ntp4.AppsPage)
+        ntp4.removeAppsPageIfEmpty(originalPage, true);
+      if (originalPage)
+        originalPage.cleanupDrag();
     },
 
     /**
      * Makes sure all the tiles are in the right place after a drag is over.
      */
     cleanupDrag: function() {
       this.repositionTiles_(currentlyDraggingTile);
       // Remove the drag mask.
       this.updateMask_();
     },
@@ skipping 74 matching lines @@
       assert(false);
     },
   };
 
   return {
     getCurrentlyDraggingTile: getCurrentlyDraggingTile,
     setCurrentDropEffect: setCurrentDropEffect,
     TilePage: TilePage,
   };
 });