From eea84e53ac1bd57a9586f99373e2eb1f6061ca57 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Thu, 6 Oct 2016 15:14:56 -0700 Subject: [PATCH 1/6] Updates to about:bookmarks - Page is now styled! It looks similar to about:history - Bookmark entries now shown in sortableTable (can't be re-ordered, but can be sorted) - favicons are now shown - URL is shown to right of bookmark on mouse over Fixes https://github.com/brave/browser-laptop/issues/4562 Fixes https://github.com/brave/browser-laptop/issues/1994 --- app/extensions/brave/about-bookmarks.html | 2 +- .../brave/locales/en-US/bookmarks.properties | 3 + .../brave/locales/en-US/history.properties | 2 +- js/about/bookmarks.js | 229 +++++++++--------- js/about/history.js | 29 ++- js/components/sortableTable.js | 59 +++-- less/about/bookmarks.less | 74 +++++- less/about/history.less | 89 +------ less/about/siteDetails.less | 88 ++++++- less/sortableTable.less | 10 + 10 files changed, 334 insertions(+), 251 deletions(-) diff --git a/app/extensions/brave/about-bookmarks.html b/app/extensions/brave/about-bookmarks.html index d9a4315a07b..089feb373db 100644 --- a/app/extensions/brave/about-bookmarks.html +++ b/app/extensions/brave/about-bookmarks.html @@ -9,7 +9,7 @@ - + diff --git a/app/extensions/brave/locales/en-US/bookmarks.properties b/app/extensions/brave/locales/en-US/bookmarks.properties index c79d7952455..4c775e94277 100644 --- a/app/extensions/brave/locales/en-US/bookmarks.properties +++ b/app/extensions/brave/locales/en-US/bookmarks.properties @@ -1,6 +1,9 @@ bookmarks=Bookmarks +bookmarkManager=Bookmark Manager folders=Folders +organize=Organize partitionNumber=Session {{partitionNumber}} bookmarksToolbar=Bookmarks Toolbar otherBookmarks=Other Bookmarks bookmarkSearch.placeholder=Search bookmarks +importBrowserData=Import browser data diff --git a/app/extensions/brave/locales/en-US/history.properties b/app/extensions/brave/locales/en-US/history.properties index 0bab31867c0..8e7e0dfab14 100644 --- a/app/extensions/brave/locales/en-US/history.properties +++ b/app/extensions/brave/locales/en-US/history.properties @@ -2,7 +2,7 @@ historyTitle=History history=History clearBrowsingDataNow=Clear browsing data removeSelectedItems=Remove selected items -historySearch=Search history time=Time title=Title domain=Domain +historySearch.placeholder=Search history diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js index c23ad3e4982..add946927de 100644 --- a/js/about/bookmarks.js +++ b/js/about/bookmarks.js @@ -5,7 +5,6 @@ // Note that these are webpack requires, not CommonJS node requiring requires const React = require('react') const Immutable = require('immutable') -const Sticky = require('react-stickynode') const ImmutableComponent = require('../components/immutableComponent') const messages = require('../constants/messages') const siteTags = require('../constants/siteTags') @@ -13,82 +12,17 @@ const dragTypes = require('../constants/dragTypes') const aboutActions = require('./aboutActions') const dndData = require('../dndData') const cx = require('../lib/classSet') +const SortableTable = require('../components/sortableTable') +const Button = require('../components/button') +const siteUtil = require('../state/siteUtil') +const iconSize = 16 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') -class BookmarkItem extends ImmutableComponent { - onDragStart (e) { - e.dataTransfer.effectAllowed = 'all' - dndData.setupDataTransferBraveData(e.dataTransfer, dragTypes.BOOKMARK, this.props.bookmark) - // TODO: Pass the location here when content scripts are fixed - dndData.setupDataTransferURL(e.dataTransfer, '', this.props.bookmark.get('customTitle') || this.props.bookmark.get('title')) - } - onDragOver (e) { - e.preventDefault() - e.dataTransfer.dropEffect = 'move' - } - onDrop (e) { - const bookmark = dndData.getDragData(e.dataTransfer, dragTypes.BOOKMARK) - if (bookmark) { - aboutActions.moveSite(bookmark.toJS(), this.props.bookmark.toJS(), dndData.shouldPrependVerticalItem(e.target, e.clientY), false) - } - } - navigate () { - aboutActions.newFrame({ - location: this.props.bookmark.get('location'), - partitionNumber: this.props.bookmark.get('partitionNumber') - }) - } - render () { - // Figure out the partition info display - let partitionNumberInfo - if (this.props.bookmark.get('partitionNumber')) { - let l10nArgs = { - partitionNumber: this.props.bookmark.get('partitionNumber') - } - partitionNumberInfo = -  () - } - - var className = 'listItem' - - // If the bookmark item is in the selected folder, show - // it as selected - if (this.props.inSelectedFolder) { - className += ' selected' - } - - return
- { - this.props.bookmark.get('customTitle') || this.props.bookmark.get('title') - ? - {this.props.bookmark.get('customTitle') || this.props.bookmark.get('title')} - {partitionNumberInfo} - -{this.props.bookmark.get('location')} - - : - {this.props.bookmark.get('location')} - {partitionNumberInfo} - - } -
- } -} - class BookmarkFolderItem extends ImmutableComponent { onDragStart (e) { if (this.props.draggable !== false) { @@ -178,38 +112,94 @@ class BookmarkFolderList extends ImmutableComponent { } } -class BookmarksList extends ImmutableComponent { +class BookmarkTitleCell extends ImmutableComponent { render () { - return + let iconStyle + let showingFavicon = false + if (!siteUtil.isFolder(this.props.siteDetail)) { + const icon = this.props.siteDetail.get('favicon') + if (icon) { + iconStyle = { + minWidth: iconSize, + width: iconSize, + backgroundImage: `url(${icon})`, + backgroundSize: iconSize, + height: iconSize + } + showingFavicon = true + } + } + + const bookmarkTitle = this.props.siteDetail.get('customTitle') || this.props.siteDetail.get('title') + const bookmarkLocation = this.props.siteDetail.get('location') + + return
{ - this.props.bookmarks.map((bookmark) => - ) + showingFavicon ? : null } - + {bookmarkTitle || bookmarkLocation} + { + bookmarkTitle ? {bookmarkLocation} : null + } +
} } -class SearchResults extends React.Component { - render () { - // Sort bookmarks by title - var sortedBookmarks = this.props.bookmarks.sort((a, b) => { - if (a.get('title').toUpperCase() < b.get('title').toUpperCase()) return -1 - if (a.get('title').toUpperCase() > b.get('title').toUpperCase()) return 1 - return 0 - }) - - // Flag each bookmark if it is in the selected folder - var selectedFolderIndex = sortedBookmarks.map((bookmark, idx) => { - return sortedBookmarks.get(idx).get('parentFolderId') === this.props.selectedFolderId +class BookmarksList extends ImmutableComponent { + onDoubleClick (entry) { + aboutActions.newFrame({ + location: entry.location, + partitionNumber: entry.partitionNumber }) + } + onDragStart (siteDetail, e) { + e.dataTransfer.effectAllowed = 'all' + dndData.setupDataTransferBraveData(e.dataTransfer, dragTypes.BOOKMARK, siteDetail) + // TODO: Pass the location here when content scripts are fixed + dndData.setupDataTransferURL(e.dataTransfer, '', siteDetail.get('customTitle') || siteDetail.get('title')) + } + onDragOver (siteDetail, e) { + e.preventDefault() + e.dataTransfer.dropEffect = 'move' + } + onDrop (siteDetail, e) { + const bookmark = dndData.getDragData(e.dataTransfer, dragTypes.BOOKMARK) + if (bookmark) { + aboutActions.moveSite(bookmark.toJS(), siteDetail.toJS(), dndData.shouldPrependVerticalItem(e.target, e.clientY), false) + } + } + render () { + const props = !this.props.sortable ? { + sortingDisabled: true + } : { + onDoubleClick: this.onDoubleClick, + onDragStart: this.onDragStart, + onDragOver: this.onDragOver, + onDrop: this.onDrop, + sortingDisabled: false + } - return ( - - { - sortedBookmarks.map((bookmark, idx) => ) - } - - ) + return
+ [ + { + cell: , + value: entry.get('customTitle') || entry.get('title') + }, + { + html: new Date(entry.get('lastAccessedTime')).toLocaleString(), + value: entry.get('lastAccessedTime') + } + ])} + rowObjects={this.props.bookmarks} + defaultHeading='Title' + addHoverClass + onDoubleClick={this.onDoubleClick} + {...props} + columnClassNames={['title', 'date']} + contextMenuName='bookmark' + onContextMenu={aboutActions.contextMenu} /> +
} } @@ -219,8 +209,9 @@ class AboutBookmarks extends React.Component { this.onChangeSelectedFolder = this.onChangeSelectedFolder.bind(this) this.onChangeSearch = this.onChangeSearch.bind(this) this.onClearSearchText = this.onClearSearchText.bind(this) + this.importBrowserData = this.importBrowserData.bind(this) this.state = { - bookmarks: Immutable.Map(), + bookmarks: Immutable.List(), bookmarkFolders: Immutable.Map(), selectedFolderId: 0, search: '' @@ -254,31 +245,51 @@ class AboutBookmarks extends React.Component { return title.match(new RegExp(searchTerm, 'gi')) }) } + get bookmarksInFolder () { + return this.state.bookmarks.filter((bookmark) => (bookmark.get('parentFolderId') || 0) === this.state.selectedFolderId) + } + importBrowserData () { + aboutActions.importBrowerDataNow() + } componentDidMount () { this.refs.bookmarkSearch.focus() } render () { return
-

- - { - this.state.search - ? - : null - } +
+
+
+
+ + { + this.state.search + ? + : + } +
+
+
+
- +
+
bookmark.get('parentFolderId') === -1)} allBookmarkFolders={this.state.bookmarkFolders} isRoot selectedFolderId={this.state.selectedFolderId} /> - - { - this.state.search - ? - : (bookmark.get('parentFolderId') || 0) === this.state.selectedFolderId)} /> - } +
+
+
+ +
} diff --git a/js/about/history.js b/js/about/history.js index 821a481ae44..646b924943a 100644 --- a/js/about/history.js +++ b/js/about/history.js @@ -18,8 +18,6 @@ const siteUtil = require('../state/siteUtil') 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') @@ -149,7 +147,7 @@ class AboutHistory extends React.Component { this.onClearSearchText = this.onClearSearchText.bind(this) this.clearBrowsingDataNow = this.clearBrowsingDataNow.bind(this) this.state = { - history: Immutable.Map(), + history: Immutable.List(), search: '', settings: Immutable.Map(), languageCodes: Immutable.Map() @@ -180,7 +178,7 @@ class AboutHistory extends React.Component { return title.match(new RegExp(searchTerm, 'gi')) }) } - historyDescendingOrder () { + get historyDescendingOrder () { return this.state.history.filter((site) => siteUtil.isHistoryEntry(site)) .sort((left, right) => { if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1 @@ -191,13 +189,16 @@ class AboutHistory extends React.Component { clearBrowsingDataNow () { aboutActions.clearBrowsingDataNow({browserHistory: true}) } + componentDidMount () { + this.refs.historySearch.focus() + } render () { return
- + { this.state.search ? @@ -209,16 +210,14 @@ class AboutHistory extends React.Component {
- { - - } +
} diff --git a/js/components/sortableTable.js b/js/components/sortableTable.js index 8fb7b55a138..807686e37ae 100644 --- a/js/components/sortableTable.js +++ b/js/components/sortableTable.js @@ -3,6 +3,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const React = require('react') +const Immutable = require('immutable') const ImmutableComponent = require('./immutableComponent') const tableSort = require('tablesort') const cx = require('../lib/classSet') @@ -104,32 +105,49 @@ class SortableTable extends ImmutableComponent { return this.props.rowClassNames && this.props.rowClassNames.length === this.props.rows.length } - get hasDoubleClickHandler () { - return typeof this.props.onDoubleClick === 'function' - } get hasContextMenu () { return typeof this.props.onContextMenu === 'function' && typeof this.props.contextMenuName === 'string' } + get sortingDisabled () { + if (typeof this.props.sortingDisabled === 'boolean') { + return this.props.sortingDisabled + } + return false + } getRowAttributes (row, index) { const rowAttributes = {} - const handlerInput = this.props.rowObjects - ? (typeof this.props.rowObjects[index].toJS === 'function' - ? this.props.rowObjects[index].toJS() + const handlerInput = this.props.rowObjects && + (this.props.rowObjects.size > 0 || this.props.rowObjects.length > 0) + ? (typeof this.props.rowObjects.toJS === 'function' + ? this.props.rowObjects.get(index).toJS() : this.props.rowObjects[index]) : row if (this.props.addHoverClass) { rowAttributes.className = 'rowHover' } - if (this.hasClickHandler) { + if (this.hasContextMenu) { + rowAttributes.onContextMenu = this.props.onContextMenu.bind(this, handlerInput, this.props.contextMenuName) + } + // Bindings for row-specific event handlers + if (typeof this.props.onClick === 'function') { rowAttributes.onClick = this.props.onClick.bind(this, handlerInput) } - if (this.hasDoubleClickHandler) { + if (typeof this.props.onDoubleClick === 'function') { rowAttributes.onDoubleClick = this.props.onDoubleClick.bind(this, handlerInput) } - if (this.hasContextMenu) { - rowAttributes.onContextMenu = this.props.onContextMenu.bind(this, handlerInput, this.props.contextMenuName) + if (typeof this.props.onDragStart === 'function') { + rowAttributes.onDragStart = this.props.onDragStart.bind(this, Immutable.fromJS(handlerInput)) + rowAttributes.draggable = true + } + if (typeof this.props.onDragOver === 'function') { + rowAttributes.onDragOver = this.props.onDragOver.bind(this, Immutable.fromJS(handlerInput)) + rowAttributes.draggable = true + } + if (typeof this.props.onDrop === 'function') { + rowAttributes.onDrop = this.props.onDrop.bind(this, Immutable.fromJS(handlerInput)) + rowAttributes.draggable = true } return rowAttributes } @@ -137,23 +155,24 @@ class SortableTable extends ImmutableComponent { if (!this.props.headings || !this.props.rows) { return false } - return { this.table = node }}> {this.props.headings.map((heading, j) => { - const firstEntry = this.props.rows[0][j] + const firstEntry = this.props.rows.length > 0 + ? this.props.rows[0][j] + : undefined let dataType = typeof firstEntry if (dataType === 'object' && firstEntry.value) { dataType = typeof firstEntry.value } return {entry} : null }) diff --git a/less/about/bookmarks.less b/less/about/bookmarks.less index f257f04ef69..278823ad6e6 100644 --- a/less/about/bookmarks.less +++ b/less/about/bookmarks.less @@ -1,23 +1,77 @@ -@import "./itemList.less"; +@import "./siteDetails.less"; .siteDetailsPage { .siteDetailsPageContent { - .bookmarkFolderList { + display: flex; + height: 100%; + + .folderView { min-width: 220px; + flex-grow: 0; - .listItem { - border-right: 1px solid @chromeBorderColor; - height: 1rem; + > .bookmarkFolderList { + display: block; + border-top: solid 1px @gray; + border-right: 1px solid @braveOrange; + height: 100%; } - .bookmarkFolderIcon { - margin-right: 5px; + .bookmarkFolderList { + .listItem { + height: 1rem; + &:hover { + background-color: #ffcc99 !important; + } + &.selected { + background-color: lighten(@braveOrange, 20%) !important; + } + } + .bookmarkFolderIcon { + margin-right: 5px; + } } } + .organizeView { + flex-grow: 1; - .sticky-outer-wrapper { - min-width: 220px; - padding-top: 10px; + .sortableTable { + border: 0; + box-shadow: none; + table-layout: fixed; + -webkit-user-select: none; + th:nth-of-type(2), td.date { + width: 250px; + } + td { + font-size: 11pt; + padding-left: 10px; + white-space: nowrap; + max-width: 0; + .bookmarkFavicon { + display: inline-block; + margin-right: 5px; + } + .bookmarkLocation { + color: @gray; + margin-left: 12px; + overflow: hidden; + text-overflow: ellipsis; + display: none; + } + } + .title { + overflow: hidden; + text-overflow: ellipsis; + span { + vertical-align: middle; + } + } + tr:hover { + .bookmarkLocation { + display: inline-block; + } + } + } } } } diff --git a/less/about/history.less b/less/about/history.less index a6cd73ae185..7ae2699d876 100644 --- a/less/about/history.less +++ b/less/about/history.less @@ -1,91 +1,9 @@ -@import "./itemList.less"; -/*@import "./preferences.less";*/ -@import "./common.less"; - -strong, div, span, li, em, p, a { - font-weight: 200; - -webkit-font-smoothing: antialiased; -} - -body { - background: @veryLightGray; -} +@import "./siteDetails.less"; .siteDetailsPage { - margin: 0; - padding-top: 40px; min-width: 704px; - /*name/color taken from preferences.less*/ - .sectionTitle { - color: @braveOrange; - } - - .siteDetailsPageHeader { - padding: 0 40px; - - .sectionTitle { - font-size: 28px; - display: inline-block; - -webkit-user-select: none; - } - .headerActions { - float: right; - - .searchWrapper { - display: inline-block; - - /*overridden from siteDetails.less*/ - input.searchInput { - float: none; - padding: 5px; - margin-top: 0; - margin-right: 12px; - font-size: 16px; - min-width: 280px; - height: 22px; - } - .searchInputPlaceholder { - color: @gray; - float: none; - font-family: FontAwesome; - left: -35px; - margin: 0; - padding: 0; - position: relative; - width: 0; - } - .searchInputClear { - float: none; - margin: 0; - padding: 0; - width: 0; - position: relative; - left: -35px; - } - } - /*copied from preferences.less*/ - span.browserButton.primaryButton.clearBrowsingDataButton { - font-size: 0.9em; - padding: 5px 20px; - margin: 0; - } - } - } - .siteDetailsPageContent { - border-top: 0px; - margin-top: 50px; - display: block; - clear: both; - - .sectionTitle { - font-size: 16px; - margin-bottom: 12px; - padding-left: 40px; - -webkit-user-select: none; - } - .sortableTable { // Time visited .time { @@ -93,6 +11,7 @@ body { font-weight: 800; text-align: center; width: 154px; + white-space: nowrap; // Delete button .deleteEntry { @@ -105,10 +24,10 @@ body { // Entry title .title { + font-size: 11pt; max-width: 415px; - text-overflow: ellipsis; overflow: hidden; - font-size: 11pt; + text-overflow: ellipsis; white-space: nowrap; } diff --git a/less/about/siteDetails.less b/less/about/siteDetails.less index 4f6c7fd6944..9e15e9d88fd 100644 --- a/less/about/siteDetails.less +++ b/less/about/siteDetails.less @@ -1,25 +1,87 @@ @import "./itemList.less"; +@import "./common.less"; + +strong, div, span, li, em, p, a { + font-weight: 200; + -webkit-font-smoothing: antialiased; +} + +body { + background: @veryLightGray; +} + +/*name/color taken from preferences.less*/ +.sectionTitle { + color: @braveOrange; + cursor: default; + -webkit-user-select: none; +} .siteDetailsPage { - margin: 20px; + margin: 0; + padding-top: 40px; - .siteDetailsPageContent { - border-top: 1px solid @chromeBorderColor; - display: flex; + .siteDetailsPageHeader { + padding: 0 40px; - .siteDetailsList { - padding-top: 10px; - overflow: hidden; + .sectionTitle { + font-size: 28px; + display: inline-block; + } + .headerActions { + float: right; - .listItem { - display: flex; - height: 1rem; + .searchWrapper { + display: inline-block; - .aboutListItem { - align-items: center; - min-width: 0; + /*overridden from siteDetails.less*/ + input.searchInput { + float: none; + padding: 5px; + margin-top: 0; + margin-right: 12px; + font-size: 16px; + min-width: 280px; + height: 22px; + } + .searchInputPlaceholder { + color: @gray; + float: none; + font-family: FontAwesome; + left: -35px; + margin: 0; + padding: 0; + position: relative; + width: 0; + } + .searchInputClear { + float: none; + margin: 0; + padding: 0; + width: 0; + position: relative; + left: -35px; } } + /*copied from preferences.less*/ + span.browserButton.primaryButton.clearBrowsingDataButton { + font-size: 0.9em; + padding: 5px 20px; + margin: 0; + } + } + } + + .siteDetailsPageContent { + border-top: 0px; + margin-top: 28px; + display: block; + clear: both; + + .sectionTitle { + font-size: 16px; + margin-bottom: 12px; + padding-left: 40px; } } } diff --git a/less/sortableTable.less b/less/sortableTable.less index 20675232ee2..28993a6378f 100644 --- a/less/sortableTable.less +++ b/less/sortableTable.less @@ -116,4 +116,14 @@ table.sortableTable { color: #d9d9d9; } } + + &:not(.sort) { + th.sort-up:after { + content: ''; + } + + th.sort-down:after { + content: ''; + } + } } From d7be3206c8909f07ce56a76450ada395b2b0676c Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 10 Oct 2016 15:51:57 -0700 Subject: [PATCH 2/6] Integrated with multi-select (had to make a few tweaks) --- js/about/bookmarks.js | 16 ++++++++++------ js/components/sortableTable.js | 15 +++++++++++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js index add946927de..79e1d266faf 100644 --- a/js/about/bookmarks.js +++ b/js/about/bookmarks.js @@ -169,18 +169,19 @@ class BookmarksList extends ImmutableComponent { } } render () { - const props = !this.props.sortable ? { - sortingDisabled: true + const props = !this.props.draggable ? { + sortingDisabled: !this.props.sortable } : { onDoubleClick: this.onDoubleClick, onDragStart: this.onDragStart, onDragOver: this.onDragOver, onDrop: this.onDrop, - sortingDisabled: false + sortingDisabled: !this.props.sortable } return
[ { cell: , @@ -192,11 +193,12 @@ class BookmarksList extends ImmutableComponent { } ])} rowObjects={this.props.bookmarks} - defaultHeading='Title' + columnClassNames={['title', 'date']} + tableID={this.props.tableID} addHoverClass + multiSelect onDoubleClick={this.onDoubleClick} {...props} - columnClassNames={['title', 'date']} contextMenuName='bookmark' onContextMenu={aboutActions.contextMenu} />
@@ -288,7 +290,9 @@ class AboutBookmarks extends React.Component { ? this.searchedBookmarks(this.state.search, this.state.bookmarks) : this.bookmarksInFolder } - sortable={!this.state.search} /> + sortable={false} + draggable={!this.state.search} + tableID={this.selectedFolderId} /> diff --git a/js/components/sortableTable.js b/js/components/sortableTable.js index 807686e37ae..6f5dbc855e0 100644 --- a/js/components/sortableTable.js +++ b/js/components/sortableTable.js @@ -27,7 +27,13 @@ class SortableTable extends ImmutableComponent { return tableSort(this.table) } onClick (e) { - const targetElement = e.target.parentNode + // Work backwards until element is TR + let targetElement = e.target + while (targetElement) { + if (targetElement.tagName === 'TR') break + targetElement = targetElement.parentNode + } + if (!targetElement) return if (eventUtil.isForSecondaryAction(e)) { if (targetElement.className.includes(' selected')) { @@ -81,7 +87,12 @@ class SortableTable extends ImmutableComponent { ? (typeof this.props.totalRowObjects[parseInt(tableID)][index].toJS === 'function' ? this.props.totalRowObjects[parseInt(tableID)][index].toJS() : this.props.totalRowObjects[parseInt(tableID)][index]) - : null + : (this.props.rowObjects.size > 0 || this.props.rowObjects.length > 0) + ? (typeof this.props.rowObjects.toJS === 'function' + ? this.props.rowObjects.get(index).toJS() + : this.props.rowObjects[index]) + : null + if (handlerInput) { handlerInputs.push(handlerInput) } From b46947323f2a40c71fbf371bc92fc3fc43dfe9b0 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 10 Oct 2016 16:05:37 -0700 Subject: [PATCH 3/6] Optimization for history page. Only one call per history date is made now. Auditors: @darkdh --- js/about/history.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/about/history.js b/js/about/history.js index 646b924943a..f0e7112e9c1 100644 --- a/js/about/history.js +++ b/js/about/history.js @@ -118,10 +118,9 @@ class GroupedHistoryList extends ImmutableComponent { } return [] } - totalEntries (local) { - const origin = this.groupEntriesByDay(local) - let result = [] - origin.forEach((entry) => { + totalEntries (entriesByDay) { + const result = [] + entriesByDay.forEach((entry) => { result.push(entry.entries) }) return result @@ -129,12 +128,13 @@ class GroupedHistoryList extends ImmutableComponent { 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) let index = 0 return { - this.groupEntriesByDay(userLanguage || defaultLanguage).map((groupedEntry) => + entriesByDay.map((groupedEntry) => ) + totalEntries={this.totalEntries(entriesByDay)} tableID={index++} />) } } From bc63ccdbc002a2ae0fa7259f975eb5d8b45bf5e0 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 10 Oct 2016 17:16:28 -0700 Subject: [PATCH 4/6] Bookmark manager no longer opens multiple tabs when double clicked Fixes https://github.com/brave/browser-laptop/issues/4523 Also includes: Update color of about:bookmarks from "been gone hunting" green to orange --- app/extensions/brave/about-bookmarks.html | 2 +- js/about/bookmarks.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/extensions/brave/about-bookmarks.html b/app/extensions/brave/about-bookmarks.html index 089feb373db..8d1b5496caf 100644 --- a/app/extensions/brave/about-bookmarks.html +++ b/app/extensions/brave/about-bookmarks.html @@ -7,7 +7,7 @@ - + diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js index 79e1d266faf..3dba728f17c 100644 --- a/js/about/bookmarks.js +++ b/js/about/bookmarks.js @@ -146,7 +146,11 @@ class BookmarkTitleCell extends ImmutableComponent { } class BookmarksList extends ImmutableComponent { - onDoubleClick (entry) { + onDoubleClick (entry, e) { + if (e && e.preventDefault) { + e.preventDefault() + } + aboutActions.newFrame({ location: entry.location, partitionNumber: entry.partitionNumber From 41a10870cbd8c9d59761e577364bc7cefca86496 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Tue, 11 Oct 2016 00:28:34 -0700 Subject: [PATCH 5/6] The refactoring done in https://github.com/brave/browser-laptop/pull/4361 unfortunately broke autofill. This commit reverts the parts that broke it and keeps the parts that were OK. Auditors: @darkdh @bridiver Test Plan: follow steps from original PR --- app/common/lib/faviconUtil.js | 2 +- app/renderer/components/bookmarksToolbar.js | 35 ++++++++-------- docs/windowActions.md | 10 +---- js/actions/windowActions.js | 18 +++------ js/constants/windowConstants.js | 2 +- js/contextMenus.js | 18 ++++++++- js/stores/windowStore.js | 44 ++++----------------- 7 files changed, 49 insertions(+), 80 deletions(-) diff --git a/app/common/lib/faviconUtil.js b/app/common/lib/faviconUtil.js index 5625860ab70..7c95007b276 100644 --- a/app/common/lib/faviconUtil.js +++ b/app/common/lib/faviconUtil.js @@ -2,7 +2,7 @@ * 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 { isSourceAboutUrl } = require('../../../js/lib/appUrlUtil') +const {isSourceAboutUrl} = require('../../../js/lib/appUrlUtil') const UrlUtil = require('../../../js/lib/urlutil') module.exports.iconSize = 16 diff --git a/app/renderer/components/bookmarksToolbar.js b/app/renderer/components/bookmarksToolbar.js index e24bca73039..90db85f268c 100644 --- a/app/renderer/components/bookmarksToolbar.js +++ b/app/renderer/components/bookmarksToolbar.js @@ -32,20 +32,9 @@ class BookmarkToolbarButton extends ImmutableComponent { this.onDragOver = this.onDragOver.bind(this) this.onContextMenu = this.onContextMenu.bind(this) } - - showBookmarkFolderMenu (e) { - e.target = ReactDOM.findDOMNode(this) - - if (e && e.stopPropagation) { - e.stopPropagation() - } - - const xPos = (e.target.getBoundingClientRect().left | 0) - 2 - const yPos = (e.target.parentNode.getBoundingClientRect().bottom | 0) - 1 - - windowActions.onMouseOverBookmarkFolder(this.props.bookmark.get('folderId'), this.props.sites, this.props.bookmark, xPos, yPos) + get activeFrame () { + return windowStore.getFrame(this.props.activeFrameKey) } - onClick (e) { if (!this.props.clickBookmarkItem(this.props.bookmark, e) && this.props.bookmark.get('tags').includes(siteTags.BOOKMARK_FOLDER)) { @@ -53,7 +42,9 @@ class BookmarkToolbarButton extends ImmutableComponent { windowActions.setContextMenuDetail() return } - this.showBookmarkFolderMenu(e) + e.target = ReactDOM.findDOMNode(this) + this.props.showBookmarkFolderMenu(this.props.bookmark, e) + return } } @@ -62,10 +53,12 @@ class BookmarkToolbarButton extends ImmutableComponent { if (this.props.selectedFolderId) { if (this.isFolder && this.props.selectedFolderId !== this.props.bookmark.get('folderId')) { // Auto-expand the menu if user mouses over another folder - this.showBookmarkFolderMenu(e) + e.target = ReactDOM.findDOMNode(this) + this.props.showBookmarkFolderMenu(this.props.bookmark, e) } else if (!this.isFolder && this.props.selectedFolderId !== -1) { // Hide the currently expanded menu if user mouses over a non-folder - windowActions.onMouseOverBookmarkFolder(-1) + windowActions.setBookmarksToolbarSelectedFolderId(-1) + windowActions.setContextMenuDetail() } } } @@ -84,7 +77,7 @@ class BookmarkToolbarButton extends ImmutableComponent { e.target = ReactDOM.findDOMNode(this) if (dnd.isMiddle(e.target, e.clientX)) { e.target.getBoundingClientRect - this.showBookmarkFolderMenu(e) + this.props.showBookmarkFolderMenu(this.props.bookmark, e) windowActions.setIsBeingDraggedOverDetail(dragTypes.BOOKMARK, this.props.bookmark, { expanded: true }) @@ -238,6 +231,7 @@ class BookmarksToolbar extends ImmutableComponent { this.onMoreBookmarksMenu = this.onMoreBookmarksMenu.bind(this) this.openContextMenu = this.openContextMenu.bind(this) this.clickBookmarkItem = this.clickBookmarkItem.bind(this) + this.showBookmarkFolderMenu = this.showBookmarkFolderMenu.bind(this) } get activeFrame () { return windowStore.getFrame(this.props.activeFrameKey) @@ -293,6 +287,10 @@ class BookmarksToolbar extends ImmutableComponent { clickBookmarkItem (bookmark, e) { return bookmarkActions.clickBookmarkItem(this.bookmarks, bookmark, this.activeFrame, e) } + showBookmarkFolderMenu (bookmark, e) { + windowActions.setBookmarksToolbarSelectedFolderId(bookmark.get('folderId')) + contextMenus.onShowBookmarkFolderMenu(this.bookmarks, bookmark, this.activeFrame, e) + } updateBookmarkData (props) { this.bookmarks = siteUtil.getBookmarks(props.sites) @@ -407,11 +405,12 @@ class BookmarksToolbar extends ImmutableComponent { ref={(node) => this.bookmarkRefs.push(node)} key={i} contextMenuDetail={this.props.contextMenuDetail} + activeFrameKey={this.props.activeFrameKey} draggingOverData={this.props.draggingOverData} openContextMenu={this.openContextMenu} clickBookmarkItem={this.clickBookmarkItem} + showBookmarkFolderMenu={this.showBookmarkFolderMenu} bookmark={bookmark} - sites={this.bookmarks} showFavicon={this.props.showFavicon} showOnlyFavicon={this.props.showOnlyFavicon} selectedFolderId={this.props.selectedFolderId} />) diff --git a/docs/windowActions.md b/docs/windowActions.md index 5f89c612503..e3a05e9cb4b 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -891,7 +891,7 @@ See `eventStore.js` for an example use-case -### onMouseOverBookmarkFolder(folderId, bookmarks, bookmark, xPos, yPos) +### setBookmarksToolbarSelectedFolderId(folderId) Fired when the mouse clicks or hovers over a bookmark folder in the bookmarks toolbar @@ -900,14 +900,6 @@ Fired when the mouse clicks or hovers over a bookmark folder in the bookmarks to **folderId**: `number`, from the siteDetail for the bookmark folder If set to null, no menu is open. If set to -1, mouse is over a bookmark, not a folder -**bookmarks**: `Object`, all of the users bookmarks - -**bookmark**: `Object`, the object representing the selected folder - -**xPos**: `number`, x position of the left corner of the context mennu - -**yPos**: `number`, y position of the top corner of the context menu - diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 7e496f87d83..8f6505c096a 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1127,19 +1127,11 @@ const windowActions = { * Fired when the mouse clicks or hovers over a bookmark folder in the bookmarks toolbar * @param {number} folderId - from the siteDetail for the bookmark folder * If set to null, no menu is open. If set to -1, mouse is over a bookmark, not a folder - * @param {Object} bookmarks - all of the users bookmarks - * @param {Object} bookmark - the object representing the selected folder - * @param {number} xPos - x position of the left corner of the context mennu - * @param {number} yPos - y position of the top corner of the context menu - */ - onMouseOverBookmarkFolder: function (folderId, bookmarks, bookmark, xPos, yPos) { - dispatch({ - actionType: WindowConstants.WINDOW_ON_MOUSE_OVER_BOOKMARK_FOLDER, - folderId, - bookmarks, - bookmark, - xPos, - yPos + */ + setBookmarksToolbarSelectedFolderId: function (folderId) { + dispatch({ + actionType: WindowConstants.WINDOW_SET_BOOKMARKS_TOOLBAR_SELECTED_FOLDER_ID, + folderId }) } } diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index b0a4a1c88f7..ab1da77c51c 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -78,7 +78,7 @@ const windowConstants = { WINDOW_SET_SUBMENU_SELECTED_INDEX: _, WINDOW_SET_LAST_FOCUSED_SELECTOR: _, WINDOW_GOT_RESPONSE_DETAILS: _, - WINDOW_ON_MOUSE_OVER_BOOKMARK_FOLDER: _ + WINDOW_SET_BOOKMARKS_TOOLBAR_SELECTED_FOLDER_ID: _ } module.exports = mapValuesByKeys(windowConstants) diff --git a/js/contextMenus.js b/js/contextMenus.js index 611ee03fec3..4567ab2f8e6 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -1178,6 +1178,20 @@ function onLedgerContextMenu (location, hostPattern) { menu.destroy() } +function onShowBookmarkFolderMenu (bookmarks, bookmark, activeFrame, e) { + if (e && e.stopPropagation) { + e.stopPropagation() + } + const menuTemplate = showBookmarkFolderInit(bookmarks, bookmark, activeFrame) + const rectLeft = e.target.getBoundingClientRect() + const rectBottom = e.target.parentNode.getBoundingClientRect() + windowActions.setContextMenuDetail(Immutable.fromJS({ + left: (rectLeft.left | 0) - 2, + top: (rectBottom.bottom | 0) - 1, + template: menuTemplate + })) +} + /** * @param {Object} usernames - map of username to plaintext password * @param {string} origin - origin of the form @@ -1317,10 +1331,10 @@ module.exports = { onTabPageContextMenu, onUrlBarContextMenu, onSiteDetailContextMenu, + onShowBookmarkFolderMenu, onShowUsernameMenu, onShowAutofillMenu, onMoreBookmarksMenu, onBackButtonHistoryMenu, - onForwardButtonHistoryMenu, - showBookmarkFolderInit + onForwardButtonHistoryMenu } diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 194fc2c7bae..07b7ceff692 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -20,7 +20,6 @@ const {tabFromFrame} = require('../state/frameStateUtil') const {l10nErrorText} = require('../../app/common/lib/httpUtil') const {aboutUrls, getSourceAboutUrl, isIntermediateAboutPage, navigatableTypes} = require('../lib/appUrlUtil') const Serializer = require('../dispatcher/serializer') -const { showBookmarkFolderInit } = require('../contextMenus') let windowState = Immutable.fromJS({ activeFrameKey: null, @@ -238,35 +237,6 @@ const newFrame = (frameOpts, openInForeground, insertionIndex) => { const windowStore = new WindowStore() const emitChanges = debounce(windowStore.emitChanges.bind(windowStore), 5) -const showContextMenu = (action) => { - windowState = windowState.set('contextMenuDetail', action.detail) - // Drag and drop bookmarks code expects this to be set sync - windowStore.emitChanges() -} - -const hideContextMenu = (action) => { - windowState = windowState.delete('contextMenuDetail') - // Drag and drop bookmarks code expects this to be set sync - windowStore.emitChanges() -} - -const onMouseOverBookmarkFolder = (action) => { - windowState = windowState.setIn(['ui', 'bookmarksToolbar', 'selectedFolderId'], action.folderId) - - if (action.folderId && action.folderId !== -1) { - const activeFrame = FrameStateUtil.getActiveFrame(windowState) - showContextMenu({ - detail: Immutable.fromJS({ - left: action.xPos, - top: action.yPos, - template: showBookmarkFolderInit(action.bookmarks, action.bookmark, activeFrame) - }) - }) - } else if (action.folderId === -1) { - hideContextMenu(action) - } -} - // Register callback to handle all updates const doAction = (action) => { // console.log(action.actionType, action, windowState.toJS()) @@ -634,10 +604,12 @@ const doAction = (action) => { return case WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL: if (!action.detail) { - hideContextMenu(action) + windowState = windowState.delete('contextMenuDetail') } else { - showContextMenu(action) + windowState = windowState.set('contextMenuDetail', action.detail) } + // Drag and drop bookmarks code expects this to be set sync + windowStore.emitChanges() return case WindowConstants.WINDOW_SET_POPUP_WINDOW_DETAIL: if (!action.detail) { @@ -824,7 +796,7 @@ const doAction = (action) => { break case WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE: if (getSetting(settings.AUTO_HIDE_MENU)) { - hideContextMenu(action) + doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL}) // Use value if provided; if not, toggle to opposite. const newVisibleStatus = typeof action.isVisible === 'boolean' ? action.isVisible @@ -841,7 +813,7 @@ const doAction = (action) => { if (getSetting(settings.AUTO_HIDE_MENU)) { doAction({actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE, isVisible: false}) } else { - hideContextMenu(action) + doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL}) } doAction({actionType: WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX}) doAction({actionType: WindowConstants.WINDOW_SET_BOOKMARKS_TOOLBAR_SELECTED_FOLDER_ID}) @@ -856,8 +828,8 @@ const doAction = (action) => { case WindowConstants.WINDOW_SET_LAST_FOCUSED_SELECTOR: windowState = windowState.setIn(['ui', 'menubar', 'lastFocusedSelector'], action.selector) break - case WindowConstants.WINDOW_ON_MOUSE_OVER_BOOKMARK_FOLDER: - onMouseOverBookmarkFolder(action) + case WindowConstants.WINDOW_SET_BOOKMARKS_TOOLBAR_SELECTED_FOLDER_ID: + windowState = windowState.setIn(['ui', 'bookmarksToolbar', 'selectedFolderId'], action.folderId) break default: From 3b730b902e96412ab577d15e5fbd9cd24bdd9157 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Tue, 11 Oct 2016 01:57:40 -0700 Subject: [PATCH 6/6] Reworking about:bookmarks after feedback from Brad. Still some more to do. --- .../brave/locales/en-US/bookmarks.properties | 1 - js/about/bookmarks.js | 15 +++++++++------ less/about/bookmarks.less | 11 +++++++++++ less/about/siteDetails.less | 4 ++-- package.json | 1 - 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/app/extensions/brave/locales/en-US/bookmarks.properties b/app/extensions/brave/locales/en-US/bookmarks.properties index 4c775e94277..681399730a4 100644 --- a/app/extensions/brave/locales/en-US/bookmarks.properties +++ b/app/extensions/brave/locales/en-US/bookmarks.properties @@ -1,7 +1,6 @@ bookmarks=Bookmarks bookmarkManager=Bookmark Manager folders=Folders -organize=Organize partitionNumber=Session {{partitionNumber}} bookmarksToolbar=Bookmarks Toolbar otherBookmarks=Other Bookmarks diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js index 3dba728f17c..8d7c9fe2635 100644 --- a/js/about/bookmarks.js +++ b/js/about/bookmarks.js @@ -13,7 +13,6 @@ const aboutActions = require('./aboutActions') const dndData = require('../dndData') const cx = require('../lib/classSet') const SortableTable = require('../components/sortableTable') -const Button = require('../components/button') const siteUtil = require('../state/siteUtil') const iconSize = 16 @@ -59,7 +58,13 @@ class BookmarkFolderItem extends ImmutableComponent { listItem: true, selected: this.props.selected })}> - + + {this.props.bookmarkFolder.get('customTitle') || this.props.bookmarkFolder.get('title')} @@ -200,7 +205,6 @@ class BookmarksList extends ImmutableComponent { columnClassNames={['title', 'date']} tableID={this.props.tableID} addHoverClass - multiSelect onDoubleClick={this.onDoubleClick} {...props} contextMenuName='bookmark' @@ -265,6 +269,7 @@ class AboutBookmarks extends React.Component {
+
{ @@ -273,13 +278,12 @@ class AboutBookmarks extends React.Component { : }
-
-
+
bookmark.get('parentFolderId') === -1)} allBookmarkFolders={this.state.bookmarkFolders} @@ -287,7 +291,6 @@ class AboutBookmarks extends React.Component { selectedFolderId={this.state.selectedFolderId} />
-
.bookmarkFolderList { display: block; border-top: solid 1px @gray; diff --git a/less/about/siteDetails.less b/less/about/siteDetails.less index 9e15e9d88fd..136d4d9c41e 100644 --- a/less/about/siteDetails.less +++ b/less/about/siteDetails.less @@ -19,13 +19,13 @@ body { .siteDetailsPage { margin: 0; - padding-top: 40px; + padding-top: 20px; .siteDetailsPageHeader { padding: 0 40px; .sectionTitle { - font-size: 28px; + font-size: 24px; display: inline-block; } .headerActions { diff --git a/package.json b/package.json index 163d6fc7c41..32b8be20294 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,6 @@ "random-lib": "2.1.0", "react": "^15.0.1", "react-dom": "^15.0.1", - "react-stickynode": "1.1.x", "spellchecker": "^3.3.1", "string.prototype.endswith": "^0.2.0", "string.prototype.startswith": "^0.2.0",
@@ -175,14 +194,20 @@ class SortableTable extends ImmutableComponent { } }) - const rowAttributes = this.getRowAttributes(row, i) + const rowAttributes = row.length + ? this.getRowAttributes(row, i) + : null return row.length ?