Merge pull request #7672 from bsclifton/fix-recently-closed-order

Order the "recently closed" items in History menu in DESC order (top == last closed)
brave · Mar 13, 2017 · 6808b8b · 6808b8b
2 parents 3378e7f + 7789585
commit 6808b8b
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 6 deletions.
diff --git a/app/common/lib/menuUtil.js b/app/common/lib/menuUtil.js
@@ -3,7 +3,7 @@
 
 'use strict'
 
-const Immutable = require('immutable')
+const {makeImmutable} = require('../../common/state/immutableUtil')
 const CommonMenu = require('../../common/commonMenu')
 const messages = require('../../../js/constants/messages')
 const siteTags = require('../../../js/constants/siteTags')
@@ -64,7 +64,7 @@ module.exports.setTemplateItemChecked = (template, label, checked) => {
   const menuItem = getTemplateItem(menu, label)
   if (menuItem.checked !== checked) {
     menuItem.checked = checked
-    return Immutable.fromJS(menu)
+    return makeImmutable(menu)
   }
   return null
 }
@@ -120,6 +120,9 @@ module.exports.createBookmarkTemplateItems = (sites) => {
  */
 module.exports.createRecentlyClosedTemplateItems = (lastClosedFrames) => {
   const payload = []
+
+  lastClosedFrames = makeImmutable(lastClosedFrames)
+
   if (lastClosedFrames && lastClosedFrames.size > 0) {
     payload.push(
       CommonMenu.separatorMenuItem,
@@ -128,7 +131,7 @@ module.exports.createRecentlyClosedTemplateItems = (lastClosedFrames) => {
         enabled: false
       })
 
-    const lastTen = (lastClosedFrames.size < 10) ? lastClosedFrames : lastClosedFrames.slice(-10)
+    const lastTen = ((lastClosedFrames.size < 10) ? lastClosedFrames : lastClosedFrames.slice(-10)).reverse()
     lastTen.forEach((closedFrame) => {
       payload.push({
         label: closedFrame.get('title') || closedFrame.get('location'),

diff --git a/test/unit/app/common/lib/menuUtilTest.js b/test/unit/app/common/lib/menuUtilTest.js
@@ -214,7 +214,8 @@ describe('menuUtil tests', function () {
       assert.equal(menuItems[2].label, windowStateClosedFrames.first().get('title'))
       assert.equal(typeof menuItems[2].click === 'function', true)
     })
-    it('only shows the last 10 items', function () {
+
+    it('shows the last 10 items in reverse order (top == last closed)', function () {
       const windowStateClosedFrames = Immutable.fromJS([
         { title: 'site01', location: 'https://brave01.com' },
         { title: 'site02', location: 'https://brave02.com' },
@@ -231,8 +232,24 @@ describe('menuUtil tests', function () {
       const menuItems = menuUtil.createRecentlyClosedTemplateItems(windowStateClosedFrames)
 
       assert.equal(menuItems.length, 12)
-      assert.equal(menuItems[2].label, windowStateClosedFrames.get(1).get('title'))
-      assert.equal(menuItems[11].label, windowStateClosedFrames.get(10).get('title'))
+      assert.equal(menuItems[11].label, windowStateClosedFrames.get(1).get('title'))
+      assert.equal(menuItems[2].label, windowStateClosedFrames.get(10).get('title'))
+    })
+
+    it('returns an empty array if lastClosedFrames is null, empty, or undefined', function () {
+      assert.deepEqual(menuUtil.createRecentlyClosedTemplateItems(), [])
+      assert.deepEqual(menuUtil.createRecentlyClosedTemplateItems(null), [])
+      assert.deepEqual(menuUtil.createRecentlyClosedTemplateItems(Immutable.fromJS([])), [])
+    })
+
+    it('makes the input immutable if passed as mutable', function () {
+      const windowStateClosedFrames = [{
+        title: 'sample',
+        location: 'https://brave.com'
+      }]
+      const menuItems = menuUtil.createRecentlyClosedTemplateItems(windowStateClosedFrames)
+      assert.equal(Array.isArray(menuItems), true)
+      assert.equal(menuItems.length, 3)
     })
   })