Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4016)

Unified Diff: chrome/browser/resources/ntp4/page_list_view.js

Issue 9116037: [NTP4] Make TilePage and CardSlider evented to simplify code and fix page switcher bug (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: rebase + event order change Created 8 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 d35f41bcebfce973595e6df5179e947fb4b8eaa2..c25f036bf16730e09e0826c8f1df994b5863d6ef 100644
--- a/chrome/browser/resources/ntp4/page_list_view.js
+++ b/chrome/browser/resources/ntp4/page_list_view.js
@@ -170,7 +170,22 @@ cr.define('ntp4', function() {
// Handle the page being changed
this.pageList.addEventListener(
cr.ui.CardSlider.EventType.CARD_CHANGED,
- this.cardChangedHandler_.bind(this));
+ this.onCardChanged_.bind(this));
+
+ // Handle cards being added to the card slider.
+ this.pageList.addEventListener(
+ cr.ui.CardSlider.EventType.CARD_ADDED,
+ this.onCardAdded_.bind(this));
+
+ // Handle cards being removed from the card slider.
+ this.pageList.addEventListener(
+ cr.ui.CardSlider.EventType.CARD_REMOVED,
+ this.onCardRemoved_.bind(this));
+
+ // Handle tiles being removed from tile pages.
+ this.pageList.addEventListener(
+ ntp4.TilePage.EventType.TILE_REMOVED,
+ this.onTileRemoved_.bind(this));
// Ensure the slider is resized appropriately with the window
window.addEventListener('resize', this.onWindowResize_.bind(this));
@@ -194,8 +209,12 @@ cr.define('ntp4', function() {
* the page list.
*/
appendTilePage: function(page, title, titleIsEditable, opt_refNode) {
- // When opt_refNode is falsey, insertBefore acts just like appendChild.
- this.pageList.insertBefore(page, opt_refNode);
+ if (opt_refNode) {
+ var refIndex = this.getTilePageIndex(opt_refNode);
+ this.cardSlider.insertCardAtIndex(page, refIndex);
+ } else {
+ this.cardSlider.appendCard(page);
+ }
// Remember special MostVisitedPage.
if (typeof ntp4.MostVisitedPage != 'undefined' &&
@@ -227,15 +246,24 @@ cr.define('ntp4', function() {
* the app.
* @param {boolean} isUninstall True if the app is being uninstalled;
* false if the app is being disabled.
+ * @param {boolean} fromPage True if the removal was from the current page.
*/
- 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)
app.replaceAppData(appData);
else
- app.remove();
+ app.remove(!!fromPage);
+ },
+
+ /**
+ * @return {boolean} If the page is still starting up.
+ * @private
+ */
+ isStartingUp_: function() {
+ return document.documentElement.classList.contains('starting-up');
},
/**
@@ -255,15 +283,8 @@ cr.define('ntp4', function() {
// uninstall. Could we re-use the existing page and dot elements? It
// seems unfortunate to have Chrome send us the entire apps list after an
// 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);
- }
+ while (this.appsPages.length > 0)
+ this.removeTilePageAndDot_(this.appsPages[0]);
// Get the array of apps and add any special synthesized entries
var apps = data.apps;
@@ -404,13 +425,8 @@ 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;
- this.updateSliderCards();
+ var pageName = localStrings.getString('appDefaultPageName');
+ this.appendTilePage(tempPage, pageName, true);
if (ntp4.getCurrentlyDraggingTile().firstChild.canBeRemoved())
$('footer').classList.add('showing-trash-mode');
@@ -423,12 +439,7 @@ cr.define('ntp4', 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();
+ this.removeTilePageAndDot_(tempPage, true);
} else {
tempPage.classList.remove('temporary');
this.saveAppPageName(tempPage,
@@ -503,12 +514,12 @@ cr.define('ntp4', function() {
* @param {Event} e The CARD_CHANGED event.
* @private
*/
- cardChangedHandler_: function(e) {
+ onCardChanged_: function(e) {
var page = e.cardSlider.currentCardValue;
// Don't change shownPage until startup is done (and page changes actually
// reflect user actions).
- if (!document.documentElement.classList.contains('starting-up')) {
+ if (!this.isStartingUp_()) {
if (page.classList.contains('apps-page')) {
this.shownPage = templateData.apps_page_id;
this.shownPageIndex = this.getAppsPageIndex(page);
@@ -529,10 +540,34 @@ 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.
+ /**
+ * Listen for card additions to update the page switchers or the current
+ * card accordingly.
+ * @param {Event} e A card removed or added event.
+ */
+ onCardAdded_: function(e) {
+ // When the second arg passed to insertBefore is falsey, it acts just like
+ // appendChild.
+ this.pageList.insertBefore(e.addedCard, this.tilePages[e.addedIndex]);
+ if (!this.isStartingUp_())
+ this.updatePageSwitchers();
+ },
+
+ /**
+ * Listen for card removals to update the page switchers or the current card
+ * accordingly.
+ * @param {Event} e A card removed or added event.
+ */
+ onCardRemoved_: function(e) {
+ e.removedCard.parentNode.removeChild(e.removedCard);
+ if (!this.isStartingUp_())
+ this.updatePageSwitchers();
+ },
+
+ /**
+ * Save the name of an apps page.
+ * Store the apps page name into the preferences store.
+ * @param {AppsPage} appsPage The app page for which we wish to save.
* @param {string} name The name of the page.
*/
saveAppPageName: function(appPage, name) {
@@ -589,7 +624,37 @@ cr.define('ntp4', function() {
this.cardSlider.selectCard(cardIndex, true);
e.stopPropagation();
- }
+ },
+
+ /**
+ * Happens when a tile is removed from a tile page.
+ * @param {Event} e An event dispatched from a tile when it is removed.
+ */
+ onTileRemoved_: function(e) {
+ // TODO(dbeam): Remove empty apps pages.
+ 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);
+ },
+
+ /**
+ * Removes a page and navigation dot (if the navdot exists).
+ * @param {TilePage} page The page to be removed.
+ * @param {boolean=} opt_animate If the removal should be animated.
+ */
+ removeTilePageAndDot_: function(page, opt_animate) {
+ if (page.navigationDot)
+ page.navigationDot.remove(opt_animate);
+ this.cardSlider.removeCard(page);
+ },
};
return {

Powered by Google App Engine
This is Rietveld 408576698