ntp4: app install improvements

chrome/browser/resources/ntp4/new_tab.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.
 
 /**
  * @fileoverview New tab page
  * This is the main code for the new tab page used by touch-enabled Chrome
  * browsers.  For now this is still a prototype.
  */
 
@@ skipping 299 matching lines @@
       if (app.id == highlightAppId) {
         highlightApp = app;
         highlightAppId = null;
       } else {
         appsPages[pageIndex].appendApp(app);
       }
     }
 
     ntp4.AppsPage.setPromo(data.showPromo ? data : null);
 
-    // Tell the slider about the pages
+    // Tell the slider about the pages.
     updateSliderCards();
 
     if (highlightApp)
-      appAdded(highlightApp);
+      appAdded(highlightApp, true);
 
-    // Mark the current page
-    dots[cardSlider.currentCard].classList.add('selected');
+    // Mark the current page.
+    cardSlider.currentCardValue.navigationDot.classList.add('selected');

[Rick Byers, 2011/08/23 20:52:13]

Looks like you're not using the 'dots' list anymore, remove it.

[Evan Stade, 2011/08/25 03:08:56]

Done.

     logEvent('apps.layout: ' + (Date.now() - startTime));
   }
 
   /**
    * Called by chrome when a new app has been added to chrome.
    * @param {Object} app A data structure full of relevant information for the
    *     app.
    */
-  function appAdded(app) {
+  function appAdded(app, opt_highlight) {
+    // If the page is already open when a new app is installed, the hash will
+    // be set once again.
+    var hash = location.hash;
+    if (hash && hash.indexOf('#app-id=') == 0 && app.id == hash.split('=')[1]) {

[Rick Byers, 2011/08/23 20:52:13]

the logic to parse the hash is duplicated in initialize.  I suggest adding a
little helper function to reduce/eliminate the duplication (eg.
getAndClearHashAppId - but it'll be a little more complicated than that if you
want to avoid unconditionally clearing it here).

[Evan Stade, 2011/08/25 03:08:56]

Done.

+      window.history.replaceState({}, '', '/');
+      opt_highlight = true;
+    }
+
     var pageIndex = app.page_index || 0;
-    var page = appsPages[pageIndex];
-    cardSlider.selectCardByValue(page);
+    var page;
+    if (pageIndex < appsPages.length) {
+      page = appsPages[pageIndex];
+    } else {
+      page = new ntp4.AppsPage();

[Rick Byers, 2011/08/23 20:52:13]

is it not possible for page_index to be more than 1 greater than the number of
current pages?  Eg., what if all apps are removed from some middle page?  I'd
suggest instead looping here until you know you have enough pages...

[Evan Stade, 2011/08/25 03:08:56]

Done.

+      appendAppsPage(page, '');
+      updateSliderCards();
+    }
+
+    if (opt_highlight)
+      cardSlider.selectCardByValue(page);
     page.appendApp(app, true);
   }
 
   /**
    * Called by chrome when an existing app has been removed/uninstalled from
    * chrome.
    * @param {Object} appData A data structure full of relevant information for
    *     the app.
    */
   function appRemoved(appData) {
@@ skipping 42 matching lines @@
    */
   function updateSliderCards() {
     var pageNo = cardSlider.currentCard;
     if (pageNo >= tilePages.length)
       pageNo = tilePages.length - 1;
     var pageArray = [];
     for (var i = 0; i < tilePages.length; i++)
       pageArray[i] = tilePages[i];
     cardSlider.setCards(pageArray, pageNo);
 
-    if (shownPage == templateData['most_visited_page_id'])
+    if (shownPage == templateData['most_visited_page_id']) {
       cardSlider.selectCardByValue(mostVisitedPage);
-    else if (shownPage == templateData['apps_page_id'])
-      cardSlider.selectCardByValue(appsPages[shownPageIndex]);
-    else if (shownPage == templateData['bookmarks_page_id'])
+    } else if (shownPage == templateData['apps_page_id']) {
+      cardSlider.selectCardByValue(
+          appsPages[Math.min(shownPageIndex, appsPages.length - 1)]);
+    } else if (shownPage == templateData['bookmarks_page_id']) {
       cardSlider.selectCardByValue(bookmarksPage);
+    }
   }
 
   /**
    * Appends a tile page (for bookmarks or most visited).
    *
    * @param {TilePage} page The page element.
    * @param {string} title The title of the tile page.
    */
   function appendTilePage(page, title) {
     pageList.appendChild(page);
@@ skipping 62 matching lines @@
       $('footer').classList.add('showing-trash-mode');
   }
 
   /**
    * Invoked whenever some app is released
    * @param {Grabber.Event} e The Grabber RELEASE event.
    */
   function leaveRearrangeMode(e) {
     var tempPage = document.querySelector('.tile-page.temporary');
     var dot = tempPage.navigationDot;
-    if (!tempPage.tileCount) {
+    if (!tempPage.tileCount && tempPage != cardSlider.currentCardValue) {
       dot.animateRemove();
       tempPage.parentNode.removeChild(tempPage);
       updateSliderCards();
     } else {
       tempPage.classList.remove('temporary');
       saveAppPageName(tempPage, '');
     }
 
     $('footer').classList.remove('showing-trash-mode');
   }
@@ skipping 120 matching lines @@
       shownPageIndex = 0;
     } else {
       console.error('unknown page selected');
     }
     chrome.send('pageSelected', [shownPage, shownPageIndex]);
 
     // Update the active dot
     var curDot = dotList.getElementsByClassName('selected')[0];
     if (curDot)
       curDot.classList.remove('selected');
-    var newPageIndex = e.cardSlider.currentCard;
-    dots[newPageIndex].classList.add('selected');
+    page.navigationDot.classList.add('selected');
     updatePageSwitchers();
   }
 
   /**
     * Timeout ID.
     * @type {number}
     */
   var notificationTimeout_ = 0;
 
   /**
@@ skipping 101 matching lines @@
 // TODO(estade): update the content handlers to use ntp namespace instead of
 // making these global.
 var assert = ntp4.assert;
 var getAppsCallback = ntp4.getAppsCallback;
 var appsPrefChangeCallback = ntp4.appsPrefChangeCallback;
 var themeChanged = ntp4.themeChanged;
 var recentlyClosedTabs = ntp4.setRecentlyClosedTabs;
 var setMostVisitedPages = ntp4.setMostVisitedPages;
 
 document.addEventListener('DOMContentLoaded', ntp4.initialize);