From cd2192cdf7b06073b8d010157811f28c5b1b5e5e Mon Sep 17 00:00:00 2001
From: Brian Clifton <brian@clifton.me>
Date: Tue, 1 Nov 2016 15:37:25 -0700
Subject: [PATCH] Bug fixes for about:bookmarks: - clear selection when click
 is outside of table - clear selection when any element is dragged

Fixes https://github.com/brave/browser-laptop/issues/5223
Fixes https://github.com/brave/browser-laptop/issues/5249

Auditors: @darkdh

Test Plan:
1. Launch Brave and visit about:bookmarks
2. Import bookmarks or have bookmarks ready
3. Drag bookmarks from the view on the right over to a different folder
4. Observe that the selection is cleared
5. Use Ctrl or Cmd and select multiple items
6. Click below the table or click the header for the table
7. Notice the selection is cleared
---
 js/about/bookmarks.js              | 44 ++++++++++++++++++++++++++++--
 js/constants/appConfig.js          |  2 +-
 test/about/bookmarksManagerTest.js | 11 ++++++++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js
index 5d8c69d8e5b..b459561ebbc 100644
--- a/js/about/bookmarks.js
+++ b/js/about/bookmarks.js
@@ -62,6 +62,11 @@ class BookmarkFolderItem extends React.Component {
         true)
     }
   }
