Commit 02a7394c authored by Kristi Park's avatar Kristi Park Committed by Commit Bot

[NTP] Fix max number of shortcuts error for grid layout

If the page was loaded with Most Visited shortcuts, the maximum number
of shortcuts was set to 8. This was not updated if users swapped to
custom links, so an error was thrown.

Do update the maximum number of shortcuts, and remove the error since
we can ignore any shortcuts over the max.

Change-Id: I95e3941ed2c9f2b266075edad7f8a60bddddd93d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1793230
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: default avatarKyle Milka <kmilka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695403}
parent a36a08e0
...@@ -113,6 +113,12 @@ const RESIZE_TIMEOUT_DELAY = 66; ...@@ -113,6 +113,12 @@ const RESIZE_TIMEOUT_DELAY = 66;
*/ */
const MD_MAX_NUM_CUSTOM_LINK_TILES = 10; const MD_MAX_NUM_CUSTOM_LINK_TILES = 10;
/**
* Maximum number of tiles if Most Visited is enabled.
* @const {number}
*/
const MD_MAX_NUM_MOST_VISITED_TILES = 8;
/** /**
* Maximum number of tiles per row for Material Design. * Maximum number of tiles per row for Material Design.
* @const {number} * @const {number}
...@@ -162,15 +168,6 @@ let loadedCounter = 1; ...@@ -162,15 +168,6 @@ let loadedCounter = 1;
*/ */
let tiles = null; let tiles = null;
/**
* Maximum number of MostVisited tiles to show at any time. If the host page
* doesn't send enough tiles and custom links is not enabled, we fill them blank
* tiles. This can be changed depending on what feature is enabled. Set by the
* host page, while 8 is default.
* @type {number}
*/
let maxNumTiles = 8;
/** /**
* List of parameters passed by query args. * List of parameters passed by query args.
* @type {Object} * @type {Object}
...@@ -296,17 +293,13 @@ class Grid { ...@@ -296,17 +293,13 @@ class Grid {
this.tilesAlwaysVisible_ = this.tilesAlwaysVisible_ =
params.tilesAlwaysVisible || MD_NUM_TILES_ALWAYS_VISIBLE; params.tilesAlwaysVisible || MD_NUM_TILES_ALWAYS_VISIBLE;
this.maxTilesPerRow_ = params.maxTilesPerRow || MD_MAX_TILES_PER_ROW; this.maxTilesPerRow_ = params.maxTilesPerRow || MD_MAX_TILES_PER_ROW;
this.maxTiles_ = params.maxTiles || maxNumTiles; this.maxTiles_ = params.maxTiles || getMaxNumTiles();
this.maxTilesPerRowWindow_ = this.getMaxTilesPerRow_(); this.maxTilesPerRowWindow_ = this.getMaxTilesPerRow_();
this.tiles_ = this.tiles_ =
this.container_.getElementsByClassName(CLASSES.GRID_TILE_CONTAINER); this.container_.getElementsByClassName(CLASSES.GRID_TILE_CONTAINER);
if (this.tiles_.length > this.maxTiles_) { // Ignore any tiles past the maximum allowed.
throw new Error(
'The number of tiles (' + this.tiles_.length +
') exceeds the maximum (' + this.maxTiles_ + ').');
}
this.position_ = new Array(this.maxTiles_); this.position_ = new Array(this.maxTiles_);
this.order_ = new Array(this.maxTiles_); this.order_ = new Array(this.maxTiles_);
for (let i = 0; i < this.maxTiles_; i++) { for (let i = 0; i < this.maxTiles_; i++) {
...@@ -737,7 +730,7 @@ function logEvent(eventType) { ...@@ -737,7 +730,7 @@ function logEvent(eventType) {
/** /**
* Log impression of an NTP tile. * Log impression of an NTP tile.
* @param {number} tileIndex Position of the tile, >= 0 and < |maxNumTiles|. * @param {number} tileIndex Position of the tile, >= 0 and < getMaxNumTiles().
* @param {number} tileTitleSource The source of the tile's title as received * @param {number} tileTitleSource The source of the tile's title as received
* from getMostVisitedItemData. * from getMostVisitedItemData.
* @param {number} tileSource The tile's source as received from * @param {number} tileSource The tile's source as received from
...@@ -754,7 +747,7 @@ function logMostVisitedImpression( ...@@ -754,7 +747,7 @@ function logMostVisitedImpression(
/** /**
* Log click on an NTP tile. * Log click on an NTP tile.
* @param {number} tileIndex Position of the tile, >= 0 and < |maxNumTiles|. * @param {number} tileIndex Position of the tile, >= 0 and < getMaxNumTiles().
* @param {number} tileTitleSource The source of the tile's title as received * @param {number} tileTitleSource The source of the tile's title as received
* from getMostVisitedItemData. * from getMostVisitedItemData.
* @param {number} tileSource The tile's source as received from * @param {number} tileSource The tile's source as received from
...@@ -771,12 +764,23 @@ function logMostVisitedNavigation( ...@@ -771,12 +764,23 @@ function logMostVisitedNavigation(
/** /**
* Returns true if custom links are enabled. * Returns true if custom links are enabled.
* @return {boolean}
*/ */
function isCustomLinksEnabled() { function isCustomLinksEnabled() {
return customLinksFeatureEnabled && return customLinksFeatureEnabled &&
!chrome.embeddedSearch.newTabPage.isUsingMostVisited; !chrome.embeddedSearch.newTabPage.isUsingMostVisited;
} }
/**
* Returns the maximum number of tiles to show at any time. This can be changed
* depending on what feature is enabled.
* @return {number}
*/
function getMaxNumTiles() {
return isCustomLinksEnabled() ? MD_MAX_NUM_CUSTOM_LINK_TILES :
MD_MAX_NUM_MOST_VISITED_TILES;
}
/** /**
* Down counts the DOM elements that we are waiting for the page to load. * Down counts the DOM elements that we are waiting for the page to load.
* When we get to 0, we send a message to the parent window. * When we get to 0, we send a message to the parent window.
...@@ -905,7 +909,7 @@ function swapInNewTiles() { ...@@ -905,7 +909,7 @@ function swapInNewTiles() {
// Add an "add new custom link" button if we haven't reached the maximum // Add an "add new custom link" button if we haven't reached the maximum
// number of tiles. // number of tiles.
if (isCustomLinksEnabled() && cur.childNodes.length < maxNumTiles) { if (isCustomLinksEnabled() && cur.childNodes.length < getMaxNumTiles()) {
const data = { const data = {
'rid': -1, 'rid': -1,
'title': queryArgs['addLink'], 'title': queryArgs['addLink'],
...@@ -989,27 +993,25 @@ function updateTileVisibility() { ...@@ -989,27 +993,25 @@ function updateTileVisibility() {
/** /**
* Handler for the 'show' message from the host page, called when it wants to * Handler for the 'show' message from the host page, called when it wants to
* add a suggestion tile. * add a suggestion tile.
* It's also used to fill up our tiles to |maxNumTiles| if necessary.
* @param {?MostVisitedData} args Data for the tile to be rendered. * @param {?MostVisitedData} args Data for the tile to be rendered.
*/ */
function addTile(args) { function addTile(args) {
if (isFinite(args.rid)) { if (!isFinite(args.rid)) {
// An actual suggestion. Grab the data from the embeddedSearch API. return;
const data = }
chrome.embeddedSearch.newTabPage.getMostVisitedItemData(args.rid);
if (!data) {
return;
}
if (!data.faviconUrl) { // Grab the tile's data from the embeddedSearch API.
data.faviconUrl = 'chrome-search://favicon/size/16@' + const data =
window.devicePixelRatio + 'x/' + data.renderViewId + '/' + data.rid; chrome.embeddedSearch.newTabPage.getMostVisitedItemData(args.rid);
} if (!data) {
tiles.appendChild(renderTile(data)); return;
} else { }
// An empty tile
tiles.appendChild(renderTile(null)); if (!data.faviconUrl) {
data.faviconUrl = 'chrome-search://favicon/size/16@' +
window.devicePixelRatio + 'x/' + data.renderViewId + '/' + data.rid;
} }
tiles.appendChild(renderTile(data));
} }
/** /**
...@@ -1348,11 +1350,6 @@ function init() { ...@@ -1348,11 +1350,6 @@ function init() {
document.body.classList.add(CLASSES.GRID_LAYOUT); document.body.classList.add(CLASSES.GRID_LAYOUT);
} }
// Set the maximum number of tiles to show.
if (isCustomLinksEnabled()) {
maxNumTiles = MD_MAX_NUM_CUSTOM_LINK_TILES;
}
currGrid = new Grid(); currGrid = new Grid();
// Set up layout updates on window resize. Throttled according to // Set up layout updates on window resize. Throttled according to
// |RESIZE_TIMEOUT_DELAY|. // |RESIZE_TIMEOUT_DELAY|.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment