Merge pull request #5600 from bsclifton/showing-history-some-love

History fixes and misc about page fixes

app/common/lib/historyUtil.js

@@ -2,7 +2,26 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 'use strict'
+
 const Immutable = require('immutable')
+const {makeImmutable} = require('../state/immutableUtil')
+const siteUtil = require('../../../js/state/siteUtil')
+const aboutHistoryMaxEntries = 500
+
+module.exports.maxEntries = aboutHistoryMaxEntries
+
+const sortTimeDescending = (left, right) => {
+  if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1
+  if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1
+  return 0
+}
+
+module.exports.getHistory = (sites) => {
+  sites = makeImmutable(sites) || new Immutable.List()
+  return sites.filter((site) => siteUtil.isHistoryEntry(site))
+      .sort(sortTimeDescending)
+      .slice(0, aboutHistoryMaxEntries)
+}
 
 const getDayString = (entry, locale) => {
   const lastAccessedTime = entry.get('lastAccessedTime')
@@ -42,15 +61,11 @@ module.exports.groupEntriesByDay = (history, locale) => {
  * Format is expected to be array containing one array per day.
  */
 module.exports.totalEntries = (entriesByDay) => {
+  entriesByDay = makeImmutable(entriesByDay) || new Immutable.List()
+
   let result = new Immutable.List()
   entriesByDay.forEach((entry) => {
     result = result.push(entry.get('entries'))
   })
   return result
 }
-
-module.exports.sortTimeDescending = (left, right) => {
-  if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1
-  if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1
-  return 0
-}

app/common/state/aboutHistoryState.js

@@ -0,0 +1,21 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const {makeImmutable} = require('./immutableUtil')
+const historyUtil = require('../lib/historyUtil')
+
+const aboutHistoryState = {
+  getHistory: (state) => {
+    state = makeImmutable(state)
+    return state.getIn(['about', 'history'])
+  },
+  setHistory: (state) => {
+    state = makeImmutable(state)
+    state = state.setIn(['about', 'history', 'entries'],
+      historyUtil.getHistory(state.get('sites')))
+    return state.setIn(['about', 'history', 'updatedStamp'], new Date().getTime())
+  }
+}
+
+module.exports = aboutHistoryState

app/renderer/components/urlBarIcon.js

@@ -47,10 +47,15 @@ class UrlBarIcon extends ImmutableComponent {
 
     const defaultToSearch = (!this.isSecure && !this.isInsecure && !showSearch) &&
                             !this.props.titleMode &&
-                            this.props.loading === false
+                            this.props.loading === false &&
+                            !this.isAboutPage
 
     return showSearch || defaultToSearch
   }
+  get isAboutPage () {
+    return isSourceAboutUrl(this.props.location) &&
+      this.props.location !== 'about:newtab'
+  }
   get iconClasses () {
     if (this.props.activateSearchEngine) {
       return cx({urlbarIcon: true})
@@ -62,7 +67,8 @@ class UrlBarIcon extends ImmutableComponent {
       // NOTE: EV style not approved yet; see discussion at https://github.com/brave/browser-laptop/issues/791
       'fa-lock': this.isSecure,
       'fa-exclamation-triangle': this.isInsecure,
-      'fa-search': this.isSearch
+      'fa-search': this.isSearch,
+      'fa-list': this.isAboutPage
     })
   }
   get iconStyles () {

app/sessionStore.js

@@ -327,7 +327,10 @@ module.exports.cleanAppData = (data, isShutdown) => {
     })
   }
 
-  delete data.versionInformation
+  if (data.about) {
+    delete data.about.brave
+    delete data.about.history
+  }
 }
 
 /**
@@ -435,18 +438,21 @@ module.exports.loadAppState = () => {
       // version information (shown on about:brave)
       const os = require('os')
       const versionInformation = [
-        {name: 'brave', version: app.getVersion()},
-        {name: 'muon', version: process.versions['atom-shell']},
+        {name: 'Brave', version: app.getVersion()},
+        {name: 'Muon', version: process.versions['atom-shell']},
         {name: 'libchromiumcontent', version: process.versions['chrome']},
         {name: 'V8', version: process.versions.v8},
         {name: 'Node.js', version: process.versions.node},
-        {name: 'channel', version: Channel.channel()},
+        {name: 'Update Channel', version: Channel.channel()},
         {name: 'os.platform', version: os.platform()},
         {name: 'os.release', version: os.release()},
         {name: 'os.arch', version: os.arch()}
         // TODO(bsclifton): read the latest commit hash from a file, etc.
       ]
-      data.versionInformation = versionInformation
+      data.about = data.about || {}
+      data.about.brave = {
+        versionInformation: versionInformation
+      }
     } catch (e) {
       // TODO: Session state is corrupted, maybe we should backup this
       // corrupted value for people to report into support.

docs/appActions.md

@@ -494,6 +494,12 @@ Dispatch a message to indicate default browser check is complete
 
 
 
+### populateHistory() 
+
+Notify the AppStore to provide default history values.
+
+
+
 
 * * *
 

docs/state.md

@@ -220,6 +220,15 @@ AppStore
     }
   },
   about: {
+    brave: {
+      versionInformation: [{
+        name: string,
+        version: string
+      }] // used on about:brave. not persisted (removed on save)
+    },
+    history: {
+      entries: [object] // used on about:history. not persisted (removed on save)
+    },
     newtab: {
       gridLayoutSize: string, // 'small', 'medium', 'large'
       sites: [string], // List of sites to be used on gridLayout

js/about/history.js

@@ -13,7 +13,6 @@ const aboutActions = require('./aboutActions')
 const getSetting = require('../settings').getSetting
 const SortableTable = require('../components/sortableTable')
 const Button = require('../components/button')
-const siteUtil = require('../state/siteUtil')
 const {makeImmutable} = require('../../app/common/state/immutableUtil')
 const historyUtil = require('../../app/common/lib/historyUtil')
 
@@ -56,13 +55,6 @@ class HistoryTimeCell extends ImmutableComponent {
 }
 
 class HistoryDay extends ImmutableComponent {
-  constructor () {
-    super()
-    this.clearSelection = this.clearSelection.bind(this)
-  }
-  clearSelection () {
-    this.refs.historyTable.clearSelection()
-  }
   navigate (entry) {
     entry = makeImmutable(entry)
     aboutActions.newFrame({
@@ -74,7 +66,6 @@ class HistoryDay extends ImmutableComponent {
     return <div>
       <div className='sectionTitle historyDayName'>{this.props.date}</div>
       <SortableTable headings={['time', 'title', 'domain']}
-        ref='historyTable'
         defaultHeading='time'
         defaultHeadingSortOrder='desc'
         rows={this.props.entries.map((entry) => [
@@ -101,36 +92,23 @@ class HistoryDay extends ImmutableComponent {
   }
 }
 
-class GroupedHistoryList extends ImmutableComponent {
-  constructor () {
-    super()
-    this.entriesByDay = []
-    this.clearSelection = this.clearSelection.bind(this)
-  }
-  clearSelection () {
-    this.entriesByDay.forEach((day) => day.clearSelection())
-  }
+class GroupedHistoryList extends React.Component {
   render () {
     const defaultLanguage = this.props.languageCodes.find((lang) => lang.includes(navigator.language)) || 'en-US'
     const userLanguage = getSetting(settings.LANGUAGE, this.props.settings) || defaultLanguage
     const entriesByDay = historyUtil.groupEntriesByDay(this.props.history, userLanguage)
     const totalEntries = historyUtil.totalEntries(entriesByDay)
     let index = 0
-    this.entriesByDay = []
     return <list className='historyList'>
       {
-        entriesByDay.map((groupedEntry) => {
-          const day = <HistoryDay
-            ref={'historyDay' + index}
+        entriesByDay.map((groupedEntry) =>
+          <HistoryDay
             date={groupedEntry.get('date')}
             entries={groupedEntry.get('entries')}
             totalEntries={totalEntries}
-            tableID={index}
+            tableID={index++}
             stateOwner={this.props.stateOwner}
-          />
-          this.entriesByDay.push(this.refs[('historyDay' + index++)])
-          return day
-        })
+          />)
       }
     </list>
   }
@@ -149,10 +127,22 @@ class AboutHistory extends React.Component {
       search: '',
       settings: Immutable.Map(),
       languageCodes: Immutable.Map(),
-      selection: Immutable.Set()
+      selection: Immutable.Set(),
+      updatedStamp: undefined
     }
     ipc.on(messages.HISTORY_UPDATED, (e, detail) => {
-      this.setState({ history: Immutable.fromJS(detail && detail.history || {}) })
+      const aboutHistory = Immutable.fromJS(detail || {})
+      const updatedStamp = aboutHistory.get('updatedStamp')
+      // Only update if the data has changed.
+      if (typeof updatedStamp === 'number' &&
+          typeof this.state.updatedStamp === 'number' &&
+          updatedStamp === this.state.updatedStamp) {
+        return
+      }
+      this.setState({
+        history: aboutHistory.get('entries') || new Immutable.List(),
+        updatedStamp: updatedStamp
+      })
     })
     ipc.on(messages.SETTINGS_UPDATED, (e, settings) => {
       this.setState({ settings: Immutable.fromJS(settings || {}) })
@@ -180,7 +170,6 @@ class AboutHistory extends React.Component {
       }
       targetElement = targetElement.parentNode
     }
-
     // Click was not a child element of sortableTable; clear selection
     this.clearSelection()
   }
@@ -190,16 +179,13 @@ class AboutHistory extends React.Component {
       return title.match(new RegExp(searchTerm, 'gi'))
     })
   }
-  get historyDescendingOrder () {
-    return this.state.history.filter((site) => siteUtil.isHistoryEntry(site))
-      .sort(historyUtil.sortTimeDescending)
-      .slice(0, 500)
-  }
   clearBrowsingDataNow () {
     aboutActions.clearBrowsingDataNow({browserHistory: true})
   }
   clearSelection () {
-    this.refs.historyList.clearSelection()
+    this.setState({
+      selection: new Immutable.Set()
+    })
   }
   componentDidMount () {
     this.refs.historySearch.focus()
@@ -223,13 +209,12 @@ class AboutHistory extends React.Component {
 
       <div className='siteDetailsPageContent'>
         <GroupedHistoryList
-          ref='historyList'
           languageCodes={this.state.languageCodes}
           settings={this.state.settings}
           history={
             this.state.search
-            ? this.searchedSiteDetails(this.state.search, this.historyDescendingOrder)
-            : this.historyDescendingOrder
+            ? this.searchedSiteDetails(this.state.search, this.state.history)
+            : this.state.history
           }
           stateOwner={this} />
       </div>

js/actions/appActions.js

@@ -579,6 +579,15 @@ const appActions = {
     AppDispatcher.dispatch({
       actionType: AppConstants.APP_DEFAULT_BROWSER_CHECK_COMPLETE
     })
+  },
+
+  /**
+   * Notify the AppStore to provide default history values.
+   */
+  populateHistory: function () {
+    AppDispatcher.dispatch({
+      actionType: AppConstants.APP_POPULATE_HISTORY
+    })
   }
 }
 

