[MD Bookmarks] Remove the bookmarks-sidebar element.

[MD Bookmarks] Remove the bookmarks-sidebar element.

This CL removes the bookmarks-sidebar element in and replaces it with
a root bookmarks-folder-node. This serves to specify the recursive nature
of the sidebar, and will allow for neater recursive behaviors such as
keyboard navigation.

BUG=692844
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2823053003
Cr-Commit-Position: refs/heads/master@{#467567}
Committed: https://chromium.googlesource.com/chromium/src/+/a5561b2dc4119384f07627fe976badda532d3fc5

[calamity, 2017-04-18 08:19:51]

Description was changed from

==========
[MD Bookmarks] Remove the bookmarks-sidebar element.

This CL removes the bookmarks-sidebar element in and replaces it with
a root bookmarks-folder-node. This serves to specify the recursive nature
of the sidebar, and will allow for neater recursive behaviors such as
keyboard navigation.

BUG=692844
==========

to

==========
[MD Bookmarks] Remove the bookmarks-sidebar element.

This CL removes the bookmarks-sidebar element in and replaces it with
a root bookmarks-folder-node. This serves to specify the recursive nature
of the sidebar, and will allow for neater recursive behaviors such as
keyboard navigation.

BUG=692844
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[calamity, 2017-04-18 08:28:45]

calamity@chromium.org changed reviewers:
+ tsergeant@chromium.org

[calamity, 2017-04-18 08:28:46]

This is a precursor to the other CL. Basically, you'll probably need to read
that before you decide if this is a good idea. Towards the end of the other CL,
I realized I had to add a bit of cruft to app.js, and sidebar.js might be a good
place to keep that?

[tsergeant, 2017-04-19 07:39:28]

Happy to go with this now that the extra cruft is gone from app.js in the
follow-up.

https://codereview.chromium.org/2823053003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/folder_node.html (right):

https://codereview.chromium.org/2823053003/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/folder_node.html:84:
draggable="[[!isRootFolder_(depth)]]"
Nit: Maybe we should name these a bit more consistently with the rest of the app
(which refers to the root folder as something else)

isRootFolder could become isTopLevelFolder
and !isPositiveDepth_ could become isRootFolder (ie, depth == -1)

https://codereview.chromium.org/2823053003/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/md_bookmarks/sidebar_test.js (left):

https://codereview.chromium.org/2823053003/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/sidebar_test.js:5:
suite('<bookmarks-sidebar>', function() {
Looks like this file didn't get deleted when it should have been: both
folder_node_test.js and sidebar_test.js exist now, but only folder_node_test is
used.

[calamity, 2017-04-26 03:33:46]

The CQ bit was checked by calamity@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 03:34:04]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 05:03:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-26 05:03:51]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-04-26 08:03:54]

https://codereview.chromium.org/2823053003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/folder_node.html (right):

https://codereview.chromium.org/2823053003/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/folder_node.html:84:
draggable="[[!isRootFolder_(depth)]]"
On 2017/04/19 07:39:28, tsergeant wrote:
> Nit: Maybe we should name these a bit more consistently with the rest of the
app
> (which refers to the root folder as something else)
> 
> isRootFolder could become isTopLevelFolder
> and !isPositiveDepth_ could become isRootFolder (ie, depth == -1)

Donion rings.

https://codereview.chromium.org/2823053003/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/md_bookmarks/sidebar_test.js (left):

https://codereview.chromium.org/2823053003/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/sidebar_test.js:5:
suite('<bookmarks-sidebar>', function() {
On 2017/04/19 07:39:28, tsergeant wrote:
> Looks like this file didn't get deleted when it should have been: both
> folder_node_test.js and sidebar_test.js exist now, but only folder_node_test
is
> used.

Done.

[tsergeant, 2017-04-26 23:47:55]

lgtm

https://codereview.chromium.org/2823053003/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/md_bookmarks/sidebar_test.js (left):

https://codereview.chromium.org/2823053003/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/sidebar_test.js:5:
suite('<bookmarks-sidebar>', function() {
On 2017/04/26 08:03:54, calamity wrote:
> On 2017/04/19 07:39:28, tsergeant wrote:
> > Looks like this file didn't get deleted when it should have been: both
> > folder_node_test.js and sidebar_test.js exist now, but only folder_node_test
> is
> > used.
> 
> Done.

This file still shows up as 'M' in Rietveld instead of 'D', but it seems to
patch in correctly. Weird.

[calamity, 2017-04-27 03:21:57]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2017-04-27 03:22:28]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 03:27:20]

CQ is committing da patch.

Bot data: {"patchset_id": 30001, "attempt_start_ts": 1493263317615790,
"parent_rev": "6e5be9d31acb3b07114e88cc3d24c59e18f47be0", "commit_rev":
"a5561b2dc4119384f07627fe976badda532d3fc5"}

[commit-bot: I haz the power, 2017-04-27 03:27:56]

Description was changed from

==========
[MD Bookmarks] Remove the bookmarks-sidebar element.

This CL removes the bookmarks-sidebar element in and replaces it with
a root bookmarks-folder-node. This serves to specify the recursive nature
of the sidebar, and will allow for neater recursive behaviors such as
keyboard navigation.

BUG=692844
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD Bookmarks] Remove the bookmarks-sidebar element.

This CL removes the bookmarks-sidebar element in and replaces it with
a root bookmarks-folder-node. This serves to specify the recursive nature
of the sidebar, and will allow for neater recursive behaviors such as
keyboard navigation.

BUG=692844
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2823053003
Cr-Commit-Position: refs/heads/master@{#467567}
Committed:
https://chromium.googlesource.com/chromium/src/+/a5561b2dc4119384f07627fe976b...
==========

[commit-bot: I haz the power, 2017-04-27 03:28:02]

Committed patchset #3 (id:30001) as
https://chromium.googlesource.com/chromium/src/+/a5561b2dc4119384f07627fe976b...