[NTP4] Auto-deletion of empty apps panes.

chrome/browser/resources/ntp4/page_list_view.js

Index: chrome/browser/resources/ntp4/page_list_view.js
diff --git a/chrome/browser/resources/ntp4/page_list_view.js b/chrome/browser/resources/ntp4/page_list_view.js
index a5065b12ff35217dad3cde04fba1ee714c689a1f..f64232abe3b34185e1bc1a904f7802f62e347f04 100644
--- a/chrome/browser/resources/ntp4/page_list_view.js
+++ b/chrome/browser/resources/ntp4/page_list_view.js
@@ -35,7 +35,7 @@ cr.define('ntp4', function() {
     cardSlider: undefined,
 
     /**
-     * The frame div for cardSlider.
+     * The frame div for this.cardSlider.
      * @type {!Element|undefined}
      */
     sliderFrame: undefined,
@@ -189,7 +189,7 @@ cr.define('ntp4', function() {
      * @param {string} title The title of the tile page.
      * @param {bool} titleIsEditable If true, the title can be changed.
      * @param {TilePage} opt_refNode Optional reference node to insert in front
-     * of.
+     *     of.
      * When opt_refNode is falsey, |page| will just be appended to the end of
      * the page list.
      */
@@ -197,6 +197,21 @@ cr.define('ntp4', function() {
       // When opt_refNode is falsey, insertBefore acts just like appendChild.
       this.pageList.insertBefore(page, opt_refNode);
 
+      if (!document.documentElement.classList.contains('starting-up')) {
+        // If we're inserting a card in front of the current card, add 1 to the
+        // current index to adjust for the new card in the order.
+        var index = Array.prototype.indexOf.call(this.tilePages, page);
+        if (typeof this.cardSlider.currentCard != 'undefined' &&
+            index <= this.cardSlider.currentCard) {
+          this.cardSlider.currentCard += 1;
+        }
+        if (page instanceof ntp4.AppsPage &&
+            this.shownPage == templateData['apps_page_id'] &&
+            this.getAppsPageIndex(page) <= this.shownPageIndex) {
+          this.shownPageIndex += 1;
+        }
+      }
+
       // Remember special MostVisitedPage.
       if (typeof ntp4.MostVisitedPage != 'undefined' &&
           page instanceof ntp4.MostVisitedPage) {
@@ -228,14 +243,29 @@ cr.define('ntp4', function() {
      * @param {boolean} isUninstall True if the app is being uninstalled;
      *     false if the app is being disabled.
      */

[csilv, 2011/12/02 01:57:02]

add comment documentation for 'fromPage'

[Dan Beam, 2011/12/05 18:05:10]

Done.

-    appRemoved: function(appData, isUninstall) {
+    appRemoved: function(appData, isUninstall, fromPage) {
       var app = $(appData.id);
       assert(app, 'trying to remove an app that doesn\'t exist');
 
-      if (!isUninstall)
+      if (!isUninstall) {
         app.replaceAppData(appData);
-      else
-        app.remove();
+      } else {
+        // Unset the ID immediately, because the app is already gone. But leave
+        // the tile on the page as it animates out.
+        app.id = '';
+        // If the uninstall was from this page, run the blipout animation and
+        // remove the tile in the webkitAnimationEnd handler in apps_page.js.
+        // Otherwise delete the tile without auto-deleting the page to avoid
+        // re-deleting the same page (or the page that slid in to take its
+        // place).
+        if (fromPage) {
+          app.classList.add('removing-tile-contents');
+        } else {
+          var tilePage = app.tile.tilePage;
+          app.tile.remove(false, true);
+          this.removeAppsPageIfEmpty(tilePage, false, true);
+        }
+      }
     },
 
     /**
@@ -257,19 +287,16 @@ cr.define('ntp4', function() {
       // uninstall.
       while (this.appsPages.length > 0) {
         var page = this.appsPages[0];
-        var dot = page.navigationDot;
 
         this.eventTracker.remove(page);
-        page.tearDown();
-        page.parentNode.removeChild(page);
-        dot.parentNode.removeChild(dot);
+        this.removeTilePage(page);
       }
 
       // Get the array of apps and add any special synthesized entries
       var apps = data.apps;
 
       // Get a list of page names
-      var pageNames = data.appPageNames;
+      var pageNames = data.appsPageNames;
 
       function stringListIsEmpty(list) {
         for (var i = 0; i < list.length; i++) {
@@ -338,15 +365,7 @@ cr.define('ntp4', function() {
       }
 
       var pageIndex = appData.page_index || 0;
-
-      if (pageIndex >= this.appsPages.length) {
-        while (pageIndex >= this.appsPages.length) {
-          this.appendTilePage(new ntp4.AppsPage(),
-                              localStrings.getString('appDefaultPageName'),
-                              true);
-        }
-        this.updateSliderCards();
-      }
+      this.ensureAppsPageAtIndex_(pageIndex);
 
       var page = this.appsPages[pageIndex];
       var app = $(appData.id);
@@ -363,14 +382,18 @@ cr.define('ntp4', function() {
      */
     appsPrefChangedCallback: function(data) {
       for (var i = 0; i < data.apps.length; ++i) {
-        $(data.apps[i].id).appData = data.apps[i];
+        var el = $(data.apps[i].id);
+        // There won't be an element when the temporary page is first saved on a
+        // drop on a new apps page.
+        if (el)
+          el.appData = data.apps[i];
       }
 
-      // Set the App dot names. Skip the first dot (Most Visited).
+      // Set the App dot names. Skip dots that don't belong to apps pages.
       var dots = this.dotList.getElementsByClassName('dot');
-      var start = this.mostVisitedPage ? 1 : 0;
-      for (var i = start; i < dots.length; ++i) {
-        dots[i].displayTitle = data.appPageNames[i - start] || '';
+      var firstAppDot = this.getTilePageIndex(this.appsPages[0]);
+      for (var i = 0; i < this.appsPages.length; ++i) {
+        dots[firstAppDot + i].displayTitle = data.appsPageNames[i] || '';
       }
     },
 
@@ -379,21 +402,36 @@ cr.define('ntp4', function() {
      * the Slider knows about the new elements.
      */
     updateSliderCards: function() {
-      var pageNo = Math.min(this.cardSlider.currentCard,
-                            this.tilePages.length - 1);
-      this.cardSlider.setCards(Array.prototype.slice.call(this.tilePages),
-                                                          pageNo);
+      var index = -1;
+      var prevPage = this.shownPage;
+      var prevIndex = this.shownPageIndex;
+      var tiles = Array.prototype.slice.call(this.tilePages);
+      // It doesn't hurt to clamp value this every time, even if it's only
+      // applicable to apps. It helps self-heal strange / un-expected data, i.e.
+      // going from an apps page to the most visited page directly without
+      // setting this.shownPageIndex = 0 somehow.
+      this.shownPageIndex = Math.max(0, Math.min(this.shownPageIndex,
+                                                 this.appsPages.length - 1));
       switch (this.shownPage) {
         case templateData['apps_page_id']:
-          this.cardSlider.selectCardByValue(
-              this.appsPages[Math.min(this.shownPageIndex,
-                                      this.appsPages.length - 1)]);
+          index = tiles.indexOf(this.appsPages[this.shownPageIndex]);
           break;
         case templateData['most_visited_page_id']:
-          if (this.mostVisitedPage)
-            this.cardSlider.selectCardByValue(this.mostVisitedPage);
+          index = tiles.indexOf(this.mostVisitedPage);
           break;
       }
+      // If shownPage was saved as a page that's now disabled or the shownPage
+      // doesn't exist any more, index will be -1. Change to the preferred
+      // default page (first apps pane) in this case.
+      if (index < 0) {
+        this.shownPage = templateData['apps_page_id'];
+        index = tiles.indexOf(this.appsPages[0]);
+      }
+      // Set the new cards and index.
+      this.cardSlider.setCards(tiles, index);
+      // If we got new page or index, update the pref.
+      if (prevIndex != this.shownPageIndex || prevPage != this.shownPage)
+        chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
     },
 
     /**
@@ -403,13 +441,10 @@ cr.define('ntp4', function() {
     enterRearrangeMode: function() {
       var tempPage = new ntp4.AppsPage();
       tempPage.classList.add('temporary');
-      this.appendTilePage(tempPage,
-                          localStrings.getString('appDefaultPageName'),
-                          true);
-      var tempIndex = Array.prototype.indexOf.call(this.tilePages, tempPage);
-      if (this.cardSlider.currentCard >= tempIndex)
-        this.cardSlider.currentCard += 1;
+      var pageName = localStrings.getString('appDefaultPageName');
+      this.appendTilePage(tempPage, pageName, true);
       this.updateSliderCards();
+      this.updatePageSwitchers();
 
       if (ntp4.getCurrentlyDraggingTile().firstChild.canBeRemoved())
         $('footer').classList.add('showing-trash-mode');
@@ -420,18 +455,10 @@ cr.define('ntp4', function() {
      */
     leaveRearrangeMode: function() {
       var tempPage = document.querySelector('.tile-page.temporary');
-      var dot = tempPage.navigationDot;
-      if (!tempPage.tileCount && tempPage != this.cardSlider.currentCardValue) {
-        dot.animateRemove();
-        var tempIndex = Array.prototype.indexOf.call(this.tilePages, tempPage);
-        if (this.cardSlider.currentCard > tempIndex)
-          this.cardSlider.currentCard -= 1;
-        tempPage.parentNode.removeChild(tempPage);
-        this.updateSliderCards();
-      } else {
+      // If there's a temp page and it wasn't removed for being empty.
+      if (tempPage && !this.removeAppsPageIfEmpty(tempPage, true)) {
+        this.saveAppsPageName(tempPage, tempPage.navigationDot.displayTitle);
         tempPage.classList.remove('temporary');
-        this.saveAppPageName(tempPage,
-                             localStrings.getString('appDefaultPageName'));
       }
 
       $('footer').classList.remove('showing-trash-mode');
@@ -488,9 +515,10 @@ cr.define('ntp4', function() {
     },
 
     /**
-     * Returns the index of the given page.
-     * @param {AppsPage} page The AppsPage for we wish to find.
-     * @return {number} The index of |page|, or -1 if it is not here.
+     * Returns the index of the given apps page.
+     * @param {AppsPage} page The AppsPage we wish to find.
+     * @return {number} The index of |page| or -1 if it is not in the
+     *    collection.
      */
     getAppsPageIndex: function(page) {
       return Array.prototype.indexOf.call(this.appsPages, page);
@@ -507,6 +535,8 @@ cr.define('ntp4', function() {
       // Don't change shownPage until startup is done (and page changes actually
       // reflect user actions).
       if (!document.documentElement.classList.contains('starting-up')) {
+        var prevPage = this.shownPage;
+        var prevIndex = this.shownPageIndex;
         if (page.classList.contains('apps-page')) {
           this.shownPage = templateData['apps_page_id'];
           this.shownPageIndex = this.getAppsPageIndex(page);
@@ -516,7 +546,8 @@ cr.define('ntp4', function() {
         } else {
           console.error('unknown page selected');
         }
-        chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
+        if (prevPage != this.shownPage || prevIndex != this.shownPageIndex)
+          chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
       }
 
       // Update the active dot
@@ -527,16 +558,16 @@ cr.define('ntp4', function() {
       this.updatePageSwitchers();
     },
 
-    /*
+    /**
      * Save the name of an app page.
      * Store the app page name into the preferences store.
-     * @param {AppsPage} appPage The app page for which we wish to save.
+     * @param {AppsPage} appsPage The app page for which we wish to save.
      * @param {string} name The name of the page.
      */
-    saveAppPageName: function(appPage, name) {
-      var index = this.getAppsPageIndex(appPage);
+    saveAppsPageName: function(appsPage, name) {
+      var index = this.getAppsPageIndex(appsPage);
       assert(index != -1);
-      chrome.send('saveAppPageName', [name, index]);
+      chrome.send('saveAppsPageName', [name, index]);
     },
 
     /**
@@ -587,7 +618,145 @@ cr.define('ntp4', function() {
       this.cardSlider.selectCard(cardIndex, true);
 
       e.stopPropagation();
-    }
+    },
+
+    /**
+     * Re-order apps on this invactive page when an active NTP gets re-ordered.

[csilv, 2011/12/02 01:57:02]

"inactive"

[Dan Beam, 2011/12/05 18:05:10]

Done.

+     * @param {Object} data Position hashmap ordered by app id with indices:
+     *    {id => {page_index: ##, app_launch_index: ##}}
+     */
+    appsReordered: function(data) {
+      var pages = Object.keys(data);
+      for (var i = 0; i < pages.length; ++i) {
+        var page = data[pages[i]];
+        var indices = Object.keys(page);
+        for (var j = 0; j < indices.length; ++j) {
+          var app = $(page[indices[j]]);
+          if (i != this.getAppsPageIndex(app.tile.tilePage) ||
+              j != app.tile.index) {
+            var tile = app.tile.remove(false, true);
+            this.ensureAppsPageAtIndex_(i);
+            var appsPage = this.appsPages[i];
+            appsPage.addTileAt(tile.firstChild, j);
+            if (appsPage.classList.contains('selected-card'))
+              app.loadIcon();
+          }
+        }
+      }
+      for (var i = 0; i < this.appsPages.length; ++i) {
+        // If a page is now empty, remove it and move backward one in our
+        // array-like object as it is a live NodeList and the length/indice
+        // change when you update it. Otherwise, reuse cleanupDrag() to
+        // re-position the tiles like when a user is done dragging.
+        if (this.removeAppsPageIfEmpty(this.appsPages[i], false, true))
+          --i;
+        else
+          this.appsPages[i].cleanupDrag();
+      }
+    },
+
+    /**
+     * Ensure there is an apps page at the given |index|. If index is smaller
+     * than the current number of apps pages, additional apps pages will be
+     * created.
+     * @param {number} index An apps page index to ensure exists.
+     */
+    ensureAppsPageAtIndex_: function(index) {
+      if (index >= this.appsPages.length) {
+        var pageName = localStrings.getString('appDefaultPageName');
+        while (index >= this.appsPages.length)
+          this.appendTilePage(new ntp4.AppsPage(), pageName, true);
+        this.updateSliderCards();
+      }
+    },
+
+    /**
+     * Returns the index of a given tile page.
+     * @param {TilePage} page The TilePage we wish to find.
+     * @return {number} The index of |page| or -1 if it is not in the
+     *    collection.
+     */
+    getTilePageIndex: function(page) {
+      return Array.prototype.indexOf.call(this.tilePages, page);
+    },
+
+    /**
+     * Move to a non-empty page before/while we delete an empty one.
+     * @param {Element} appsPage An empty apps page that we want to move off of
+     *     (probably because it's empty).
+     * @param {=boolean} animate Whether we should animate or not.
+     * @param {=function} callback A callback to be executed when the move is
+     *     done.
+     */
+    moveOffEmptyAppsPage: function(appsPage, opt_animate, opt_callback) {
+      assert(appsPage instanceof ntp4.AppsPage,
+             '|appsPage| is really an AppsPage');
+      // Select the page further toward the end than the empty |appsPage|. If
+      // the empty |appsPage| is at the end, the apps page before it will be
+      // selected.
+      var index = this.getAppsPageIndex(appsPage);
+      assert(index >= 0, '|appsPage| is in this.appsPages');
+      var direction = (index == this.appsPages.length - 1 ? -1 : 1);
+      this.cardSlider.selectCard(this.cardSlider.currentCard + direction,
+                                 opt_animate, opt_callback);
+    },
+
+    /**
+     * Utility function to remove both a tile page and corresponding nav dot.
+     * NOTE: If |animated| is true, this function will return before the
+     * animation is finished.
+     * @param {Element} appsPage An apps page to be removed if it has no tiles.
+     * @param {=boolean} opt_animate An optional param to animate the removal.
+     * @param {=boolean} opt_dontSave Whether we should notify other NTPs this
+     *     page was deleted (defaults to yes).
+     * @returns {boolean} If the page was removed.
+     */
+    removeAppsPageIfEmpty: function(appsPage, opt_animate, opt_dontSave) {
+      if (appsPage && appsPage.tileCount === 0) {
+        var whenDone = function() {
+          if (!opt_dontSave) {
+            chrome.send('deleteAppsPage',
+                [this.getAppsPageIndex(appsPage),
+                 appsPage.classList.contains('temporary')]);
+          }
+          this.removeTilePage(appsPage, opt_animate);
+          this.updateSliderCards();
+          this.updatePageSwitchers();
+        };
+        if (appsPage == this.cardSlider.currentCardValue)
+          this.moveOffEmptyAppsPage(appsPage, opt_animate, whenDone.bind(this));
+        else
+          whenDone.call(this);
+        return true;
+      }
+      return false;
+    },
+
+    /**
+     * Removes a page and navigation dot (if the navdot exists).
+     * @param {Element} page The page to be removed.
+     * @param {boolean} animate If the removal should be animated.
+     */
+    removeTilePage: function(page, animate) {
+      if (!document.documentElement.classList.contains('starting-up')) {
+        // If the card being deleted is before the current card, keep the same
+        // spot in the list by removing 1 from the current card index.
+        var index = Array.prototype.indexOf.call(this.tilePages, page);
+        if (index < this.cardSlider.currentCard) {
+          this.cardSlider.currentCard -= 1;
+        }
+        // If the page we're about to delete is in front of the current page,
+        // remove 1 from the shown page index.
+        if (page instanceof ntp4.AppsPage &&
+            this.shownPage == templateData['apps_page_id'] &&
+            this.getAppsPageIndex(page) < this.shownPageIndex) {
+          this.shownPageIndex -= 1;
+        }
+      }
+      if (page.navigationDot)
+        page.navigationDot.remove(animate);
+      page.remove();
+    },
   };
 
   return {