Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
fix unwanted persisted pinned top site
Browse files Browse the repository at this point in the history
-
default pinned top site was persistent even after restart. changes address it
  • Loading branch information
cezaraugusto committed Jan 4, 2018
1 parent 06e44b3 commit f95dd73
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 9 deletions.
8 changes: 6 additions & 2 deletions app/browser/api/topSites.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const getTopSiteData = () => {
const preDefined = staticData
// TODO: this doesn't work properly
.filter((site) => {
return !isIgnored(state, site.get('key'))
return !isPinned(state, site.get('key')) && !isIgnored(state, site.get('key'))
})
.map(site => {
const bookmarkKey = bookmarkLocationCache.getCacheKey(state, site.get('location'))
Expand All @@ -133,7 +133,12 @@ const getTopSiteData = () => {
}

const pinnedTopSites = aboutNewTabState.getPinnedTopSites(state)

let gridSites = pinnedTopSites.map(pinned => {
// do not allow duplicates
if (pinned) {
sites = sites.filter(site => site.get('key') !== pinned.get('key'))
}
// topsites are populated once user visit a new site.
// pinning a site to a given index is a user decision
// and should be taken as priority. If there's an empty
Expand All @@ -148,7 +153,6 @@ const getTopSiteData = () => {
})

gridSites = gridSites.filter(site => site != null)
gridSites = removeDuplicateDomains(gridSites)

appActions.topSiteDataAvailable(gridSites)
}
Expand Down
15 changes: 12 additions & 3 deletions app/common/state/aboutNewTabState.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
const Immutable = require('immutable')
const {makeImmutable} = require('./immutableUtil')
const topSites = require('../../browser/api/topSites')
const newTabData = require('../../../js/data/newTabData')
/**
* topSites are defined by users. Pinned sites are attached to their positions
* in the grid, and the non pinned indexes are populated with newly accessed sites
*/

const defaultPinnedSite = Immutable.fromJS(newTabData.pinnedTopSites)
const aboutNewTabState = {
getSites: (state) => {
return state.getIn(['about', 'newtab', 'sites'])
Expand All @@ -18,9 +19,18 @@ const aboutNewTabState = {
getPinnedTopSites: (state) => {
// add same number as fallback to avoid race condition on startup
const maxEntries = topSites.aboutNewTabMaxEntries || 100

const pinnedTopSites = state
.getIn(['about', 'newtab', 'pinnedTopSites'], Immutable.List())

// pinned list is only empty on startup. Since we have a default pinned site
// let's add it to the list
if (pinnedTopSites.isEmpty()) {
state = state.setIn(['about', 'newtab', 'pinnedTopSites'], defaultPinnedSite)
}
// we need null spaces in order to proper pin a topSite in the right position.
// so let's set it to the same number as max new tab entries.
return state.getIn(['about', 'newtab', 'pinnedTopSites'], Immutable.List()).setSize(maxEntries)
return pinnedTopSites.setSize(maxEntries)
},

getIgnoredTopSites: (state) => {
Expand All @@ -32,7 +42,6 @@ const aboutNewTabState = {
if (!props) {
return state
}

state = state.mergeIn(['about', 'newtab'], props.newTabPageDetail)
return state.setIn(['about', 'newtab', 'updatedStamp'], new Date().getTime())
},
Expand Down
3 changes: 1 addition & 2 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const windowState = require('./common/state/windowState')

// Utils
const locale = require('./locale')
const {pinnedTopSites} = require('../js/data/newTabData')
const {defaultSiteSettingsList} = require('../js/data/siteSettingsList')
const filtering = require('./filtering')
const autofill = require('./autofill')
Expand Down Expand Up @@ -1019,7 +1018,7 @@ module.exports.defaultAppState = () => {
gridLayoutSize: 'small',
sites: [],
ignoredTopSites: [],
pinnedTopSites: pinnedTopSites
pinnedTopSites: []
},
welcome: {
showOnLoad: !['test', 'development'].includes(process.env.NODE_ENV)
Expand Down
3 changes: 1 addition & 2 deletions js/about/newtab.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class NewTabPage extends React.Component {
}

get pinnedTopSites () {
return this.state.newTabData.getIn(['newTabDetail', 'pinnedTopSites'], Immutable.List())
return this.state.newTabData.getIn(['newTabDetail', 'pinnedTopSites'], Immutable.List()).setSize(100)
}

get ignoredTopSites () {
Expand All @@ -120,7 +120,6 @@ class NewTabPage extends React.Component {
get gridLayout () {
const sizeToCount = {large: 18, medium: 12, small: 6}
const count = sizeToCount[this.gridLayoutSize]

return this.topSites.take(count)
}

Expand Down

0 comments on commit f95dd73

Please sign in to comment.