From 784dc16e2d8a7cad396f4f22923c820a9087ad96 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Wed, 17 Aug 2016 17:58:31 -0700 Subject: [PATCH 1/2] History follow up: - about:history now supports context menu (shares code w/ bookmarks) - added isBookmark to siteUtil and updated code to use it - fixed comment on UPDATE_MENU_BOOKMARKED_STATUS - Delete history item now works - Broke out common styles (between history and bookmarks about pages) to siteDetail.less, updated both to use it - Sending menu update (via IPC) only sends what's needed; needs to be refactored to live in appState - Updated directory structure for menu, menuUtil, and commonMenu per https://github.com/brave/browser-laptop/blob/master/docs/directoryStructure.md --- {js => app/browser}/lib/menuUtil.js | 12 +-- app/{ => browser}/menu.js | 20 ++--- {js => app/common}/commonMenu.js | 12 +-- .../brave/locales/en-US/menu.properties | 1 + app/index.js | 21 ++--- app/locale.js | 1 + js/about/bookmarks.js | 9 ++- js/about/history.js | 11 +-- js/actions/bookmarkActions.js | 3 +- js/components/bookmarksToolbar.js | 2 +- js/components/navigationBar.js | 7 +- js/constants/messages.js | 2 +- js/contextMenus.js | 77 ++++++++++++------- js/entry.js | 9 ++- js/state/frameStateUtil.js | 1 - js/state/siteUtil.js | 17 +++- js/stores/windowStore.js | 11 ++- less/about/bookmarks.less | 37 +-------- less/about/history.less | 41 +--------- less/about/siteDetails.less | 38 +++++++++ test/unit/lib/menuUtilTest.js | 2 +- test/unit/state/siteUtilTest.js | 39 +++++++++- 22 files changed, 207 insertions(+), 166 deletions(-) rename {js => app/browser}/lib/menuUtil.js (94%) rename app/{ => browser}/menu.js (97%) rename {js => app/common}/commonMenu.js (96%) create mode 100644 less/about/siteDetails.less diff --git a/js/lib/menuUtil.js b/app/browser/lib/menuUtil.js similarity index 94% rename from js/lib/menuUtil.js rename to app/browser/lib/menuUtil.js index 1cc8f896971..45cc12976b4 100644 --- a/js/lib/menuUtil.js +++ b/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 diff --git a/app/menu.js b/app/browser/menu.js similarity index 97% rename from app/menu.js rename to app/browser/menu.js index 4226ae49520..d4ac6e001b4 100644 --- a/app/menu.js +++ b/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,7 +618,7 @@ 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) { @@ -626,7 +626,7 @@ module.exports.init = (appState, windowData) => { } // 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 { diff --git a/js/commonMenu.js b/app/common/commonMenu.js similarity index 96% rename from js/commonMenu.js rename to app/common/commonMenu.js index 82ea01c4ec0..e5ecc4e5613 100644 --- a/js/commonMenu.js +++ b/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' diff --git a/app/extensions/brave/locales/en-US/menu.properties b/app/extensions/brave/locales/en-US/menu.properties index e3db72f153f..03429dfd934 100644 --- a/app/extensions/brave/locales/en-US/menu.properties +++ b/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 diff --git a/app/index.js b/app/index.js index 873816f53d0..5b3518371ab 100644 --- a/app/index.js +++ b/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) } }) diff --git a/app/locale.js b/app/locale.js index 011a3983240..ac15d1a9d94 100644 --- a/app/locale.js +++ b/app/locale.js @@ -48,6 +48,7 @@ var rendererIdentifiers = function () { 'unpinTab', 'deleteFolder', 'deleteBookmark', + 'deleteHistoryEntry', 'editFolder', 'editBookmark', 'unmuteTabs', diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js index 7476d9f5b18..9043087a1fe 100644 --- a/js/about/bookmarks.js +++ b/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 + return { this.props.bookmarks.map((bookmark) => ) @@ -203,7 +204,7 @@ class SearchResults extends React.Component { }) return ( - + { sortedBookmarks.map((bookmark, idx) => ) } @@ -254,13 +255,13 @@ class AboutBookmarks extends React.Component { }) } render () { - return
+ return

{this.state.search ? : null} -
+
bookmark.get('parentFolderId') === -1)} diff --git a/js/about/history.js b/js/about/history.js index 5523bd2e381..69dea8fc855 100644 --- a/js/about/history.js +++ b/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') @@ -49,7 +50,7 @@ class HistoryItem extends ImmutableComponent { data-context-menu-disable onDoubleClick={this.navigate.bind(this)}> { - this.props.history.get('customTitle') || this.props.history.get('title') + this.props.history.get('customTitle') || this.props.history.get('title') || this.props.history.get('location') ? {new Date(this.props.history.get('lastAccessedTime')).toLocaleDateString()} {this.props.history.get('customTitle') || this.props.history.get('title')} @@ -68,7 +69,7 @@ class HistoryItem extends ImmutableComponent { class HistoryList extends ImmutableComponent { render () { - return + return { this.props.history.map((entry) => ) @@ -111,12 +112,12 @@ class AboutHistory extends React.Component { }) } render () { - return
+ return

