Refactoring NTP5: new implementation of TilePage and MostVisitedPage (which now inherits from Thumb…

chrome/browser/resources/ntp_search/thumbnail_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';
 
-  var TilePage = ntp.TilePage;
+  var Tile = ntp.Tile2;
+  var TilePage = ntp.TilePage2;
 
   /**
    * A counter for generating unique tile IDs.
    */
   var tileID = 0;
 
   /**
    * Creates a new Most Visited object for tiling.

[jeremycho_google, 2012/08/02 03:00:49]

most visited (please make another pass cleaning this up)

[pedrosimonetti2, 2012/08/03 18:14:12]

Done.

    * @constructor

[jeremycho_google, 2012/08/02 03:00:49]

@param

[pedrosimonetti2, 2012/08/03 18:14:12]

Done.

    * @extends {HTMLAnchorElement}
    */
-  function MostVisited() {
+  function Thumbnail(gridValues) {
     var el = cr.doc.createElement('a');
-    el.__proto__ = MostVisited.prototype;
-    el.initialize();
+    el.__proto__ = Thumbnail.prototype;
+    el.initialize(gridValues);
 
     return el;
   }
 
-  MostVisited.prototype = {
+  Thumbnail.prototype = Tile.subclass({
     __proto__: HTMLAnchorElement.prototype,
 
-    initialize: function() {
+    // TODO(pedrosimonetti): document
+    isRemovable: true,
+
+    initialize: function(gridValues) {
+      Tile.prototype.initialize.apply(this, arguments);
       this.reset();
 
       this.addEventListener('click', this.handleClick_);
       this.addEventListener('keydown', this.handleKeyDown_);
     },
 
     get index() {

[jeremycho_google, 2012/08/02 00:19:11]

Can this be deleted?  It's almost identical to 'index' in TileGetters
(tile_page.js).

[pedrosimonetti2, 2012/08/03 18:14:12]

Done.

       assert(this.tile);
       return this.tile.index;
     },
 
     get data() {
       return this.data_;
     },
 
     /**
      * Clears the DOM hierarchy for this node, setting it back to the default
      * for a blank thumbnail.
      */
     reset: function() {
-      this.className = 'most-visited filler real';
+      this.className = 'tile-content most-visited real';
       this.innerHTML =
           '<span class="thumbnail-wrapper fills-parent">' +
-            '<div class="close-button"></div>' +
+            (this.isRemovable ? '<div class="close-button"></div>' : '') +
             '<span class="thumbnail fills-parent">' +
+              // TODO(pedrosimonetti): remove all references (HTML+CSS+JS)
               // thumbnail-shield provides a gradient fade effect.
-              '<div class="thumbnail-shield fills-parent"></div>' +
+              //'<div class="thumbnail-shield fills-parent"></div>' +
             '</span>' +
-            '<span class="favicon"></span>' +
+            // TODO(pedrosimonetti): remove all references (HTML+CSS+JS)
+            //'<span class="favicon"></span>' +
           '</span>' +
-          '<div class="color-stripe"></div>' +
+          // TODO(pedrosimonetti): remove all references (HTML+CSS+JS)
+          //'<div class="color-stripe"></div>' +
           '<span class="title"></span>';
 
-      this.querySelector('.close-button').title =
-          loadTimeData.getString('removethumbnailtooltip');
+      if (this.isRemovable) {
+        this.querySelector('.close-button').title =
+            loadTimeData.getString('removethumbnailtooltip');
+      }
 
       this.tabIndex = -1;
       this.data_ = null;
       this.removeAttribute('id');
       this.title = '';
     },
 
     /**
      * Update the appearance of this tile according to |data|.
      * @param {Object} data A dictionary of relevant data for the page.
      */
     updateForData: function(data) {
       if (this.classList.contains('blacklisted') && data) {
         // Animate appearance of new tile.
         this.classList.add('new-tile-contents');
       }
       this.classList.remove('blacklisted');
 
       if (!data || data.filler) {
         if (this.data_)
           this.reset();
         return;
       }
 
       var id = tileID++;
+      // TODO(pedrosimonetti): refactor
       this.id = 'most-visited-tile-' + id;
       this.data_ = data;
       this.classList.add('focusable');
 
-      var faviconDiv = this.querySelector('.favicon');
-      var faviconUrl = 'chrome://favicon/size/16/' + data.url;
-      faviconDiv.style.backgroundImage = url(faviconUrl);
-      chrome.send('getFaviconDominantColor', [faviconUrl, this.id]);
+      // TODO(pedrosimonetti): remove all references (HTML+CSS+JS)
+      //var faviconDiv = this.querySelector('.favicon');
+      //var faviconUrl = 'chrome://favicon/size/16/' + data.url;
+      //faviconDiv.style.backgroundImage = url(faviconUrl);
+      //chrome.send('getFaviconDominantColor', [faviconUrl, this.id]);
 
       var title = this.querySelector('.title');
       title.textContent = data.title;
       title.dir = data.direction;
 
       // Sets the tooltip.
       this.title = data.title;
 
-      var thumbnailUrl = 'chrome://thumb/' + data.url;
+      var thumbnailUrl = ntp.getThumbnailUrl(data.url);
       this.querySelector('.thumbnail').style.backgroundImage =
           url(thumbnailUrl);
 
       this.href = data.url;
 
       this.classList.remove('filler');
     },
 
     /**
      * Sets the color of the favicon dominant color bar.
      * @param {string} color The css-parsable value for the color.
      */
-    set stripeColor(color) {
-      this.querySelector('.color-stripe').style.backgroundColor = color;
-    },
+    // TODO(xci) delete
+    //set stripeColor(color) {
+    //  this.querySelector('.color-stripe').style.backgroundColor = color;
+    //},
 
     /**
      * Handles a click on the tile.
      * @param {Event} e The click event.
      */
     handleClick_: function(e) {
+      debugger;
       if (e.target.classList.contains('close-button')) {
         this.blacklist_();
         e.preventDefault();
       } else {
         // Records an app launch from the most visited page (Chrome will decide
         // whether the url is an app). TODO(estade): this only works for clicks;
         // other actions like "open in new tab" from the context menu won't be
         // recorded. Can this be fixed?
         chrome.send('recordAppLaunchByURL',
                     [encodeURIComponent(this.href),
                      ntp.APP_LAUNCH.NTP_MOST_VISITED]);
         // Records the index of this tile.
         chrome.send('metricsHandler:recordInHistogram',
-                    ['NewTabPage.MostVisited', this.index, 8]);
+                    ['NewTabPage.Thumbnail', this.index, 8]);
         chrome.send('mostVisitedAction',

[jeremycho_google, 2012/08/02 03:00:49]

This is MostVisited specific.  Should handleClick_ be virtual?

[pedrosimonetti2, 2012/08/03 18:14:12]

handleClick_ is not implemented in the Thumbnail class anymore.

On 2012/08/02 03:00:49, jeremycho wrote:

                     [ntp.NtpFollowAction.CLICKED_TILE]);
       }
     },
 
     /**
      * Allow blacklisting most visited site using the keyboard.

[jeremycho_google, 2012/08/02 03:00:49]

Move to MostVisited?

[pedrosimonetti2, 2012/08/03 18:14:12]

Done.

      */
     handleKeyDown_: function(e) {
       if (!cr.isMac && e.keyCode == 46 || // Del
           cr.isMac && e.metaKey && e.keyCode == 8) { // Cmd + Backspace
         this.blacklist_();
       }
     },
 
     /**
      * Permanently removes a page from Most Visited.
      */
     blacklist_: function() {
       this.showUndoNotification_();
-      chrome.send('blacklistURLFromMostVisited', [this.data_.url]);
+      chrome.send('blacklistURLFromThumbnail', [this.data_.url]);
       this.reset();
-      chrome.send('getMostVisited');
+      chrome.send('getThumbnail');
       this.classList.add('blacklisted');
     },
 
     showUndoNotification_: function() {
       var data = this.data_;
       var self = this;
       var doUndo = function() {
-        chrome.send('removeURLsFromMostVisitedBlacklist', [data.url]);
+        chrome.send('removeURLsFromThumbnailBlacklist', [data.url]);
         self.updateForData(data);
       }
 
       var undo = {
         action: doUndo,
         text: loadTimeData.getString('undothumbnailremove'),
       };
 
       var undoAll = {
         action: function() {
-          chrome.send('clearMostVisitedURLsBlacklist');
+          chrome.send('clearThumbnailURLsBlacklist');
         },
         text: loadTimeData.getString('restoreThumbnailsShort'),
       };
 
       ntp.showNotification(
           loadTimeData.getString('thumbnailremovednotification'),
           [undo, undoAll]);
     },
 
     /**
-     * Set the size and position of the most visited tile.
-     * @param {number} size The total size of |this|.
-     * @param {number} x The x-position.
-     * @param {number} y The y-position.
-     *     animate.
-     */
-    setBounds: function(size, x, y) {
-      this.style.width = toCssPx(size);
-      this.style.height = toCssPx(heightForWidth(size));
-
-      this.style.left = toCssPx(x);
-      this.style.right = toCssPx(x);
-      this.style.top = toCssPx(y);
-    },
-
-    /**
      * Returns whether this element can be 'removed' from chrome (i.e. whether
      * the user can drag it onto the trash and expect something to happen).
      * @return {boolean} True, since most visited pages can always be
      *     blacklisted.
      */
     canBeRemoved: function() {
-      return true;
+      return this.isRemovable;
     },
 
     /**
      * Removes this element from chrome, i.e. blacklists it.
      */
     removeFromChrome: function() {
       this.blacklist_();
       this.parentNode.classList.add('finishing-drag');
     },
 
     /**
      * Called when a drag of this tile has ended (after all animations have
      * finished).
      */
     finalizeDrag: function() {
       this.parentNode.classList.remove('finishing-drag');
     },
 
     /**
      * Called when a drag is starting on the tile. Updates dataTransfer with
      * data for this tile (for dragging outside of the NTP).
      */
+    // TODO(xci) delete
     setDragData: function(dataTransfer) {
       dataTransfer.setData('Text', this.data_.title);
       dataTransfer.setData('URL', this.data_.url);
     },
-  };
+  });
 
-  var mostVisitedPageGridValues = {
-    // The fewest tiles we will show in a row.
-    minColCount: 2,
-    // The most tiles we will show in a row.
-    maxColCount: 4,
-
-    // The smallest a tile can be.
-    minTileWidth: 122,
-    // The biggest a tile can be. 212 (max thumbnail width) + 2.
-    maxTileWidth: 214,
-
-    // The padding between tiles, as a fraction of the tile width.
-    tileSpacingFraction: 1 / 8,
-  };
-  TilePage.initGridValues(mostVisitedPageGridValues);
+  var THUMBNAIL_COUNT = 8; // TODO(xci)
 
   /**
-   * Calculates the height for a Most Visited tile for a given width. The size
-   * is based on the thumbnail, which should have a 212:132 ratio.
-   * @return {number} The height.
-   */
-  function heightForWidth(width) {
-    // The 2s are for borders, the 31 is for the title.
-    return (width - 2) * 132 / 212 + 2 + 31;
-  }
-
-  var THUMBNAIL_COUNT = 8;
-
-  /**
-   * Creates a new MostVisitedPage object.
+   * Creates a new ThumbnailPage object.
    * @constructor
    * @extends {TilePage}
    */
-  function MostVisitedPage() {
-    var el = new TilePage(mostVisitedPageGridValues);
-    el.__proto__ = MostVisitedPage.prototype;
+  function ThumbnailPage() {
+    var el = new TilePage();
+    el.__proto__ = ThumbnailPage.prototype;
     el.initialize();
 
     return el;
   }
 
-  MostVisitedPage.prototype = {
+  ThumbnailPage.prototype = {
     __proto__: TilePage.prototype,
 
+    gridValues_: {
+      tileWidth: 130,
+      tileHeight: 78,
+      tileHorMargin: 18, // TODO margin with CSS. There's no margin in first col
+      tileVerMargin: 22,
+      tileBorderWidth: 1,
+      bottomPanelHorMargin: 100,
+
+      tileCount: 10, // debug
+      tileClassName: 'thumbnail',
+      reinforceStyles: true,
+
+      // debug
+      slowFactor: 1,
+      debug: false
+    },
+
+    // TODO(pedrosimonetti): document
+    ThumbnailClass: Thumbnail,
+
     initialize: function() {
       this.classList.add('most-visited-page');

[jeremycho_google, 2012/08/02 03:00:49]

most-visited

[pedrosimonetti2, 2012/08/03 18:14:12]

I renamed it to 'thumbnail-page' and overrided the
MostVisited.prototype.initialize to include the 'most-visited' class too.

       this.data_ = null;
       this.mostVisitedTiles_ = this.getElementsByClassName('most-visited real');
 
       this.addEventListener('carddeselected', this.handleCardDeselected_);
       this.addEventListener('cardselected', this.handleCardSelected_);
     },
 
     /**
      * Create blank (filler) tiles.
      * @private
      */
     createTiles_: function() {
       for (var i = 0; i < THUMBNAIL_COUNT; i++) {
-        this.appendTile(new MostVisited());
+        this.appendTile(new this.ThumbnailClass(this.gridValues_));
       }
     },
 
     /**
      * Update the tiles after a change to |data_|.
      */
     updateTiles_: function() {
       for (var i = 0; i < THUMBNAIL_COUNT; i++) {
         var page = this.data_[i];
         var tile = this.mostVisitedTiles_[i];
 
+        // TODO(pedrosimonetti): what do we do when there's no tile here?
+        if (!tile) {
+          return;
+        }
+
         if (i >= this.data_.length)
           tile.reset();
         else
           tile.updateForData(page);
       }
     },
 
     /**
      * Handles the 'card deselected' event (i.e. the user clicked to another
      * pane).
@@ skipping 16 matching lines @@
         chrome.send('mostVisitedSelected');
     },
 
     /**
      * Array of most visited data objects.
      * @type {Array}
      */
     get data() {
       return this.data_;
     },
     set data(data) {

[jeremycho_google, 2012/08/02 03:00:49]

As we discussed, make virtual?

[pedrosimonetti2, 2012/08/03 18:14:12]

Done.

       var startTime = Date.now();
 
       // The first time data is set, create the tiles.
       if (!this.data_) {
         this.createTiles_();
         this.data_ = data.slice(0, THUMBNAIL_COUNT);
       } else {
         this.data_ = refreshData(this.data_, data);
       }
 
       this.updateTiles_();
       logEvent('mostVisited.layout: ' + (Date.now() - startTime));
     },
 
     /** @inheritDoc */
     shouldAcceptDrag: function(e) {
       return false;
     },
-
-    /** @inheritDoc */
-    heightForWidth: heightForWidth,
   };
 
   /**
    * Executed once the NTP has loaded. Checks if the Most Visited pane is
    * shown or not. If it is shown, the 'mostVisitedSelected' message is sent
    * to the C++ code, to record the fact that the user has seen this pane.
    */
-  MostVisitedPage.onLoaded = function() {
+  // TODO(pedrosimonetti) is it possible to keep this inside Thumbnail class?
+   /*
+  ThumbnailPage.onLoaded = function() {
     if (ntp.getCardSlider() &&
         ntp.getCardSlider().currentCardValue &&
         ntp.getCardSlider().currentCardValue.classList
         .contains('most-visited-page')) {
       chrome.send('mostVisitedSelected');
     }
   }
-
-  /**
-   * We've gotten additional Most Visited data. Update our old data with the
-   * new data. The ordering of the new data is not important, except when a
-   * page is pinned. Thus we try to minimize re-ordering.
-   * @param {Array} oldData The current Most Visited page list.
-   * @param {Array} newData The new Most Visited page list.
-   * @return {Array} The merged page list that should replace the current page
-   *     list.
-   */
-  function refreshData(oldData, newData) {
-    oldData = oldData.slice(0, THUMBNAIL_COUNT);
-    newData = newData.slice(0, THUMBNAIL_COUNT);
-
-    // Copy over pinned sites directly.
-    for (var j = 0; j < newData.length; j++) {
-      if (newData[j].pinned) {
-        oldData[j] = newData[j];
-        // Mark the entry as 'updated' so we don't try to update again.
-        oldData[j].updated = true;
-        // Mark the newData page as 'used' so we don't try to re-use it.
-        newData[j].used = true;
-      }
-    }
-
-    // Look through old pages; if they exist in the newData list, keep them
-    // where they are.
-    for (var i = 0; i < oldData.length; i++) {
-      if (!oldData[i] || oldData[i].updated)
-        continue;
-
-      for (var j = 0; j < newData.length; j++) {
-        if (newData[j].used)
-          continue;
-
-        if (newData[j].url == oldData[i].url) {
-          // The background image and other data may have changed.
-          oldData[i] = newData[j];
-          oldData[i].updated = true;
-          newData[j].used = true;
-          break;
-        }
-      }
-    }
-
-    // Look through old pages that haven't been updated yet; replace them.
-    for (var i = 0; i < oldData.length; i++) {
-      if (oldData[i] && oldData[i].updated)
-        continue;
-
-      for (var j = 0; j < newData.length; j++) {
-        if (newData[j].used)
-          continue;
-
-        oldData[i] = newData[j];
-        oldData[i].updated = true;
-        newData[j].used = true;
-        break;
-      }
-
-      if (oldData[i] && !oldData[i].updated)
-        oldData[i] = null;
-    }
-
-    // Clear 'updated' flags so this function will work next time it's called.
-    for (var i = 0; i < THUMBNAIL_COUNT; i++) {
-      if (oldData[i])
-        oldData[i].updated = false;
-    }
-
-    return oldData;
-  };
+  */
 
   return {
-    MostVisitedPage: MostVisitedPage,
-    refreshData: refreshData,
+    Thumbnail: Thumbnail,
+    ThumbnailPage: ThumbnailPage
   };
 });
 
-document.addEventListener('ntpLoaded', ntp.MostVisitedPage.onLoaded);