Merge pull request #3253 from bsclifton/history-follow-up

History follow up

js/lib/menuUtil.js → app/browser/lib/menuUtil.js

@@ -3,12 +3,12 @@
 
 'use strict'
 
-const CommonMenu = require('../commonMenu')
-const messages = require('../constants/messages')
-const siteTags = require('../constants/siteTags')
-const eventUtil = require('./eventUtil')
-const siteUtil = require('../state/siteUtil')
-const locale = require('../../app/locale')
+const CommonMenu = require('../../common/commonMenu')
+const messages = require('../../../js/constants/messages')
+const siteTags = require('../../../js/constants/siteTags')
+const eventUtil = require('../../../js/lib/eventUtil')
+const siteUtil = require('../../../js/state/siteUtil')
+const locale = require('../../locale')
 
 // States which can trigger dynamic menus to change
 let lastSettingsState, lastSites, lastClosedFrames

app/menu.js → app/browser/menu.js

@@ -6,17 +6,17 @@
 
 const Immutable = require('immutable')
 const electron = require('electron')
-const appConfig = require('../js/constants/appConfig')
+const appConfig = require('../../js/constants/appConfig')
 const Menu = electron.Menu
 const MenuItem = electron.MenuItem
-const messages = require('../js/constants/messages')
-const settings = require('../js/constants/settings')
+const messages = require('../../js/constants/messages')
+const settings = require('../../js/constants/settings')
 const dialog = electron.dialog
-const appActions = require('../js/actions/appActions')
-const menuUtil = require('../js/lib/menuUtil')
-const getSetting = require('../js/settings').getSetting
-const locale = require('./locale')
-const {isSiteBookmarked} = require('../js/state/siteUtil')
+const appActions = require('../../js/actions/appActions')
+const menuUtil = require('./lib/menuUtil')
+const getSetting = require('../../js/settings').getSetting
+const locale = require('../locale')
+const {isSiteBookmarked} = require('../../js/state/siteUtil')
 const isDarwin = process.platform === 'darwin'
 const aboutUrl = 'https://brave.com/'
 
