Merge pull request #5370 from bsclifton/fix-history-double-click

Pulled code from about:history into a new historyUtil module

app/common/lib/historyUtil.js

@@ -0,0 +1,56 @@
+/* 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/. */
+
+'use strict'
+const Immutable = require('immutable')
+
+const getDayString = (entry, locale) => {
+  const lastAccessedTime = entry.get('lastAccessedTime')
+  return lastAccessedTime
+    ? new Date(lastAccessedTime).toLocaleDateString(locale, { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' })
+    : ''
+}
+
+module.exports.groupEntriesByDay = (history, locale) => {
+  const reduced = history.reduce((previousValue, currentValue, currentIndex, array) => {
+    const result = currentIndex === 1 ? [] : previousValue
+    if (currentIndex === 1) {
+      const firstDate = getDayString(currentValue, locale)
+      result.push({date: firstDate, entries: [previousValue]})
+    }
+    const date = getDayString(currentValue, locale)
+    const dateIndex = result.findIndex((entryByDate) => entryByDate.date === date)
+    if (dateIndex !== -1) {
+      result[dateIndex].entries.push(currentValue)
+    } else {
+      result.push({date: date, entries: [currentValue]})
+    }
+    return result
+  })
+  if (reduced) {
+    return Immutable.fromJS(
+      Array.isArray(reduced)
+        ? reduced
+        : [{date: getDayString(reduced, locale), entries: [reduced]}]
+    )
+  }
+  return Immutable.fromJS([])
+}
+
+/**
+ * Return an array with ALL entries.
+ * Format is expected to be array containing one array per day.
+ */
+module.exports.totalEntries = (entriesByDay) => {
+  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
+}

js/about/history.js

@@ -14,6 +14,8 @@ 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')
 
 const ipc = window.chrome.ipc
 
@@ -57,6 +59,7 @@ class HistoryTimeCell extends ImmutableComponent {
 
 class HistoryDay extends ImmutableComponent {
   navigate (entry) {
+    entry = makeImmutable(entry)
     aboutActions.newFrame({
       location: entry.get('location'),
       partitionNumber: entry.get('partitionNumber')
@@ -79,7 +82,7 @@ class HistoryDay extends ImmutableComponent {
           urlutils.getHostname(entry.get('location'), true)
         ])}
         rowObjects={this.props.entries}
-        totalRowObjects={this.props.totalEntries}
+        totalRowObjects={this.props.totalEntries.toJS()}
         tableID={this.props.tableID}
         columnClassNames={['time', 'title', 'domain']}
         addHoverClass
@@ -93,53 +96,19 @@ class HistoryDay extends ImmutableComponent {
 }
 
 class GroupedHistoryList extends ImmutableComponent {
-  getDayString (locale, item) {
-    const lastAccessedTime = item.get('lastAccessedTime')
-    return lastAccessedTime
-      ? new Date(lastAccessedTime).toLocaleDateString(locale, { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' })
-      : ''
-  }
-  groupEntriesByDay (locale) {
-    const reduced = this.props.history.reduce((previousValue, currentValue, currentIndex, array) => {
-      const result = currentIndex === 1 ? [] : previousValue
-      if (currentIndex === 1) {
-        const firstDate = this.getDayString(locale, currentValue)
-        result.push({date: firstDate, entries: [previousValue]})
-      }
-      const date = this.getDayString(locale, currentValue)
-      const dateIndex = result.findIndex((entryByDate) => entryByDate.date === date)
-      if (dateIndex !== -1) {
-        result[dateIndex].entries.push(currentValue)
-      } else {
-        result.push({date: date, entries: [currentValue]})
-      }
-      return result
-    })
-    if (reduced) {
-      return Array.isArray(reduced)
-        ? reduced
-        : [{date: this.getDayString(locale, reduced), entries: [reduced]}]
-    }
-    return []
-  }
-  totalEntries (entriesByDay) {
-    const result = []
-    entriesByDay.forEach((entry) => {
-      result.push(entry.entries)
-    })
-    return result
-  }
   render () {
     const defaultLanguage = this.props.languageCodes.find((lang) => lang.includes(navigator.language)) || 'en-US'
-    const userLanguage = getSetting(settings.LANGUAGE, this.props.settings)
-    const entriesByDay = this.groupEntriesByDay(userLanguage || defaultLanguage)
+    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
     return <list className='historyList'>
       {
         entriesByDay.map((groupedEntry) =>
-          <HistoryDay date={groupedEntry.date}
-            entries={groupedEntry.entries}
-            totalEntries={this.totalEntries(entriesByDay)}
+          <HistoryDay
+            date={groupedEntry.get('date')}
+            entries={groupedEntry.get('entries')}
+            totalEntries={totalEntries}
             tableID={index++}
             stateOwner={this.props.stateOwner}
           />)
@@ -189,11 +158,8 @@ class AboutHistory extends React.Component {
   }
   get historyDescendingOrder () {
     return this.state.history.filter((site) => siteUtil.isHistoryEntry(site))
-      .sort((left, right) => {
-        if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1
-        if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1
-        return 0
-      }).slice(-500)
+      .sort(historyUtil.sortTimeDescending)
+      .slice(-500)
   }
   clearBrowsingDataNow () {
     aboutActions.clearBrowsingDataNow({browserHistory: true})

test/unit/lib/historyUtilTest.js

@@ -0,0 +1,92 @@
+/* global describe, it */
+const assert = require('assert')
+const Immutable = require('immutable')
+const historyUtil = require('../../../app/common/lib/historyUtil')
+require('../braveUnit')
+
+describe('historyUtil', function () {
+  const historyDayOne = Immutable.fromJS({
+    lastAccessedTime: 1477944718876,
+    location: 'https://brave.com/page1',
+    title: 'sample 1',
+    tags: []
+  })
+  const historyDayTwo = Immutable.fromJS({
+    lastAccessedTime: 1478079042097,
+    location: 'https://brave.com/page2',
+    title: 'sample 2',
+    tags: []
+  })
+  const historyDayThree = Immutable.fromJS([{
+    lastAccessedTime: 1478157051910,
+    location: 'https://brave.com/page3',
+    title: 'sample 3',
+    tags: []
+  }, {
+    lastAccessedTime: 1478157051921,
+    location: 'https://brave.com/page4',
+    title: 'sample 4',
+    tags: []
+  }, {
+    lastAccessedTime: 1478157051932,
+    location: 'https://brave.com/page5',
+    title: 'sample 5',
+    tags: []
+  }])
+  const historyMultipleDays = historyDayThree.push(historyDayTwo, historyDayOne)
+
+  describe('groupEntriesByDay', function () {
+    it('returns the result as an Immutable.List', function () {
+      const result = historyUtil.groupEntriesByDay(historyDayThree)
+      assert.equal(Immutable.List.isList(result), true)
+    })
+    it('has one object for each day', function () {
+      const result = historyUtil.groupEntriesByDay(historyDayThree)
+      assert.equal(result.size, 1)
+    })
+    it('can handle multiple days', function () {
+      const result = historyUtil.groupEntriesByDay(historyMultipleDays)
+      assert.equal(result.size, 3)
+    })
+    describe('with the object representing a day', function () {
+      it('formats a readable `date` field', function () {
+        const result = historyUtil.groupEntriesByDay(historyDayThree, 'en-US')
+        assert.equal(result.getIn([0, 'date']), 'Thursday, November 3, 2016')
+      })
+      it('has an entry for each history item', function () {
+        const result = historyUtil.groupEntriesByDay(historyDayThree, 'en-US')
+        const entries = result.getIn([0, 'entries'])
+        assert.equal(entries && entries.size, historyDayThree.size)
+      })
+    })
+  })
+
+  describe('totalEntries', function () {
+    it('returns the result as an Immutable.List', function () {
+      const result1 = historyUtil.groupEntriesByDay(historyMultipleDays)
+      const result2 = historyUtil.totalEntries(result1)
+      assert.equal(Immutable.List.isList(result2), true)
+    })
+    it('combines entries for multiple days into one response', function () {
+      const result1 = historyUtil.groupEntriesByDay(historyMultipleDays)
+      const result2 = historyUtil.totalEntries(result1)
+      const expectedResult = [
+        historyDayThree.toJS(),
+        [historyDayTwo.toJS()],
+        [historyDayOne.toJS()]
+      ]
+      assert.deepEqual(result2.toJS(), expectedResult)
+    })
+  })
+
+  describe('sortTimeDescending', function () {
+    it('sorts the items by date/time DESC', function () {
+      const result = historyMultipleDays.sort(historyUtil.sortTimeDescending)
+      const expectedResult = historyDayThree.toJS().reverse()
+      expectedResult.push(historyDayTwo.toJS())
+      expectedResult.push(historyDayOne.toJS())
+
+      assert.deepEqual(result.toJS(), expectedResult)
+    })
+  })
+})