Merge pull request #4690 from bsclifton/bookmark-manager-style-update

Style updates to bookmark manager per @bradleyrichter
brave · Oct 13, 2016 · e7214f8 · e7214f8
2 parents c868874 + 92da6ee
commit e7214f8
Show file tree

Hide file tree

Showing 10 changed files with 130 additions and 49 deletions.
diff --git a/app/extensions/brave/locales/en-US/bookmarks.properties b/app/extensions/brave/locales/en-US/bookmarks.properties
@@ -5,4 +5,5 @@ partitionNumber=Session {{partitionNumber}}
 bookmarksToolbar=Bookmarks Toolbar
 otherBookmarks=Other Bookmarks
 bookmarkSearch.placeholder=Search bookmarks
-importBrowserData=Import browser data
+importBrowserData.title=Import browser data
+allFolders=All Folders
diff --git a/js/about/bookmarks.js b/js/about/bookmarks.js
@@ -14,7 +14,7 @@ const dndData = require('../dndData')
 const cx = require('../lib/classSet')
 const SortableTable = require('../components/sortableTable')
 const siteUtil = require('../state/siteUtil')
-const iconSize = 16
+const iconSize = require('../../app/common/lib/faviconUtil').iconSize
 
 const ipc = window.chrome.ipc
 
