Bookmark manager style update #4658

bsclifton · 2016-10-11T00:20:02Z

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).

Fixes #4562
Fixes #1994
Fixes #4523

Completely updated styles

Almost done- needs some tweaks

Test Plan:

Visit about:bookmarks
Test the functionality:
- Try drag and dropping (both folders and bookmarks and between each)
- Try right click > context menu on bookmarks
- Try clicking the import button
- Try resizing the window; how does it look?
- Use the search in the top right; ensure this works as expected

- 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 #4562 Fixes #1994

@darkdh

… now. Auditors: @darkdh

Fixes #4523 Also includes: Update color of about:bookmarks from "been gone hunting" green to orange

@darkdh

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

…o do.

bsclifton · 2016-10-11T09:04:55Z

Almost there- one more session it'll be knocked out. All the code is done- just needs some style updates / tweaks.

@bbondy since it requires additional work and we're in a code freeze, we can always kick to 0.12.6. Let me know

bbondy · 2016-10-11T12:37:14Z

docs/windowActions.md

@@ -891,7 +891,7 @@ See `eventStore.js` for an example use-case



-### onMouseOverBookmarkFolder(folderId, bookmarks, bookmark, xPos, yPos) 
+### setBookmarksToolbarSelectedFolderId(folderId) 


Just noting I think we're trying to move away from setter actions

bbondy · 2016-10-11T12:38:16Z

++

bradleyrichter · 2016-10-11T13:22:28Z

Be sure to ref #4562 for a few more changes...

darkdh · 2016-10-11T15:06:02Z

js/components/sortableTable.js

+      if (targetElement.tagName === 'TR') break
+      targetElement = targetElement.parentNode
+    }
+    if (!targetElement) return


Nice for parent traversing!!!

darkdh · 2016-10-11T15:07:02Z

js/components/sortableTable.js

@@ -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)


Thanks for taking care of non multi table selection. 👍

bsclifton · 2016-10-11T21:55:37Z

Tests were added here:
#4690

bsclifton added 4 commits October 10, 2016 17:18

Updates to about:bookmarks

eea84e5

- 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 #4562 Fixes #1994

Integrated with multi-select (had to make a few tweaks)

d7be320

Optimization for history page. Only one call per history date is made…

b469473

… now. Auditors: @darkdh

Bookmark manager no longer opens multiple tabs when double clicked

bc63ccd

Fixes #4523 Also includes: Update color of about:bookmarks from "been gone hunting" green to orange

bsclifton added the feature/bookmarks label Oct 11, 2016

bsclifton added this to the 0.12.5dev milestone Oct 11, 2016

bsclifton self-assigned this Oct 11, 2016

bsclifton added 2 commits October 11, 2016 00:31

The refactoring done in #4361 unfortunately broke autofill.

41a1087

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

Reworking about:bookmarks after feedback from Brad. Still some more t…

3b730b9

…o do.

bbondy reviewed Oct 11, 2016

View reviewed changes

bbondy merged commit c1fffde into brave:master Oct 11, 2016

bsclifton mentioned this pull request Oct 11, 2016

Fix autofill regression caused by refactor #4669

Closed

bsclifton deleted the bookmark-manager-style-update branch October 11, 2016 14:56

darkdh reviewed Oct 11, 2016

View reviewed changes

bsclifton changed the title ~~WIP: Bookmark manager style update~~ Bookmark manager style update Oct 11, 2016

bsclifton added the QA/test-plan-specified label Oct 11, 2016

bsclifton mentioned this pull request Oct 11, 2016

Nested bookmark folders go off screen on about:bookmarks #1187

Closed

This was referenced Oct 12, 2016

Manual tests for Windows ia-32 0.12.5 Preview 1 #4704

Closed

Manual tests for Windows x64 0.12.5 Preview 1 #4705

Closed

Manual tests for Ubuntu 0.12.5 Preview 1 #4706

Closed

Manual tests for OS X 0.12.5 Preview 1 #4707

Closed

srirambv added the QA/checked-Win64 label Oct 12, 2016

alexwykoff added the QA/checked-macOS label Oct 13, 2016

This was referenced Oct 13, 2016

Manual tests for Ubuntu 0.12.5 Preview 2 #4734

Closed

Manual tests for OS X 0.12.5 Preview 2 #4735

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bookmark manager style update #4658

Bookmark manager style update #4658

bsclifton commented Oct 11, 2016 •

edited

Loading

bsclifton commented Oct 11, 2016 •

edited

Loading

bbondy Oct 11, 2016

bbondy commented Oct 11, 2016

bradleyrichter commented Oct 11, 2016

darkdh Oct 11, 2016

darkdh Oct 11, 2016

bsclifton commented Oct 11, 2016

		@@ -891,7 +891,7 @@ See `eventStore.js` for an example use-case



		### onMouseOverBookmarkFolder(folderId, bookmarks, bookmark, xPos, yPos)
		### setBookmarksToolbarSelectedFolderId(folderId)

Bookmark manager style update #4658

Bookmark manager style update #4658

Conversation

bsclifton commented Oct 11, 2016 • edited Loading

Completely updated styles

Test Plan:

bsclifton commented Oct 11, 2016 • edited Loading

bbondy Oct 11, 2016

Choose a reason for hiding this comment

bbondy commented Oct 11, 2016

bradleyrichter commented Oct 11, 2016

darkdh Oct 11, 2016

Choose a reason for hiding this comment

darkdh Oct 11, 2016

Choose a reason for hiding this comment

bsclifton commented Oct 11, 2016

bsclifton commented Oct 11, 2016 •

edited

Loading

bsclifton commented Oct 11, 2016 •

edited

Loading