js/components/frame.js

@@ -96,9 +96,10 @@ class Frame extends ImmutableComponent {
         bookmarkFolders: this.props.bookmarkFolders.toJS()
       })
     } else if (location === 'about:history') {
-      this.webview.send(messages.HISTORY_UPDATED, {
-        history: this.props.history.toJS()
-      })
+      const aboutHistoryState = this.props.history && this.props.history.toJS
+        ? this.props.history.toJS()
+        : {}
+      this.webview.send(messages.HISTORY_UPDATED, aboutHistoryState)
       this.webview.send(messages.SETTINGS_UPDATED, this.props.settings ? this.props.settings.toJS() : null)
     } else if (location === 'about:extensions') {
       this.webview.send(messages.EXTENSIONS_UPDATED, {
@@ -171,7 +172,7 @@ class Frame extends ImmutableComponent {
         this.webview.send(messages.AUTOFILL_CREDIT_CARDS_UPDATED, list)
       }
     } else if (location === 'about:brave') {
-      const versionInformation = appStoreRenderer.state.get('versionInformation')
+      const versionInformation = appStoreRenderer.state.getIn(['about', 'brave', 'versionInformation'])
       if (versionInformation && versionInformation.toJS) {
         this.webview.send(messages.VERSION_INFORMATION_UPDATED, versionInformation.toJS())
       }