@@ -618,15 +618,15 @@ const updateMenu = (CommonMenu, appState, windowData) => {
  * @param {Object} appState - Application state. Used to fetch bookmarks and settings (like homepage)
  * @param {Object} windowData - Information specific to the current window (recently closed tabs, etc)
  */
-module.exports.init = (appState, windowData) => {
+module.exports.rebuild = (appState, windowData) => {
   // The menu will always be called once localization is done
   // so don't bother loading anything until it is done.
   if (!locale.initialized) {
     return
   }
 
   // This needs to be within the init method to handle translations
-  const CommonMenu = require('../js/commonMenu')
+  const CommonMenu = require('../common/commonMenu')
   if (appMenu.items.length === 0) {
     createMenu(CommonMenu)
   } else {

js/commonMenu.js → app/common/commonMenu.js

@@ -4,13 +4,13 @@
 
 'use strict'
 
-const appConfig = require('./constants/appConfig')
-const appActions = require('../js/actions/appActions')
-const messages = require('../js/constants/messages')
+const appConfig = require('../../js/constants/appConfig')
+const appActions = require('../../js/actions/appActions')
+const messages = require('../../js/constants/messages')
 const Immutable = require('immutable')
-const locale = require('../js/l10n')
-const settings = require('./constants/settings')
-const getSetting = require('./settings').getSetting
+const locale = require('../../js/l10n')
+const settings = require('../../js/constants/settings')
+const getSetting = require('../../js/settings').getSetting
 const issuesUrl = 'https://github.com/brave/browser-laptop/issues'
 const isDarwin = process.platform === 'darwin'
 

app/extensions/brave/locales/en-US/menu.properties

@@ -93,6 +93,7 @@ editFolder=Edit Folder...
 editBookmark=Edit Bookmark...
 deleteFolder=Delete Folder
 deleteBookmark=Delete Bookmark
+deleteHistoryEntry=Delete History Entry
 stop=Stop
 clone=Clone
 reloadTab=Reload

app/index.js

@@ -32,7 +32,7 @@ const BrowserWindow = electron.BrowserWindow
 const dialog = electron.dialog
 const ipcMain = electron.ipcMain
 const app = electron.app
-const Menu = require('./menu')
+const Menu = require('./browser/menu')
 const Updater = require('./updater')
 const messages = require('../js/constants/messages')
 const appConfig = require('../js/constants/appConfig')
@@ -60,7 +60,6 @@ const spellCheck = require('./spellCheck')
 const ledger = require('./ledger')
 const flash = require('../js/flash')
 const contentSettings = require('../js/state/contentSettings')
-const FrameStateUtil = require('../js/state/frameStateUtil')
 
 // Used to collect the per window state when shutting down the application
 let perWindowState = []
@@ -300,16 +299,10 @@ app.on('ready', () => {
     saveIfAllCollected()
   })
 
-  // Window state must be fetched from main process; this is fired once it's retrieved
-  ipcMain.on(messages.RESPONSE_MENU_DATA_FOR_WINDOW, (wnd, windowState) => {
-    if (windowState) {
-      const activeFrame = FrameStateUtil.getActiveFrame(Immutable.fromJS(windowState))
-      const windowData = Immutable.fromJS({
-        location: activeFrame ? activeFrame.get('location') : 'about:blank',
-        closedFrames: windowState.closedFrames
-      })
-
-      Menu.init(AppStore.getState(), windowData)
+  // Window state is fetched via the renderer process; this is fired once it's retrieved
+  ipcMain.on(messages.RESPONSE_MENU_DATA_FOR_WINDOW, (wnd, windowData) => {
+    if (windowData) {
+      Menu.rebuild(AppStore.getState(), Immutable.fromJS(windowData))
     }
   })
 
@@ -386,7 +379,7 @@ app.on('ready', () => {
     // reset the browser window. This will default to en-US if
     // not yet configured.
     locale.init(initialState.settings[settings.LANGUAGE], (strings) => {
-      Menu.init(AppStore.getState(), null)
+      Menu.rebuild(AppStore.getState(), null)
     })
 
     // Do this after loading the state
@@ -545,7 +538,7 @@ app.on('ready', () => {
       if (BrowserWindow.getFocusedWindow()) {
         BrowserWindow.getFocusedWindow().webContents.send(messages.REQUEST_MENU_DATA_FOR_WINDOW)
       } else {
-        Menu.init(AppStore.getState(), null)
+        Menu.rebuild(AppStore.getState(), null)
       }
     })
 

app/locale.js

@@ -48,6 +48,7 @@ var rendererIdentifiers = function () {
     'unpinTab',
     'deleteFolder',
     'deleteBookmark',
+    'deleteHistoryEntry',
     'editFolder',
     'editBookmark',
     'unmuteTabs',

js/about/bookmarks.js

@@ -18,6 +18,7 @@ const ipc = window.chrome.ipc
 
 // Stylesheets
 require('../../less/about/itemList.less')
+require('../../less/about/siteDetails.less')
 require('../../less/about/bookmarks.less')
 require('../../node_modules/font-awesome/css/font-awesome.css')
 
@@ -179,7 +180,7 @@ class BookmarkFolderList extends ImmutableComponent {
 
 class BookmarksList extends ImmutableComponent {
   render () {
-    return <list className='bookmarkList'>
+    return <list className='siteDetailsList'>
     {
       this.props.bookmarks.map((bookmark) =>
         <BookmarkItem bookmark={bookmark} />)
@@ -203,7 +204,7 @@ class SearchResults extends React.Component {
     })
 
     return (
-      <list className='bookmarkList'>
+      <list className='siteDetailsList'>
       {
         sortedBookmarks.map((bookmark, idx) => <BookmarkItem bookmark={bookmark} inSelectedFolder={selectedFolderIndex.get(idx)} />)
       }
@@ -254,13 +255,13 @@ class AboutBookmarks extends React.Component {
     })
   }
   render () {
-    return <div className='bookmarksPage'>
+    return <div className='siteDetailsPage'>
       <h2 data-l10n-id='folders' />
       <input type='text' className='searchInput' id='bookmarkSearch' value={this.state.search} onChange={this.onChangeSearch} data-l10n-id='bookmarkSearch' />
       {this.state.search
         ? <span onClick={this.onClearSearchText} className='fa fa-close searchInputClear'></span>
         : null}
-      <div className='bookmarkPageContent'>
+      <div className='siteDetailsPageContent'>
         <Sticky enabled top={10}>
           <BookmarkFolderList onChangeSelectedFolder={this.onChangeSelectedFolder}
             bookmarkFolders={this.state.bookmarkFolders.filter((bookmark) => bookmark.get('parentFolderId') === -1)}

js/about/history.js

@@ -14,6 +14,7 @@ const ipc = window.chrome.ipc
 
 // Stylesheets
 require('../../less/about/itemList.less')
+require('../../less/about/siteDetails.less')
 require('../../less/about/history.less')
 require('../../node_modules/font-awesome/css/font-awesome.css')
 
@@ -51,13 +52,11 @@ class HistoryItem extends ImmutableComponent {
     {
       this.props.history.get('customTitle') || this.props.history.get('title')
       ? <span className='aboutListItem' title={this.props.history.get('location')}>
-        <span className='aboutItemDate'>{new Date(this.props.history.get('lastAccessedTime')).toLocaleDateString()}</span>
         <span className='aboutItemTitle'>{this.props.history.get('customTitle') || this.props.history.get('title')}</span>
         {partitionNumberInfo}
         <span className='aboutItemSeparator'>-</span><span className='aboutItemLocation'>{this.props.history.get('location')}</span>
       </span>
       : <span className='aboutListItem' title={this.props.history.get('location')}>
-        <span className='aboutItemDate'>{new Date(this.props.history.get('lastAccessedTime')).toLocaleDateString()}</span>
         <span>{this.props.history.get('location')}</span>
         {partitionNumberInfo}
       </span>
@@ -68,7 +67,7 @@ class HistoryItem extends ImmutableComponent {
 
 class HistoryList extends ImmutableComponent {
   render () {
-    return <list className='historyList'>
+    return <list className='siteDetailsList'>
     {
       this.props.history.map((entry) =>
         <HistoryItem history={entry} />)
@@ -111,12 +110,12 @@ class AboutHistory extends React.Component {
     })
   }
   render () {
-    return <div className='historyPage'>
+    return <div className='siteDetailsPage'>
       <h2 data-l10n-id='history' />
 
-      <div className='historyPageContent'>
+      <div className='siteDetailsPageContent'>
         <Sticky enabled top={10}>
-          <HistoryList history={this.state.history.filter((site) => site.get('tags').isEmpty())}
+          <HistoryList history={this.state.history.filter((site) => site.get('tags').isEmpty()).slice(-500)}
             onChangeSelectedEntry={this.onChangeSelectedEntry}
             selectedEntry={this.state.selectedEntry} />
         </Sticky>

js/actions/bookmarkActions.js

@@ -5,15 +5,14 @@
 'use strict'
 
 const siteUtil = require('../state/siteUtil')
-const siteTags = require('../constants/siteTags')
 const windowActions = require('./windowActions')
 const eventUtil = require('../lib/eventUtil.js')
 
 const bookmarkActions = {
   openBookmarksInFolder: function (allBookmarkItems, folderDetail) {
     // We have a middle clicked folder
     allBookmarkItems
-      .filter((bookmark) => bookmark.get('parentFolderId') === folderDetail.get('folderId') && bookmark.get('tags').includes(siteTags.BOOKMARK))
+      .filter((bookmark) => bookmark.get('parentFolderId') === folderDetail.get('folderId') && siteUtil.isBookmark(bookmark))
       .forEach((bookmark) =>
         windowActions.newFrame(siteUtil.toFrameOpts(bookmark), false))
   },

js/components/bookmarksToolbar.js

@@ -250,7 +250,7 @@ class BookmarksToolbar extends ImmutableComponent {
         appActions.addSite({ location: url }, siteTags.BOOKMARK))
   }
   openContextMenu (bookmark, e) {
-    contextMenus.onBookmarkContextMenu(bookmark, this.activeFrame, e)
+    contextMenus.onSiteDetailContextMenu(bookmark, this.activeFrame, e)
   }
   clickBookmarkItem (bookmark, e) {
     return bookmarkActions.clickBookmarkItem(this.bookmarks, bookmark, this.activeFrame, e)

js/components/navigationBar.js

@@ -11,7 +11,6 @@ const Button = require('./button')
 const UrlBar = require('./urlBar')
 const appActions = require('../actions/appActions')
 const windowActions = require('../actions/windowActions')
-const {isSiteBookmarked} = require('../state/siteUtil')
 const siteTags = require('../constants/siteTags')
 const messages = require('../constants/messages')
 const settings = require('../constants/settings')
@@ -46,7 +45,7 @@ class NavigationBar extends ImmutableComponent {
     const siteDetail = siteUtil.getDetailFromFrame(this.activeFrame, siteTags.BOOKMARK)
     const showBookmarksToolbar = getSetting(settings.SHOW_BOOKMARKS_TOOLBAR)
     const hasBookmark = this.props.sites.find(
-      (site) => site.get('tags').includes(siteTags.BOOKMARK) || site.get('tags').includes(siteTags.BOOKMARK_FOLDER)
+      (site) => siteUtil.isBookmark(site) || siteUtil.isFolder(site)
     )
     if (!isBookmarked) {
       appActions.addSite(siteDetail, siteTags.BOOKMARK)
@@ -80,7 +79,7 @@ class NavigationBar extends ImmutableComponent {
 
   get bookmarked () {
     return this.props.activeFrameKey !== undefined &&
-      isSiteBookmarked(this.props.sites, Immutable.fromJS({
+      siteUtil.isSiteBookmarked(this.props.sites, Immutable.fromJS({
         location: this.props.location,
         partitionNumber: this.props.partitionNumber,
         title: this.props.title
@@ -115,7 +114,7 @@ class NavigationBar extends ImmutableComponent {
   componentDidUpdate (prevProps) {
     // Update the app menu to reflect whether the current page is bookmarked
     const prevBookmarked = this.props.activeFrameKey !== undefined &&
-      isSiteBookmarked(prevProps.sites, Immutable.fromJS({
+      siteUtil.isSiteBookmarked(prevProps.sites, Immutable.fromJS({
         location: prevProps.location,
         partitionNumber: prevProps.partitionNumber,
         title: prevProps.title

js/constants/messages.js

@@ -112,7 +112,7 @@ const messages = {
   // Menu rebuilding
   REQUEST_MENU_DATA_FOR_WINDOW: _,
   RESPONSE_MENU_DATA_FOR_WINDOW: _,
-  UPDATE_MENU_BOOKMARKED_STATUS: _, /** @arg {Object} currently only has a boolean "bookmarked" */
+  UPDATE_MENU_BOOKMARKED_STATUS: _, /** @isBookmarked {boolean} should menu show "Bookmark Page" as checked */
   // Ad block, safebrowsing, and tracking protection
   BLOCKED_RESOURCE: _,
   BLOCKED_PAGE: _,