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

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

Issue 7701013: [ntp4] Limit the maximum bookmark tiles that will display at once, for now. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: cosmetic tweak Created 9 years, 4 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/bookmarks_page.js
===================================================================
--- chrome/browser/resources/ntp4/bookmarks_page.js (revision 97886)
+++ chrome/browser/resources/ntp4/bookmarks_page.js (working copy)
@@ -14,6 +14,16 @@
var tileId = 0;
/**
+ * The maximum number of tiles that we will display on this page. If there
+ * are not enough spaces to show all bookmarks, we'll include a link to the
+ * Bookmarks manager.
+ • TODO(csilv): Eliminate the need for this restraint.
+ * @type {number}
+ * @const
+ */
+ var MAX_BOOKMARK_TILES = 60;
Evan Stade 2011/08/25 01:46:24 I'd do 18
csilv 2011/08/25 17:41:56 Done.
+
+ /**
* Creates a new bookmark object.
* @param {Object} data The url and title.
* @constructor
@@ -174,7 +184,19 @@
initialize: function() {
this.classList.add('bookmarks-page');
+
+ // insert the bookmark titles header which is unique to bookmark pages.
this.insertBefore($('bookmarks-title-wrapper'), this.firstChild);
+
+ // insert a container for a link to a Bookmarks Manager page.
+ var link = document.createElement('a');
+ link.className = 'bookmarks-manager-link';
+ link.textContent = localStrings.getString('bookmarksManagerLinkTitle');
+ var container = document.createElement('div');
+ container.className = 'bookmarks-manager-link-container';
+ container.hidden = true;
+ container.appendChild(link);
+ this.querySelector('.tile-page-content').appendChild(container);
},
/**
@@ -207,8 +229,18 @@
*/
updateBookmarkTiles_: function(items) {
this.removeAllTiles();
- for (var i = 0; i < items.length; i++)
+ var tile_count = Math.min(items.length, MAX_BOOKMARK_TILES);
+ for (var i = 0; i < tile_count; i++)
this.appendTile(new Bookmark(items[i]), false);
+
+ var container = this.querySelector('.bookmarks-manager-link-container');
+ if (items.length > MAX_BOOKMARK_TILES) {
+ var link = container.querySelector('.bookmarks-manager-link');
+ link.href = 'chrome://bookmarks/#' + this.id;
+ container.hidden = false;
+ } else {
+ container.hidden = true;
+ }
},
/** @inheritDoc */
@@ -221,6 +253,7 @@
* data.
*/
set data(data) {
+ this.id = data.navigationItems[0].id;
this.updateBookmarkTiles_(data.items);
this.updateBookmarkTitles_(data.navigationItems);
},

Powered by Google App Engine
This is Rietveld 408576698