Fix drag and drop for about:newtab (and other fixes)

docs/state.md

@@ -224,7 +224,8 @@ AppStore
       gridLayoutSize: string, // 'small', 'medium', 'large'
       sites: [string], // List of sites to be used on gridLayout
       ignoredTopSites: [string], // List of ignored sites
-      pinnedTopSites: [string] // List of pinned sites
+      pinnedTopSites: [string], // List of pinned sites
+      updatedStamp: number // timestamp for when the data was last updated
     }
   },
   menu: {

js/about/newtab.js

@@ -31,10 +31,36 @@ class NewTabPage extends React.Component {
     this.state = {
       showSiteRemovalNotification: false,
       backgroundImage: this.randomBackgroundImage,
-      imageLoadFailed: false
+      imageLoadFailed: false,
+      updatedStamp: undefined
     }
     ipc.on(messages.NEWTAB_DATA_UPDATED, (e, newTabData) => {
-      this.setState({ newTabData: Immutable.fromJS(newTabData || {}) })
+      this.sanitize(newTabData)
+    })
+  }
+  sanitize (newTabData) {
+    let sanitizedData = Immutable.fromJS(newTabData || {})
+    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,
+      updatedStamp: updatedStamp
     })
   }
 
@@ -87,20 +113,11 @@ class NewTabPage extends React.Component {
 
     // We need to know which sites are pinned first, so we can skip them while populating
     gridSites = gridSites.push.apply(pinnedTopSites, gridSites)
-
-    for (let i = 0; i < gridSites.size; i++) {
-      // skip pinnedTopSites while populating
-      if (!this.isPinned(i)) {
-        gridSites = gridSites.set(i, sites.first())
-        sites = sites.shift()
-      }
-    }
+    gridSites = gridSites.map((item, i) => item == null ? sites.get(i) : item)
 
     // Remove from grid all ignored sites
     gridSites = gridSites.filter((site) => ignoredTopSites.indexOf(site) === -1)
 
-    // Filter duplicated and remove null
-    gridSites = gridSites.toSet().toList()
     gridSites = gridSites.filter(site => site != null)
 
     return gridSites
@@ -171,7 +188,14 @@ class NewTabPage extends React.Component {
       newTabState.pinnedTopSites = pinnedTopSites
     }
     newTabState.sites = gridSites
-    aboutActions.setNewTabDetail(newTabState)
+
+    // Only update if there was an actual change
+    const stateDiff = Immutable.fromJS(newTabState)
+    const existingState = this.state.newTabData || Immutable.fromJS({})
+    const proposedState = existingState.mergeIn(['newTabDetail'], stateDiff)
+    if (!proposedState.isSubset(existingState)) {
+      aboutActions.setNewTabDetail(stateDiff)
+    }
   }
 
   onToggleBookmark (siteProps) {
@@ -278,7 +302,9 @@ class NewTabPage extends React.Component {
                     href={site.get('location')}
                     favicon={
                       site.get('favicon') == null
-                      ? site.get('title').charAt(0).toUpperCase()
+                      ? site.get('title')
+                        ? site.get('title').charAt(0).toUpperCase()
+                        : '?'
                       : <img src={site.get('favicon')} />
                     }
                     style={{backgroundColor: site.get('themeColor')}}

js/stores/appStore.js

@@ -431,6 +431,7 @@ const handleAppAction = (action) => {
       break
     case AppConstants.APP_CHANGE_NEW_TAB_DETAIL:
       appState = appState.mergeIn(['about', 'newtab'], action.newTabPageDetail)
+      appState = appState.setIn(['about', 'newtab', 'updatedStamp'], new Date().getTime())
       break
     case AppConstants.APP_ADD_SITE:
       const oldSiteSize = appState.get('sites').size
@@ -448,10 +449,13 @@ const handleAppAction = (action) => {
       if (oldSiteSize !== appState.get('sites').size) {
         filterOutNonRecents()
       }
-      let newVisitedSites = appState.getIn(['about', 'newtab', 'sites'])
+      let newVisitedSites = appState.getIn(['about', 'newtab', 'sites']) || new Immutable.List()
       newVisitedSites = newVisitedSites.unshift(action.siteDetail)
       // Filter duplicated entries by its location
-      newVisitedSites = newVisitedSites.filter((set => site => !set.has(site.get('location')) && set.add(site.get('location')))(new Set()))
+      newVisitedSites = newVisitedSites.filter((element, index, list) => {
+        if (!element) return false
+        return index === list.findIndex((site) => site && site.get('location') === element.get('location'))
+      })
       newVisitedSites = newVisitedSites.take(18)
       // TODO: @cezaraugusto.
       // Sort should respect unshift and don't prioritize bookmarks
@@ -460,6 +464,7 @@ const handleAppAction = (action) => {
       // .sort(suggestion.sortByAccessCountWithAgeDecay)
 
       appState = appState.setIn(['about', 'newtab', 'sites'], siteUtil.addSite(newVisitedSites, action.siteDetail, action.tag, action.originalSiteDetail))
+      appState = appState.setIn(['about', 'newtab', 'updatedStamp'], new Date().getTime())
       break
     case AppConstants.APP_REMOVE_SITE:
       appState = appState.set('sites', siteUtil.removeSite(appState.get('sites'), action.siteDetail, action.tag))