Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Commit

Permalink
fix(topsites): Limit the number of topsites being rendered after pinn…
Browse files Browse the repository at this point in the history
…ing (#3081)
  • Loading branch information
piatra authored and Mardak committed Aug 3, 2017
1 parent 835317a commit fc9a4ac
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 12 deletions.
6 changes: 4 additions & 2 deletions system-addon/common/Reducers.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

const {actionTypes: at} = Components.utils.import("resource://activity-stream/common/Actions.jsm", {});

const TOP_SITES_SHOWMORE_LENGTH = 12;

const INITIAL_STATE = {
App: {
// Have we received real data from the app yet?
Expand Down Expand Up @@ -139,7 +141,7 @@ function TopSites(prevState = INITIAL_STATE.TopSites, action) {
return Object.assign({}, prevState, {rows: newRows});
case at.PINNED_SITES_UPDATED:
pinned = action.data;
newRows = insertPinned(prevState.rows, pinned);
newRows = insertPinned(prevState.rows, pinned).slice(0, TOP_SITES_SHOWMORE_LENGTH);
return Object.assign({}, prevState, {rows: newRows});
default:
return prevState;
Expand Down Expand Up @@ -258,4 +260,4 @@ this.INITIAL_STATE = INITIAL_STATE;
this.reducers = {TopSites, App, Snippets, Prefs, Dialog, Sections};
this.insertPinned = insertPinned;

this.EXPORTED_SYMBOLS = ["reducers", "INITIAL_STATE", "insertPinned"];
this.EXPORTED_SYMBOLS = ["reducers", "INITIAL_STATE", "insertPinned", "TOP_SITES_SHOWMORE_LENGTH"];
6 changes: 2 additions & 4 deletions system-addon/lib/TopSitesFeed.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");

const {actionCreators: ac, actionTypes: at} = Cu.import("resource://activity-stream/common/Actions.jsm", {});
const {TippyTopProvider} = Cu.import("resource://activity-stream/lib/TippyTopProvider.jsm", {});
const {insertPinned} = Cu.import("resource://activity-stream/common/Reducers.jsm", {});
const {insertPinned, TOP_SITES_SHOWMORE_LENGTH} = Cu.import("resource://activity-stream/common/Reducers.jsm", {});
const {Dedupe} = Cu.import("resource://activity-stream/common/Dedupe.jsm", {});
const {shortURL} = Cu.import("resource://activity-stream/common/ShortURL.jsm", {});

Expand All @@ -17,7 +17,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils",
XPCOMUtils.defineLazyModuleGetter(this, "Screenshots",
"resource://activity-stream/lib/Screenshots.jsm");

const TOP_SITES_SHOWMORE_LENGTH = 12;
const UPDATE_TIME = 15 * 60 * 1000; // 15 minutes
const DEFAULT_SITES_PREF = "default.sites";
const DEFAULT_TOP_SITES = [];
Expand Down Expand Up @@ -173,6 +172,5 @@ this.TopSitesFeed = class TopSitesFeed {
};

this.UPDATE_TIME = UPDATE_TIME;
this.TOP_SITES_SHOWMORE_LENGTH = TOP_SITES_SHOWMORE_LENGTH;
this.DEFAULT_TOP_SITES = DEFAULT_TOP_SITES;
this.EXPORTED_SYMBOLS = ["TopSitesFeed", "UPDATE_TIME", "DEFAULT_TOP_SITES", "TOP_SITES_SHOWMORE_LENGTH"];
this.EXPORTED_SYMBOLS = ["TopSitesFeed", "UPDATE_TIME", "DEFAULT_TOP_SITES"];
12 changes: 11 additions & 1 deletion system-addon/test/unit/common/Reducers.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const {reducers, INITIAL_STATE, insertPinned} = require("common/Reducers.jsm");
const {reducers, INITIAL_STATE, insertPinned, TOP_SITES_SHOWMORE_LENGTH} = require("common/Reducers.jsm");
const {TopSites, App, Snippets, Prefs, Dialog, Sections} = reducers;

const {actionTypes: at} = require("common/Actions.jsm");
Expand Down Expand Up @@ -126,6 +126,16 @@ describe("Reducers", () => {
const nextState = TopSites(oldState, action);
assert.deepEqual(nextState.rows, [{url: "baz.com", title: "baz", isPinned: true, pinIndex: 0, pinTitle: "baz"}, {url: "foo.com"}, {url: "bar.com"}]);
});
it("should return at most TOP_SITES_SHOWMORE_LENGTH sites on PINNED_SITES_UPDATED", () => {
const oldState = {rows: [{url: "foo.com"}, {url: "bar.com"}]};
const data = new Array(20).fill(null).map((s, i) => ({
url: "foo.com",
pinIndex: i
}));
const action = {type: at.PINNED_SITES_UPDATED, data};
const nextState = TopSites(oldState, action);
assert.lengthOf(nextState.rows, TOP_SITES_SHOWMORE_LENGTH);
});
});
describe("Prefs", () => {
function prevState(custom = {}) {
Expand Down
10 changes: 5 additions & 5 deletions system-addon/test/unit/lib/TopSitesFeed.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"use strict";
const injector = require("inject!lib/TopSitesFeed.jsm");
const {UPDATE_TIME, TOP_SITES_SHOWMORE_LENGTH} = require("lib/TopSitesFeed.jsm");
const {UPDATE_TIME} = require("lib/TopSitesFeed.jsm");
const {FakePrefs, GlobalOverrider} = require("test/unit/utils");
const action = {meta: {fromTarget: {}}};
const {actionCreators: ac, actionTypes: at} = require("common/Actions.jsm");
const {insertPinned} = require("common/Reducers.jsm");
const {insertPinned, TOP_SITES_SHOWMORE_LENGTH} = require("common/Reducers.jsm");
const FAKE_LINKS = new Array(TOP_SITES_SHOWMORE_LENGTH).fill(null).map((v, i) => ({url: `http://www.site${i}.com`}));
const FAKE_SCREENSHOT = "data123";

Expand Down Expand Up @@ -46,7 +46,7 @@ describe("Top Sites Feed", () => {
({TopSitesFeed, DEFAULT_TOP_SITES} = injector({
"lib/ActivityStreamPrefs.jsm": {Prefs: FakePrefs},
"common/Dedupe.jsm": {Dedupe: fakeDedupe},
"common/Reducers.jsm": {insertPinned},
"common/Reducers.jsm": {insertPinned, TOP_SITES_SHOWMORE_LENGTH},
"lib/Screenshots.jsm": {Screenshots: fakeScreenshot},
"lib/TippyTopProvider.jsm": {TippyTopProvider: FakeTippyTopProvider},
"common/ShortURL.jsm": {shortURL: shortURLStub}
Expand Down Expand Up @@ -144,7 +144,7 @@ describe("Top Sites Feed", () => {
beforeEach(() => {
({TopSitesFeed, DEFAULT_TOP_SITES} = injector({
"lib/ActivityStreamPrefs.jsm": {Prefs: FakePrefs},
"common/Reducers.jsm": {insertPinned},
"common/Reducers.jsm": {insertPinned, TOP_SITES_SHOWMORE_LENGTH},
"lib/Screenshots.jsm": {Screenshots: fakeScreenshot}
}));
feed = new TopSitesFeed();
Expand All @@ -157,7 +157,7 @@ describe("Top Sites Feed", () => {

const sites = await feed.getLinksWithDefaults();

assert.lengthOf(sites, 12);
assert.lengthOf(sites, TOP_SITES_SHOWMORE_LENGTH);
assert.equal(sites[0].url, fakeNewTabUtils.pinnedLinks.links[0].url);
assert.equal(sites[1].url, fakeNewTabUtils.pinnedLinks.links[1].url);
assert.equal(sites[0].hostname, sites[1].hostname);
Expand Down

0 comments on commit fc9a4ac

Please sign in to comment.