[ntp4] Bookmarks page implementation, first-pass.

chrome/browser/resources/ntp4/bookmarks_page.js

Index: chrome/browser/resources/ntp4/bookmarks_page.js
===================================================================
--- chrome/browser/resources/ntp4/bookmarks_page.js	(revision 96850)
+++ chrome/browser/resources/ntp4/bookmarks_page.js	(working copy)
@@ -5,6 +5,142 @@
 cr.define('ntp4', function() {
   'use strict';
 
+  var localStrings = new LocalStrings;
+
+  /**
+   * A running count of bookmark tiles that we create so that each will have
+   * a unique ID.

[Sheridan Rawlins, 2011/08/16 05:50:28]

nit: @type {number}

+   */
+  var tileId = 0;
+
+  /**
+   * Creates a new bookmark object.
+   * @param {Object} data The url and title.
+   * @constructor
+   * @extends {HTMLDivElement}
+   */
+  function Bookmark(data) {
+    var el = $('bookmark-template').cloneNode(true);
+    el.__proto__ = Bookmark.prototype;
+    el.data = data;
+    el.initialize();
+
+    return el;
+  }
+
+  Bookmark.prototype = {
+    __proto__: HTMLDivElement.prototype,
+
+    initialize: function() {

[Sheridan Rawlins, 2011/08/16 05:50:28]

nit: JSDoc (@inheritDoc?)

[arv (Not doing code reviews), 2011/08/16 19:36:56]

HTMLDivElement does not have initialize so this should not be @inheritDoc.

+      var id = tileId++;
+      this.id = 'bookmark_tile_' + id;
+
+      var title = this.querySelector('.title');
+      title.textContent = this.data.title;
+
+      if (this.data.url) {
+        var button = this.querySelector('.button');
+        button.href = title.href = this.data.url;
+      }
+
+      var faviconDiv = this.querySelector('.favicon');
+      var faviconUrl;
+      if (this.data.url) {
+        faviconUrl = 'chrome://favicon/size/32/' + this.data.url;
+        chrome.send('getFaviconDominantColor',
+                    [faviconUrl, id, 'ntp4.setBookmarksFaviconDominantColor']);
+      } else {
+        // TODO(csilv): We need a large (32px) icon for this URL.
+        faviconUrl = 'chrome://theme/IDR_BOOKMARK_BAR_FOLDER';
+        // TODO(csilv): Should we vary this color by platform?
+        this.stripeColor = '#919191';
+      }
+      faviconDiv.style.backgroundImage = url(faviconUrl);
+
+      this.addEventListener('click', this.handleClick_.bind(this));
+    },
+
+    /**
+     * 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;
+    },
+
+    /**
+     * Set the size and position of the bookmark tile.
+     * @param {number} size The total size of |this|.
+     * @param {number} x The x-position.
+     * @param {number} y The y-position.
+     *     animate.

[Sheridan Rawlins, 2011/08/16 05:50:28]

Sorry, what does "animate" mean?

+     */
+    setBounds: function(size, x, y) {
+      this.style.width = this.style.height = size + 'px';
+      this.style.left = x + 'px';
+      this.style.top = y + 'px';
+    },
+
+    /**
+     * Invoked when a bookmark is clicked
+     * @param {Event} e The click event.
+     * @private
+     */
+    handleClick_: function(e) {
+      if (e.target.classList.contains('close-button')) {
+        this.handleDelete_();
+        e.preventDefault();
+      } else if (!this.data.url) {
+        chrome.send('getBookmarksData', [this.data.id]);
+        e.preventDefault();
+      }
+    },
+
+    /**
+     * Delete a bookmark from the data model.
+     * @private
+     */
+    handleDelete_: function() {
+      // TODO(csilv): add support for deleting bookmarks
+    },
+  };
+
+  /**
+   * Creates a new bookmark title object.
+   * @param {Object} data The url and title.
+   * @constructor
+   * @extends {HTMLDivElement}
+   */
+  function BookmarkTitle(data) {
+    var el = cr.doc.createElement('div');
+    el.__proto__ = BookmarkTitle.prototype;
+    el.initialize(data);
+
+    return el;
+  }
+
+  BookmarkTitle.prototype = {
+    __proto__: HTMLDivElement.prototype,
+
+    initialize: function(data) {

[Sheridan Rawlins, 2011/08/16 05:50:28]

nit: JSDoc (@inheritDoc ?)

+      this.className = 'title-crumb';
+      this.folderId_ = data.id;
+      this.textContent = data.parentId ? data.title :
+          localStrings.getString('bookmarksPage');
+
+      this.addEventListener('click', this.handleClick_);

[Sheridan Rawlins, 2011/08/16 05:50:28]

Will this work, or does it need to be "bound"?  It looks like the method relies
on |this| - will |this| be set properly without .bind() here?

[arv (Not doing code reviews), 2011/08/16 19:36:56]

It will work since addEventListener is called on this and the callbacks will use
that as the receiver.

+    },
+
+    /**
+     * Invoked when a bookmark title is clicked
+     * @param {Event} e The click event.
+     * @private
+     */
+    handleClick_: function(e) {
+      chrome.send('getBookmarksData', [this.folderId_]);
+    },
+  };
+
   var TilePage = ntp4.TilePage;
 
   var bookmarksPageGridValues = {
@@ -14,9 +150,9 @@
     maxColCount: 6,
 
     // The smallest a tile can be.
-    minTileWidth: 100,
+    minTileWidth: 150,
     // The biggest a tile can be.
-    maxTileWidth: 100,
+    maxTileWidth: 150,
   };
   TilePage.initGridValues(bookmarksPageGridValues);
 
@@ -38,21 +174,94 @@
 
     initialize: function() {
       this.classList.add('bookmarks-page');
+      this.insertBefore($('bookmarks-title-wrapper'), this.firstChild);
+    },
 
-      // TODO(csilv): Remove this placeholder.
-      var placeholder = this.ownerDocument.createElement('div');
-      placeholder.textContent = 'Bookmarks coming soon...';
-      placeholder.className = 'placeholder';
-      this.insertBefore(placeholder, this.firstChild);
+    /**
+     * Build the bookmark titles bar (ie, navigation hiearchy).
+     * @param {Array} items The parent hiearchy of the current folder.
+     * @private
+     */
+    updateBookmarkTitles_: function(items) {
+      var wrapper = $('bookmarks-title-wrapper');
+      var title = wrapper.querySelector('.section-title');
+      title.innerHTML = '';
+
+      for (var i = items.length - 1; i > 0; i--) {

[Sheridan Rawlins, 2011/08/16 05:50:28]

nit: --i

[Evan Stade, 2011/08/16 06:05:38]

why? I don't see any mention of post vs pre increment on either the internal js
style guide or the external chrome web development guide but the google C++
style guide says "For simple scalar (non-object) values there is no reason to
prefer one form and we allow either"

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

[Sheridan Rawlins, 2011/08/16 06:39:13]

I would have thought that getting in the habit of preinc/dec was preferred. 
Thanks for the correction and reference.

On 2011/08/16 06:05:38, Evan Stade wrote:

[arv (Not doing code reviews), 2011/08/16 19:36:56]

All the Google JS uses post inc/dec for consistency.

+        title.appendChild(new BookmarkTitle(items[i]));
+
+        var separator = document.createElement('hr');
+        separator.className = 'bookmark-separator';
+        title.appendChild(separator);
+      }
+
+      var titleCrumb = new BookmarkTitle(items[0]);
+      titleCrumb.classList.add('title-crumb-active');
+      title.appendChild(titleCrumb);
     },
 
+    /**
+     * Build the bookmark tiles.
+     * @param {Array} items The contents of the current folder.
+     * @private
+     */
+    updateBookmarkTiles_: function(items) {
+      this.removeAllTiles();
+      for (var i = 0; i < items.length; i++)

[Sheridan Rawlins, 2011/08/16 05:50:28]

nit: ++i

[Sheridan Rawlins, 2011/08/16 06:39:13]

Please ignore

+        this.appendTile(new Bookmark(items[i]), false);
+    },
+
     /** @inheritDoc */
     shouldAcceptDrag: function(dataTransfer) {
       return false;
-    }
+    },
+
+    /**
+     * Set the bookmark data that should be displayed, replacing any existing
+     * data.
+     */
+    set data(data) {
+      this.updateBookmarkTiles_(data.items);
+      this.updateBookmarkTitles_(data.navigationItems);
+    },
   };
 
+  /**
+   * Initializes and renders the bookmark chevron canvas.  This needs to be
+   * performed after the page has been loaded so that we have access to the
+   * style sheet values.
+   */
+  function initBookmarkChevron() {
+    var wrapperStyle = window.getComputedStyle($('bookmarks-title-wrapper'));
+    var width = 10;
+    var height = parseInt(wrapperStyle.height, 10);
+    var ctx = document.getCSSCanvasContext('2d', 'bookmark-chevron',
+                                           width, height);
+    ctx.strokeStyle = wrapperStyle.borderBottomColor;
+    ctx.beginPath();
+    ctx.moveTo(0, 0);
+    ctx.lineTo(width, height / 2);
+    ctx.lineTo(0, height);
+    ctx.stroke();
+  };
+
+  /**
+   * Set the dominant color for a bookmark tile.  This is the callback method
+   * from a request made when the tile was created.
+   * @param {number} id The numeric ID of the bookmark tile.
+   * @param {string} color The color represented as a CSS string.
+   */
+  function setBookmarksFaviconDominantColor(id, color) {
+    var tile = $('bookmark_tile_' + id);
+    if (tile)
+      tile.stripeColor = color;
+  };
+
   return {
-    BookmarksPage: BookmarksPage
+    BookmarksPage: BookmarksPage,
+    initBookmarkChevron: initBookmarkChevron,
+    setBookmarksFaviconDominantColor: setBookmarksFaviconDominantColor
   };
 });
+
+window.addEventListener('load', ntp4.initBookmarkChevron);