+  clearSelection () {
+    if (this.props.onClearSelection) {
+      this.props.onClearSelection()
+    }
+  }
   // NOTE: both folders AND bookmarks can be dropped here
   onDrop (e) {
     this.setState({
@@ -74,10 +79,12 @@ class BookmarkFolderItem extends React.Component {
         bookmarkData.forEach((bookmark) => {
           this.moveBookmark(e, bookmark)
         })
+        this.clearSelection()
         return
       }
 
       this.moveBookmark(e, bookmarkData)
+      this.clearSelection()
     }
   }
   render () {
@@ -137,6 +144,7 @@ class BookmarkFolderList extends ImmutableComponent {
       {
         this.props.isRoot
         ? <BookmarkFolderItem
+          onClearSelection={this.props.onClearSelection}
           search={this.props.search}
           selected={!this.props.search && this.props.selectedFolderId === 0}
           dataL10nId='bookmarksToolbar'
@@ -151,7 +159,9 @@ class BookmarkFolderList extends ImmutableComponent {
         this.props.bookmarkFolders.map((bookmarkFolder) =>
           this.props.isRoot && bookmarkFolder.get('parentFolderId') === -1
           ? null
-          : <BookmarkFolderItem bookmarkFolder={bookmarkFolder}
+          : <BookmarkFolderItem
+            onClearSelection={this.props.onClearSelection}
+            bookmarkFolder={bookmarkFolder}
             allBookmarkFolders={this.props.allBookmarkFolders}
             search={this.props.search}
             selected={!this.props.search && this.props.selectedFolderId === bookmarkFolder.get('folderId')}
@@ -161,6 +171,7 @@ class BookmarkFolderList extends ImmutableComponent {
       {
         this.props.isRoot
         ? <BookmarkFolderItem
+          onClearSelection={this.props.onClearSelection}
           search={this.props.search}
           selected={!this.props.search && this.props.selectedFolderId === -1}
           dataL10nId='otherBookmarks'
@@ -293,12 +304,17 @@ class BookmarksList extends ImmutableComponent {
         bookmarkData.forEach((bookmark) => {
           this.moveBookmark(e, bookmark, siteDetail)
         })
+        this.clearSelection()
         return
       }
 
       this.moveBookmark(e, bookmarkData, siteDetail)
+      this.clearSelection()
     }
   }
+  clearSelection () {
+    this.refs.bookmarkTable.clearSelection()
+  }
   render () {
     const props = !this.props.draggable ? {
       onDragStart: this.onDragStart,
@@ -312,6 +328,7 @@ class BookmarksList extends ImmutableComponent {
     }
     return <div>
       <SortableTable
+        ref='bookmarkTable'
         headings={[
           <BookmarkTitleHeader heading='Title' selectedFolderId={this.props.selectedFolderId} />,
           'Last Visited'
@@ -348,6 +365,8 @@ class AboutBookmarks extends React.Component {
     this.onClearSearchText = this.onClearSearchText.bind(this)
     this.importBrowserData = this.importBrowserData.bind(this)
     this.addBookmarkFolder = this.addBookmarkFolder.bind(this)
+    this.onClick = this.onClick.bind(this)
+    this.clearSelection = this.clearSelection.bind(this)
     this.state = {
       bookmarks: Immutable.List(),
       bookmarkFolders: Immutable.Map(),
@@ -377,6 +396,19 @@ class AboutBookmarks extends React.Component {
       search: ''
     })
   }
+  onClick (e) {
+    // Determine if click was on sortableTable
+    let targetElement = e.target
+    while (targetElement) {
+      if (targetElement.tagName === 'TBODY') {
+        return
+      }
+      targetElement = targetElement.parentNode
+    }
+
+    // Click was not a child element of sortableTable; clear selection
+    this.clearSelection()
+  }
   searchedBookmarks (searchTerm, bookmarks) {
     return bookmarks.filter((bookmark) => {
       const title = bookmark.get('customTitle') + bookmark.get('title') + bookmark.get('location')
@@ -396,6 +428,9 @@ class AboutBookmarks extends React.Component {
     })
     aboutActions.showAddBookmarkFolder(newFolder)
   }
+  clearSelection () {
+    this.refs.bookmarkList.clearSelection()
+  }
   componentDidMount () {
     this.refs.bookmarkSearch.focus()
   }
@@ -415,14 +450,16 @@ class AboutBookmarks extends React.Component {
         </div>
       </div>
 
-      <div className='siteDetailsPageContent'>
+      <div className='siteDetailsPageContent' onClick={this.onClick}>
         <div className='folderView'>
           <div className='columnHeader'>
             <span data-l10n-id='folders' />
             <span data-l10n-id='importBrowserData' className='fa fa-download clearBrowsingDataButton' onClick={this.importBrowserData} />
             <span data-l10n-id='addBookmarkFolder' className='addBookmarkFolder' onClick={this.addBookmarkFolder} />
           </div>
-          <BookmarkFolderList onChangeSelectedFolder={this.onChangeSelectedFolder}
+          <BookmarkFolderList
+            onClearSelection={this.clearSelection}
+            onChangeSelectedFolder={this.onChangeSelectedFolder}
             bookmarkFolders={this.state.bookmarkFolders.filter((bookmark) => bookmark.get('parentFolderId') === -1)}
             allBookmarkFolders={this.state.bookmarkFolders}
             isRoot
@@ -431,6 +468,7 @@ class AboutBookmarks extends React.Component {
         </div>
         <div className='organizeView'>
           <BookmarksList
+            ref='bookmarkList'
             bookmarks={
               this.state.search
               ? this.searchedBookmarks(this.state.search, this.state.bookmarks)
diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js
index 4fe211e53a7..7d4fba66de4 100644
--- a/js/constants/appConfig.js
+++ b/js/constants/appConfig.js
@@ -99,7 +99,7 @@ module.exports = {
     'general.language': null, // null means to use the OS lang
     'general.startup-mode': 'lastTime',
     'general.homepage': 'https://www.brave.com',
-    'general.newtab-mode-TEMP': 'blank',
+    'general.newtab-mode-TEMP': process.env.NODE_ENV === 'test' ? 'newTabPage' : 'blank',
     'general.show-home-button': false,
     'general.useragent.value': null, // Set at runtime
     'general.autohide-menu': true,
diff --git a/test/about/bookmarksManagerTest.js b/test/about/bookmarksManagerTest.js
index acb559cd4e7..9b5be059067 100644
--- a/test/about/bookmarksManagerTest.js
+++ b/test/about/bookmarksManagerTest.js
@@ -188,5 +188,16 @@ describe('about:bookmarks', function () {
         .keys(Brave.keys.SHIFT)
         .click('table.sortableTable td.title[data-sort="Brave"]')
     })
+    it('deselects everything if something other than a row is clicked', function * () {
+      yield this.app.client
+        .tabByUrl(aboutBookmarksUrl)
+        .loadUrl(aboutBookmarksUrl)
+        // Click one bookmark, to select it
+        .click('table.sortableTable td.title[data-sort="Brave"]')
+        .waitForVisible('table.sortableTable tr.selected td.title[data-sort="Brave"]')
+        // Click the header; this should dismiss and release selection
+        .click('table.sortableTable th')
+        .waitForVisible('table.sortableTable tr.selected td.title[data-sort="Brave"]', 5000, true)
+    })
   })
 })