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

Fix gridLayout pinning action bug #5406

Merged
merged 1 commit into from
Nov 4, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 20 additions & 24 deletions js/about/newtab.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,10 @@ class NewTabPage extends React.Component {
const updatedStamp = sanitizedData.getIn(['newTabDetail', 'updatedStamp'])

// Only update if the data has changed.
// console.log(updatedStamp + ']DATA_TS==LAST_TS[' + this.state.updatedStamp)
if (updatedStamp === this.state.updatedStamp) {
return
}

// Ensure top sites only has unique values
const pinnedTopSites = sanitizedData.getIn(['newTabDetail', 'pinnedTopSites'])
if (pinnedTopSites) {
const uniqueValues = pinnedTopSites.filter((element, index, list) => {
if (!element) return false
return index === list.findIndex((site) => site && site.get('location') === element.get('location'))
})
sanitizedData = sanitizedData.setIn(['newTabDetail', 'pinnedTopSites'], uniqueValues)
}

this.setState({
newTabData: sanitizedData,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this logic is removed (which is OK), maybe we can move what's left into ipc.on() line 😄 I originally had added this because my state got into a situation where I had many duplicates, which caused problems

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(can be done as a follow up in a later commit)

updatedStamp: updatedStamp
Expand All @@ -82,7 +71,7 @@ class NewTabPage extends React.Component {
}

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

get ignoredTopSites () {
Expand All @@ -97,6 +86,10 @@ class NewTabPage extends React.Component {
return this.pinnedTopSites.includes(siteProps)
}

isIgnored (siteProps) {
return this.ignoredTopSites.includes(siteProps)
}

isBookmarked (siteProps) {
return siteUtil.isSiteBookmarked(this.topSites, siteProps)
}
Expand All @@ -106,21 +99,23 @@ class NewTabPage extends React.Component {
* in the grid, and the non pinned indexes are populated with newly accessed sites
*/
get topSites () {
let gridSites = Immutable.List().setSize(18)
let sites = this.sites
const pinnedTopSites = this.pinnedTopSites.setSize(gridSites.size)
const ignoredTopSites = this.ignoredTopSites
const pinnedTopSites = this.pinnedTopSites
const sites = this.sites
let unpinnedTopSites = sites.filter((site) => !this.isPinned(site) ? site : null)
let gridSites

const getUnpinned = () => {
const firstSite = unpinnedTopSites.first()
unpinnedTopSites = unpinnedTopSites.shift()
return firstSite
}

// We need to know which sites are pinned first, so we can skip them while populating
gridSites = gridSites.push.apply(pinnedTopSites, gridSites)
gridSites = gridSites.map((item, i) => item == null ? sites.get(i) : item)
gridSites = pinnedTopSites.map(pinned => pinned || getUnpinned())

// Remove from grid all ignored sites
gridSites = gridSites.filter((site) => ignoredTopSites.indexOf(site) === -1)

gridSites = gridSites.filter(site => site != null)
gridSites = gridSites.filter((site) => !this.isIgnored(site))

return gridSites
return gridSites.filter(site => site != null)
}

get gridLayout () {
Expand Down Expand Up @@ -210,7 +205,7 @@ class NewTabPage extends React.Component {
const currentPositionIndex = gridSites.indexOf(currentPosition)

// If pinned, leave it null. Otherwise stores site on ignoredTopSites list, retaining the same position
let pinnedTopSites = this.pinnedTopSites.setSize(18)
let pinnedTopSites = this.pinnedTopSites
pinnedTopSites = pinnedTopSites.splice(currentPositionIndex, 1, this.isPinned(siteProps) ? null : siteProps)

aboutActions.setNewTabDetail({pinnedTopSites: pinnedTopSites})
Expand All @@ -234,6 +229,7 @@ class NewTabPage extends React.Component {
}

onUndoIgnoredTopSite () {
// Remove last List's entry
const ignoredTopSites = this.ignoredTopSites.splice(-1, 1)
aboutActions.setNewTabDetail({ignoredTopSites: ignoredTopSites})
this.hideSiteRemovalNotification()
Expand Down