From 0e6ba2884266a62f93a7c9bc8e8914746dbba95b Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Thu, 27 Oct 2016 15:50:49 -0700 Subject: [PATCH 1/2] Context menu now works on sortableTable with no selection Also fixes: focus for new folder / edit folder / new bookmark now set properly Fixes https://github.com/brave/browser-laptop/issues/5202 Auditors: @darkdh --- app/renderer/components/addEditBookmarkHanger.js | 7 +++++-- js/components/addEditBookmark.js | 4 ++++ js/components/sortableTable.js | 15 ++++++++++++++- js/contextMenus.js | 2 ++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/app/renderer/components/addEditBookmarkHanger.js b/app/renderer/components/addEditBookmarkHanger.js index d2fbbf68968..a03e1d642b2 100644 --- a/app/renderer/components/addEditBookmarkHanger.js +++ b/app/renderer/components/addEditBookmarkHanger.js @@ -55,6 +55,10 @@ class AddEditBookmarkHanger extends ImmutableComponent { get isFolder () { return siteUtil.isFolder(this.props.currentDetail) } + setDefaultFocus () { + this.bookmarkName.select() + this.bookmarkName.focus() + } updateFolders (props) { this.folders = siteUtil.getFolders(this.props.sites, props.currentDetail.get('folderId')) } @@ -71,8 +75,7 @@ class AddEditBookmarkHanger extends ImmutableComponent { if (!this.props.isModal && !this.props.shouldShowLocation) { this.onSave(false) } - this.bookmarkName.select() - this.bookmarkName.focus() + this.setDefaultFocus() } onKeyDown (e) { switch (e.keyCode) { diff --git a/js/components/addEditBookmark.js b/js/components/addEditBookmark.js index 284eecd8c63..0070e294e91 100644 --- a/js/components/addEditBookmark.js +++ b/js/components/addEditBookmark.js @@ -16,9 +16,13 @@ class AddEditBookmark extends ImmutableComponent { onClose () { windowActions.setBookmarkDetail() } + componentDidMount () { + this.refs.bookmarkHanger.setDefaultFocus() + } render () { return 1 @@ -261,9 +266,17 @@ class SortableTable extends React.Component { // Bindings for multi-select-specific event handlers if (this.props.multiSelect) { - rowAttributes.onContextMenu = this.onContextMenu + // Table supports multi-select rowAttributes.onClick = this.onClick + if (this.nullSelection && this.hasContextMenu) { + // If nothing is selected yet, offer a default per-item context menu + rowAttributes.onContextMenu = this.props.onContextMenu.bind(this, handlerInput, this.props.contextMenuName) + } else { + // If items are selected we must use the multiple item handler + rowAttributes.onContextMenu = this.onContextMenu + } } else { + // Table does not support multi-select if (this.hasContextMenu) { rowAttributes.onContextMenu = this.props.onContextMenu.bind(this, handlerInput, this.props.contextMenuName) } diff --git a/js/contextMenus.js b/js/contextMenus.js index 2bd3d0a390a..1b7c4a369e7 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -255,6 +255,8 @@ function siteDetailTemplateInit (siteDetail, activeFrame) { } else if (multipleHistoryEntries) { deleteLabel = 'deleteHistoryEntries' } + } else { + deleteLabel = '' } const template = [] From d41a2d835292d32af6e8a56766d888dc91d599cd Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Thu, 27 Oct 2016 20:03:14 -0400 Subject: [PATCH 2/2] Additional immutable check for rowObjects Auditors: @bsclifton Test Plan: 1. go to "about:history" 2. do not select anything 3. right click on an arbitrary entry 4. context menu should show --- js/components/sortableTable.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js/components/sortableTable.js b/js/components/sortableTable.js index 567eace1a99..f19260e8901 100644 --- a/js/components/sortableTable.js +++ b/js/components/sortableTable.js @@ -253,7 +253,9 @@ class SortableTable extends React.Component { (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]) + : (typeof this.props.rowObjects[index].toJS === 'function' + ? this.props.rowObjects[index].toJS() + : this.props.rowObjects[index])) : row // Allow parent control to optionally specify context