-
+
- site.get('tags').isEmpty())} + site.get('tags').isEmpty()).slice(-500)} onChangeSelectedEntry={this.onChangeSelectedEntry} selectedEntry={this.state.selectedEntry} /> diff --git a/js/actions/bookmarkActions.js b/js/actions/bookmarkActions.js index 3f4adc020e7..5c043b693de 100644 --- a/js/actions/bookmarkActions.js +++ b/js/actions/bookmarkActions.js @@ -5,7 +5,6 @@ 'use strict' const siteUtil = require('../state/siteUtil') -const siteTags = require('../constants/siteTags') const windowActions = require('./windowActions') const eventUtil = require('../lib/eventUtil.js') @@ -13,7 +12,7 @@ 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)) }, diff --git a/js/components/bookmarksToolbar.js b/js/components/bookmarksToolbar.js index 9d0ad41fd4c..de304522c27 100644 --- a/js/components/bookmarksToolbar.js +++ b/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) diff --git a/js/components/navigationBar.js b/js/components/navigationBar.js index 630c4595a1d..ba402829e0d 100644 --- a/js/components/navigationBar.js +++ b/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 diff --git a/js/constants/messages.js b/js/constants/messages.js index b67b6724484..b86a7058296 100644 --- a/js/constants/messages.js +++ b/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: _, diff --git a/js/contextMenus.js b/js/contextMenus.js index 5c635ace847..b6071bf9235 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -19,7 +19,7 @@ const siteTags = require('./constants/siteTags') const dragTypes = require('./constants/dragTypes') const siteUtil = require('./state/siteUtil') const downloadUtil = require('./state/downloadUtil') -const CommonMenu = require('./commonMenu') +const CommonMenu = require('../app/common/commonMenu') const dnd = require('./dnd') const dndData = require('./dndData') const appStoreRenderer = require('./stores/appStoreRenderer') @@ -188,7 +188,6 @@ function downloadsToolbarTemplateInit (downloadId, downloadItem) { }) } if (menu.length) { - console.log('----added sep') menu.push(CommonMenu.separatorMenuItem) } } @@ -212,12 +211,31 @@ function downloadsToolbarTemplateInit (downloadId, downloadItem) { return menu } -function bookmarkTemplateInit (siteDetail, activeFrame) { - const location = siteDetail.get('location') - const isFolder = siteUtil.isFolder(siteDetail) +function siteDetailTemplateInit (siteDetail, activeFrame) { + let isHistoryEntry = false + let isFolder = false + let isRootFolder = false + let deleteLabel + let deleteTag + + if (siteUtil.isBookmark(siteDetail)) { + deleteLabel = 'deleteBookmark' + deleteTag = siteTags.BOOKMARK + } else if (siteUtil.isFolder(siteDetail)) { + isFolder = true + isRootFolder = siteDetail.get('folderId') === 0 + deleteLabel = 'deleteFolder' + deleteTag = siteTags.BOOKMARK_FOLDER + } else { + isHistoryEntry = true + deleteLabel = 'deleteHistoryEntry' + } + const template = [] if (!isFolder) { + const location = siteDetail.get('location') + template.push(openInNewTabMenuItem(location, undefined, siteDetail.get('partitionNumber')), openInNewPrivateTabMenuItem(location), openInNewSessionTabMenuItem(location), @@ -228,29 +246,32 @@ function bookmarkTemplateInit (siteDetail, activeFrame) { CommonMenu.separatorMenuItem) } - // We want edit / delete items for everything except for the bookmarks toolbar item - if (!isFolder || siteDetail.get('folderId') !== 0) { + if (!isRootFolder) { + // History can be deleted but not edited + // Picking this menu item pops up the AddEditBookmark modal + if (!isHistoryEntry) { + template.push( + { + label: locale.translation(isFolder ? 'editFolder' : 'editBookmark'), + click: () => windowActions.setBookmarkDetail(siteDetail, siteDetail) + }, + CommonMenu.separatorMenuItem) + } + template.push( { - label: isFolder ? locale.translation('editFolder') : locale.translation('editBookmark'), - click: () => { - // originalLocation is undefined signifies add mode - windowActions.setBookmarkDetail(siteDetail, siteDetail) - } - }) + label: locale.translation(deleteLabel), + click: () => appActions.removeSite(siteDetail, deleteTag) + }, + CommonMenu.separatorMenuItem) + } + if (!isHistoryEntry) { template.push( - CommonMenu.separatorMenuItem, { - label: isFolder ? locale.translation('deleteFolder') : locale.translation('deleteBookmark'), - click: () => { - appActions.removeSite(siteDetail, siteDetail.get('tags').includes(siteTags.BOOKMARK_FOLDER) ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK) - } - }, CommonMenu.separatorMenuItem) + addBookmarkMenuItem('addBookmark', siteUtil.getDetailFromFrame(activeFrame, siteTags.BOOKMARK), siteDetail, true), + addFolderMenuItem(siteDetail, true)) } - template.push(addBookmarkMenuItem('addBookmark', siteUtil.getDetailFromFrame(activeFrame, siteTags.BOOKMARK), siteDetail, true), - addFolderMenuItem(siteDetail, true)) - return template } @@ -291,7 +312,7 @@ function bookmarkItemsInit (allBookmarkItems, items, activeFrame) { icon: showFavicon ? site.get('favicon') : undefined, faIcon, contextMenu: function (e) { - onBookmarkContextMenu(site, activeFrame, e) + onSiteDetailContextMenu(site, activeFrame, e) }, dragEnd: function (e) { dnd.onDragEnd(dragTypes.BOOKMARK, site, e) @@ -948,9 +969,9 @@ function onHamburgerMenu (location, e) { function onMainContextMenu (nodeProps, frame, contextMenuType) { if (contextMenuType === 'bookmark' || contextMenuType === 'bookmark-folder') { - onBookmarkContextMenu(Immutable.fromJS(nodeProps), Immutable.fromJS({ location: '', title: '', partitionNumber: frame.get('partitionNumber') })) + onSiteDetailContextMenu(Immutable.fromJS(nodeProps), Immutable.fromJS({ location: '', title: '', partitionNumber: frame.get('partitionNumber') })) } else if (contextMenuType === 'history') { - // TODO: add new onHistoryContextMenu() and associated methods. + onSiteDetailContextMenu(Immutable.fromJS(nodeProps), Immutable.fromJS({ location: '', title: '', partitionNumber: frame.get('partitionNumber') })) } else if (contextMenuType === 'download') { onDownloadsToolbarContextMenu(nodeProps.downloadId, Immutable.fromJS(nodeProps)) } else { @@ -997,11 +1018,11 @@ function onUrlBarContextMenu (searchDetail, activeFrame, e) { inputMenu.destroy() } -function onBookmarkContextMenu (siteDetail, activeFrame, e) { +function onSiteDetailContextMenu (siteDetail, activeFrame, e) { if (e) { e.stopPropagation() } - const menu = Menu.buildFromTemplate(bookmarkTemplateInit(siteDetail, activeFrame)) + const menu = Menu.buildFromTemplate(siteDetailTemplateInit(siteDetail, activeFrame)) menu.popup(currentWindow) menu.destroy() } @@ -1141,7 +1162,7 @@ module.exports = { onDownloadsToolbarContextMenu, onTabPageContextMenu, onUrlBarContextMenu, - onBookmarkContextMenu, + onSiteDetailContextMenu, onShowBookmarkFolderMenu, onShowUsernameMenu, onMoreBookmarksMenu, diff --git a/js/entry.js b/js/entry.js index 753706086b5..020cc73fcd8 100644 --- a/js/entry.js +++ b/js/entry.js @@ -34,6 +34,7 @@ const messages = require('./constants/messages') const Immutable = require('immutable') const patch = require('immutablepatch') const l10n = require('./l10n') +const FrameStateUtil = require('./state/frameStateUtil') // don't allow scaling or zooming of the ui webFrame.setPageScaleLimits(1, 1) @@ -54,7 +55,13 @@ ipc.on(messages.REQUEST_WINDOW_STATE, () => { }) ipc.on(messages.REQUEST_MENU_DATA_FOR_WINDOW, () => { - ipc.send(messages.RESPONSE_MENU_DATA_FOR_WINDOW, windowStore.getState().toJS()) + const windowState = windowStore.getState() + const activeFrame = FrameStateUtil.getActiveFrame(Immutable.fromJS(windowState)) + const windowData = { + location: activeFrame.get('location'), + closedFrames: windowState.get('closedFrames').toJS() + } + ipc.send(messages.RESPONSE_MENU_DATA_FOR_WINDOW, windowData) }) if (process.env.NODE_ENV === 'test') { diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index 00161dbdb4a..982e64f2561 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -426,7 +426,6 @@ function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey) { // If the frame being removed IS ACTIVE, then try to replace activeFrameKey with parentFrameKey let isActiveFrameBeingRemoved = frameProps.get('key') === activeFrameKey - let parentFrameIndex = findIndexForFrameKey(frames, frameProps.get('parentFrameKey')) let activeFrameIndex diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index 0a05694ff10..81364a8c05b 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -9,7 +9,10 @@ const getSetting = require('../settings').getSetting const urlParse = require('url').parse const isBookmark = (tags) => { - return tags && tags.includes(siteTags.BOOKMARK) + if (!tags) { + return false + } + return tags.includes(siteTags.BOOKMARK) } const isBookmarkFolder = (tags) => { @@ -272,6 +275,18 @@ module.exports.isEquivalent = function (siteDetail1, siteDetail2) { return siteDetail1.get('location') === siteDetail2.get('location') && siteDetail1.get('partitionNumber') === siteDetail2.get('partitionNumber') } +/** + * Determines if the site detail is a bookmark. + * @param siteDetail The site detail to check. + * @return true if the site detail has a bookmark tag. + */ +module.exports.isBookmark = function (siteDetail) { + if (siteDetail) { + return isBookmark(siteDetail.get('tags')) + } + return false +} + /** * Determines if the site detail is a folder. * @param siteDetail The site detail to check. diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index a34c0bbb739..9ed2094ae02 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -440,13 +440,16 @@ const doAction = (action) => { windowState = windowState.merge(FrameStateUtil.removeFrame(windowState.get('frames'), windowState.get('tabs'), windowState.get('closedFrames'), frameProps.set('closedAtIndex', index), activeFrameKey)) - let totalOpenTabs = windowState.get('frames').filter((frame) => !frame.get('pinnedLocation')).size - // History menu needs update (since it shows "Recently Closed" items) - ipc.send(messages.RESPONSE_MENU_DATA_FOR_WINDOW, windowState.toJS()) - + const activeFrameLocation = FrameStateUtil.getActiveFrame(windowState).get('location') + const windowData = { + location: activeFrameLocation, + closedFrames: windowState.get('closedFrames').toJS() + } + ipc.send(messages.RESPONSE_MENU_DATA_FOR_WINDOW, windowData) // If we reach the limit of opened tabs per page while closing tabs, switch to // the active tab's page otherwise the user will hang on empty page + let totalOpenTabs = windowState.get('frames').filter((frame) => !frame.get('pinnedLocation')).size if ((totalOpenTabs % getSetting(settings.TABS_PER_PAGE)) === 0) { updateTabPageIndex(FrameStateUtil.getActiveFrame(windowState)) } diff --git a/less/about/bookmarks.less b/less/about/bookmarks.less index fe1c17ee0ba..f257f04ef69 100644 --- a/less/about/bookmarks.less +++ b/less/about/bookmarks.less @@ -1,27 +1,7 @@ @import "./itemList.less"; -.bookmarksPage { - margin: 20px; - - .bookmarkPageContent { - border-top: 1px solid @chromeBorderColor; - display: flex; - - .bookmarkList { - padding-top: 10px; - overflow: hidden; - - .listItem { - display: flex; - height: 1rem; - - .aboutListItem { - align-items: center; - min-width: 0; - } - } - } - +.siteDetailsPage { + .siteDetailsPageContent { .bookmarkFolderList { min-width: 220px; @@ -45,16 +25,3 @@ .bookmarkFolderList list >* { padding-left: 12px; } - -.searchInput { - float: right; - padding: 5px; - margin-top: -35px; -} - -.searchInputClear { - float: right; - padding: 8px; - margin-top: -39px; - color: #999; -} diff --git a/less/about/history.less b/less/about/history.less index bbe9c24260e..0870d760a35 100644 --- a/less/about/history.less +++ b/less/about/history.less @@ -1,47 +1,10 @@ @import "./itemList.less"; -.historyPage { - margin: 20px; - - .historyPageContent { - border-top: 1px solid @chromeBorderColor; - display: flex; - - .historyList { - padding-top: 10px; - overflow: hidden; - - .listItem { - display: flex; - height: 1rem; - - .aboutListItem { - align-items: center; - } - - .aboutItemDate { - color: #aaa; - margin-right: 10px; - } - } - } - +.siteDetailsPage { + .siteDetailsPageContent { .sticky-outer-wrapper { min-width: 100%; padding-top: 10px; } } } - -.searchInput { - float: right; - padding: 5px; - margin-top: -35px; -} - -.searchInputClear { - float: right; - padding: 8px; - margin-top: -39px; - color: #999; -} diff --git a/less/about/siteDetails.less b/less/about/siteDetails.less new file mode 100644 index 00000000000..0f3a66e8763 --- /dev/null +++ b/less/about/siteDetails.less @@ -0,0 +1,38 @@ +@import "./itemList.less"; + +.siteDetailsPage { + margin: 20px; + + .siteDetailsPageContent { + border-top: 1px solid @chromeBorderColor; + display: flex; + + .siteDetailsList { + padding-top: 10px; + overflow: hidden; + + .listItem { + display: flex; + height: 1rem; + + .aboutListItem { + align-items: center; + min-width: 0; + } + } + } + } +} + +.searchInput { + float: right; + padding: 5px; + margin-top: -35px; +} + +.searchInputClear { + float: right; + padding: 8px; + margin-top: -39px; + color: #999; +} diff --git a/test/unit/lib/menuUtilTest.js b/test/unit/lib/menuUtilTest.js index c956dcdae1b..4552c50e814 100644 --- a/test/unit/lib/menuUtilTest.js +++ b/test/unit/lib/menuUtilTest.js @@ -51,7 +51,7 @@ describe('menuUtil', function () { } mockery.registerMock('electron', fakeElectron) - menuUtil = require('../../../js/lib/menuUtil') + menuUtil = require('../../../app/browser/lib/menuUtil') }) after(function () { diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index f191d332fed..7ead0f19979 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -181,6 +181,15 @@ describe('siteUtil', function () { const expectedSites = sites.setIn([0, 'parentFolderId'], 0).setIn([0, 'tags'], Immutable.List([])) assert.deepEqual(processedSites, expectedSites) }) + it('deletes a history entry (no tag specified)', function () { + const siteDetail = { + tags: [], + location: testUrl1 + } + const sites = Immutable.fromJS([siteDetail]) + const processedSites = siteUtil.removeSite(sites, Immutable.fromJS(siteDetail)) + assert.deepEqual(processedSites, Immutable.fromJS([])) + }) }) describe('moveSite', function () { @@ -285,19 +294,19 @@ describe('siteUtil', function () { }) describe('isFolder', function () { - it('returns true if the input is a siteDetail and has a BOOKMARK_FOLDER tag', function () { + it('returns true if the input is a siteDetail and has a `BOOKMARK_FOLDER` tag', function () { const siteDetail = Immutable.fromJS({ tags: [siteTags.BOOKMARK_FOLDER] }) assert.equal(siteUtil.isFolder(siteDetail), true) }) - it('returns false if the input does not have a BOOKMARK_FOLDER tag', function () { + it('returns false if the input does not have a `BOOKMARK_FOLDER` tag', function () { const siteDetail = Immutable.fromJS({ tags: [siteTags.BOOKMARK] }) assert.equal(siteUtil.isFolder(siteDetail), false) }) - it('returns false if there is no `tags` property', function () { + it('returns false if there is not a `tags` property', function () { const siteDetail = Immutable.fromJS({ notTags: null }) @@ -311,6 +320,30 @@ describe('siteUtil', function () { }) }) + describe('isBookmark', function () { + it('returns true if the input is a siteDetail and has a `BOOKMARK` tag', function () { + const siteDetail = Immutable.fromJS({ + tags: [siteTags.BOOKMARK] + }) + assert.equal(siteUtil.isBookmark(siteDetail), true) + }) + it('returns false if the input does not have a `BOOKMARK` tag', function () { + const siteDetail = Immutable.fromJS({ + tags: [siteTags.BOOKMARK_FOLDER] + }) + assert.equal(siteUtil.isBookmark(siteDetail), false) + }) + it('returns false if there is not a `tags` property', function () { + const siteDetail = Immutable.fromJS({ + notTags: null + }) + assert.equal(siteUtil.isBookmark(siteDetail), false) + }) + it('returns false if the input is falsey', function () { + assert.equal(siteUtil.isBookmark(null), false) + }) + }) + describe('getFolders', function () { }) From bfcb6bc7d783507baf5088fe7f104b1fcae627ec Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Thu, 18 Aug 2016 22:57:22 -0700 Subject: [PATCH 2/2] Removed date from history screen. Fixes https://github.com/brave/browser-laptop/issues/3259 --- js/about/history.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/js/about/history.js b/js/about/history.js index 69dea8fc855..3d7b4016a1a 100644 --- a/js/about/history.js +++ b/js/about/history.js @@ -50,15 +50,13 @@ class HistoryItem extends ImmutableComponent { data-context-menu-disable onDoubleClick={this.navigate.bind(this)}> { - this.props.history.get('customTitle') || this.props.history.get('title') || this.props.history.get('location') + this.props.history.get('customTitle') || this.props.history.get('title') ? - {new Date(this.props.history.get('lastAccessedTime')).toLocaleDateString()} {this.props.history.get('customTitle') || this.props.history.get('title')} {partitionNumberInfo} -{this.props.history.get('location')} : - {new Date(this.props.history.get('lastAccessedTime')).toLocaleDateString()} {this.props.history.get('location')} {partitionNumberInfo}