@@ -54,6 +54,7 @@ class BookmarkFolderItem extends ImmutableComponent {
         onContextMenu={aboutActions.contextMenu.bind(this, this.props.bookmarkFolder.toJS(), 'bookmark-folder')}
         onClick={this.props.onChangeSelectedFolder.bind(null, this.props.bookmarkFolder.get('folderId'))}
         draggable={this.props.draggable !== false ? 'true' : 'false'}
+        data-folder-id={this.props.bookmarkFolder.get('folderId')}
         className={cx({
           listItem: true,
           selected: this.props.selected
@@ -66,7 +67,8 @@ class BookmarkFolderItem extends ImmutableComponent {
           'fa-folder-open-o': this.props.selected
         })} />
         <span data-l10n-id={this.props.dataL10nId}>
-          {this.props.bookmarkFolder.get('customTitle') || this.props.bookmarkFolder.get('title')}</span>
+          {this.props.bookmarkFolder.get('customTitle') || this.props.bookmarkFolder.get('title')}
+        </span>
       </div>
       {
         childBookmarkFolders.size > 0
@@ -83,9 +85,17 @@ class BookmarkFolderItem extends ImmutableComponent {
 class BookmarkFolderList extends ImmutableComponent {
   render () {
     return <list className='bookmarkFolderList'>
+      {
+        this.props.isRoot && this.props.search
+        ? <div role='listitem' className='listItem selected'>
+          <span className='bookmarkFolderIcon fa fa-search' />
+          <span data-l10n-id='allFolders' />
+        </div>
+        : null
+      }
       {
         this.props.isRoot
-        ? <BookmarkFolderItem selected={this.props.selectedFolderId === 0}
+        ? <BookmarkFolderItem selected={!this.props.search && this.props.selectedFolderId === 0}
           dataL10nId='bookmarksToolbar'
           draggable={false}
           onChangeSelectedFolder={this.props.onChangeSelectedFolder}
@@ -98,7 +108,7 @@ class BookmarkFolderList extends ImmutableComponent {
         this.props.bookmarkFolders.map((bookmarkFolder) =>
           <BookmarkFolderItem bookmarkFolder={bookmarkFolder}
             allBookmarkFolders={this.props.allBookmarkFolders}
-            selected={this.props.selectedFolderId === bookmarkFolder.get('folderId')}
+            selected={!this.props.search && this.props.selectedFolderId === bookmarkFolder.get('folderId')}
             selectedFolderId={this.props.selectedFolderId}
             onChangeSelectedFolder={this.props.onChangeSelectedFolder} />)
       }
@@ -194,7 +204,7 @@ class BookmarksList extends ImmutableComponent {
         rows={this.props.bookmarks.map((entry) => [
           {
             cell: <BookmarkTitleCell siteDetail={entry} />,
-            value: entry.get('customTitle') || entry.get('title')
+            value: entry.get('customTitle') || entry.get('title') || entry.get('location')
           },
           {
             html: new Date(entry.get('lastAccessedTime')).toLocaleString(),
@@ -269,7 +279,6 @@ class AboutBookmarks extends React.Component {
       <div className='siteDetailsPageHeader'>
         <div data-l10n-id='bookmarkManager' className='sectionTitle' />
         <div className='headerActions'>
-          <span l10nId='importBrowserData' className='fa fa-download clearBrowsingDataButton' onClick={this.importBrowserData} />
           <div className='searchWrapper'>
             <input type='text' className='searchInput' ref='bookmarkSearch' id='bookmarkSearch' value={this.state.search} onChange={this.onChangeSearch} data-l10n-id='bookmarkSearch' />
             {
@@ -283,12 +292,16 @@ class AboutBookmarks extends React.Component {
 
       <div className='siteDetailsPageContent'>
         <div className='folderView'>
-          <div data-l10n-id='folders' className='columnHeader' />
+          <div className='columnHeader'>
+            <span data-l10n-id='folders' />
+            <span data-l10n-id='importBrowserData' className='fa fa-download clearBrowsingDataButton' onClick={this.importBrowserData} />
+          </div>
           <BookmarkFolderList onChangeSelectedFolder={this.onChangeSelectedFolder}
             bookmarkFolders={this.state.bookmarkFolders.filter((bookmark) => bookmark.get('parentFolderId') === -1)}
             allBookmarkFolders={this.state.bookmarkFolders}
             isRoot
-            selectedFolderId={this.state.selectedFolderId} />
+            selectedFolderId={this.state.selectedFolderId}
+            search={this.state.search} />
         </div>
         <div className='organizeView'>
           <BookmarksList

diff --git a/less/about/bookmarks.less b/less/about/bookmarks.less
@@ -1,13 +1,22 @@
 @import "./siteDetails.less";
 
 .siteDetailsPage {
+  .siteDetailsPageHeader {
+    .headerActions {
+      input.searchInput {
+        margin-right: 0 !important;
+      }
+    }
+  }
+
   .siteDetailsPageContent {
     display: flex;
+    flex-wrap: wrap;
     height: 100%;
 
     .folderView {
       min-width: 220px;
-      flex-grow: 0;
+      flex-grow: 1;
 
       .columnHeader {
         background: linear-gradient(to bottom, @lightGray, @veryLightGray);
@@ -18,23 +27,35 @@
         box-sizing: border-box;
         border-right: 1px solid @braveOrange;
         padding-left: 10px;
+        -webkit-user-select: none;
+        cursor: default;
+
+        .clearBrowsingDataButton {
+          color: @buttonColor;
+          float: right;
+          font-size: 20px;
+          margin: 0;
+        }
+        .clearBrowsingDataButton:hover {
+          color: #000;
+        }
       }
 
       > .bookmarkFolderList {
         display: block;
-        border-top: solid 1px @gray;
         border-right: 1px solid @braveOrange;
         height: 100%;
+
+        .listItem >* {
+          color: @buttonColor;
+        }
       }
 
       .bookmarkFolderList {
         .listItem {
           height: 1rem;
-          &:hover {
-            background-color: #ffcc99 !important;
-          }
-          &.selected {
-            background-color: lighten(@braveOrange, 20%) !important;
+          &.selected >* {
+            font-weight: 800;
           }
         }
         .bookmarkFolderIcon {
@@ -43,12 +64,9 @@
       }
     }
     .organizeView {
-      flex-grow: 1;
+      flex-grow: 5;
 
       .sortableTable {
-        border: 0;
-        box-shadow: none;
-        table-layout: fixed;
         -webkit-user-select: none;
         th:nth-of-type(2), td.date {
           width: 250px;

diff --git a/less/about/history.less b/less/about/history.less
@@ -5,6 +5,7 @@
 
   .siteDetailsPageContent {
     .sortableTable {
+      -webkit-user-select: none;
       // Time visited
       .time {
         font-size: 11pt;

diff --git a/less/about/itemList.less b/less/about/itemList.less
@@ -5,13 +5,13 @@ list .listItem {
   overflow: hidden;
   white-space: nowrap;
 
-  padding: 12px;
+  padding: 8px 12px;
   -webkit-user-select: none;
   &:hover {
-    background-color: lighten(@highlightBlue, 30%);
+    background-color: @lightGray;
   }
   &.selected {
-    background-color: lighten(@highlightBlue, 20%);
+    background-color: lighten(@gray, 20%);
   }
   .aboutListItem {
     display: flex;

diff --git a/less/about/siteDetails.less b/less/about/siteDetails.less
@@ -19,10 +19,10 @@ body {
 
 .siteDetailsPage {
   margin: 0;
-  padding-top: 20px;
+  padding-top: 24px;
 
   .siteDetailsPageHeader {
-    padding: 0 40px;
+    padding: 0 @aboutPageSectionPadding;
 
     .sectionTitle {
       font-size: 24px;
@@ -74,24 +74,18 @@ body {
 
   .siteDetailsPageContent {
     border-top: 0px;
-    margin-top: 28px;
+    margin-top: 24px;
     display: block;
     clear: both;
 
     .sectionTitle {
       font-size: 16px;
       margin-bottom: 12px;
-      padding-left: 40px;
+      padding-left: @aboutPageSectionPadding;
     }
   }
 }
 
-.searchInput {
-  float: right;
-  padding: 5px;
-  margin-top: -35px;
-}
-
 .searchInputClear {
   float: right;
   padding: 8px;

diff --git a/less/sortableTable.less b/less/sortableTable.less
@@ -6,7 +6,7 @@
 
 table.sort {
   th {
-    color: @gray;
+    color: @buttonColor;
 
     &:hover {
       color: #000;
@@ -16,11 +16,7 @@ table.sort {
 
 table.sortableTable {
   width: 100%;
-  border: solid 1px @lightGray;
-  border-bottom-left-radius: @borderRadius;
-  border-bottom-right-radius: @borderRadius;
   margin-bottom: 40px;
-  box-shadow: @softBoxShadow;
   cursor: default;
   border-spacing: 0;
 
@@ -96,24 +92,24 @@ table.sortableTable {
     }
   }
   tr.rowHover:hover {
-    background: #ffcc99;
+    background: @lightGray;
     &.selected {
       background: @braveOrange;
       td {
-        color: @lightGray;
+        color: white;
       }
       div {
-        color: @lightGray;
+        color: white;
       }
     }
   }
   tr.selected {
-    background: @braveLightOrange;
+    background-color: lighten(@gray, 20%);
     td {
-      color: #d9d9d9;
+      color: @buttonColor;
     }
     div {
-      color: #d9d9d9;
+      color: @buttonColor;
     }
   }
 

diff --git a/less/variables.less b/less/variables.less
@@ -133,6 +133,8 @@
 @breakpointTinyWin32: 500px;
 @transitionDuration: 100ms;
 
+@aboutPageSectionPadding: 24px;
+
 @transition: all 600ms linear;
 @transitionFast: all 100ms linear;
 @transitionSlow: all 1s linear;

diff --git a/test/about/bookmarksManagerTest.js b/test/about/bookmarksManagerTest.js
@@ -0,0 +1,60 @@
+/* global describe, it, before */
+
+const Brave = require('../lib/brave')
+const {urlInput} = require('../lib/selectors')
+const {getTargetAboutUrl} = require('../../js/lib/appUrlUtil')
+const siteTags = require('../../js/constants/siteTags')
+const aboutBookmarksUrl = getTargetAboutUrl('about:bookmarks')
+
+describe('about:bookmarks', function () {
+  Brave.beforeAll(this)
+
+  const folderId = Math.random()
+  const lastVisit = 1476140184441
+  const bookmarkTag = [siteTags.BOOKMARK]
+
+  before(function * () {
+    yield this.app.client
+      .waitUntilWindowLoaded()
+      .waitForUrl(Brave.newTabUrl)
+      .waitForBrowserWindow()
+      .waitForVisible(urlInput)
+      .windowByUrl(Brave.browserWindowUrl)
+      .addSite({
+        customTitle: 'demo1',
+        folderId: folderId,
+        parentFolderId: 0,
+        tags: [siteTags.BOOKMARK_FOLDER]
+      }, siteTags.BOOKMARK_FOLDER)
+      .addSite({ location: 'https://brave.com', title: 'Brave', tags: bookmarkTag, parentFolderId: 0, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .addSite({ location: 'https://brave.com/test', title: 'Test', customTitle: 'customTest', tags: bookmarkTag, parentFolderId: 0, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .addSite({ location: 'https://www.youtube.com', tags: bookmarkTag, parentFolderId: 0, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .addSite({ location: 'https://www.facebook.com', title: 'facebook', tags: bookmarkTag, parentFolderId: 0, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .addSite({ location: 'https://duckduckgo.com', title: 'duckduckgo', tags: bookmarkTag, parentFolderId: folderId, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .addSite({ location: 'https://google.com', title: 'Google', tags: bookmarkTag, parentFolderId: folderId, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .addSite({ location: 'https://bing.com', title: 'Bing', tags: bookmarkTag, parentFolderId: folderId, lastAccessedTime: lastVisit }, siteTags.BOOKMARK)
+      .waitForExist('.tab[data-frame-key="1"]')
+      .tabByIndex(0)
+      .url(aboutBookmarksUrl)
+  })
+
+  it('displays entries with title', function * () {
+    yield this.app.client
+      .waitForVisible('table.sortableTable td.title[data-sort="Brave"]')
+  })
+
+  it('displays entries without a title using their URL', function * () {
+    yield this.app.client
+      .waitForVisible('table.sortableTable td.title[data-sort="https://www.youtube.com"]')
+  })
+
+  it('displays entries using customTitle (if available)', function * () {
+    yield this.app.client
+      .waitForVisible('table.sortableTable td.title[data-sort="customTest"]')
+  })
+
+  it('shows bookmark folders', function * () {
+    yield this.app.client
+      .waitForVisible('.bookmarkFolderList .listItem[data-folder-id="' + folderId + '"]')
+  })
+})
diff --git a/test/about/historyTest.js b/test/about/historyTest.js
@@ -34,15 +34,11 @@ describe('about:history', function () {
       .waitForVisible('table.sortableTable td.title[data-sort="customTest"]', 1000, true)
   })
 
-  // shows ordered by date
-
   it('defaults to sorting table by time DESC', function * () {
     yield this.app.client
-      .waitForVisible('table.sortableTable thead tr th.sort-up[data-l10n-id="time"]')
+      .waitForVisible('table.sortableTable thead tr th.sort-up div[data-l10n-id="time"]')
   })
 
-  // search box
-
   // Multi Select
   it('Simulate cmd/control click behavior', function * () {
     yield this.app.client