[MD Bookmarks] Edit Bookmarks.

[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

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

[jiaxi, 2017-01-06 05:43:25]

Description was changed from

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks.

BUG=
==========

to

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks.

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

[jiaxi, 2017-01-06 05:43:49]

Description was changed from

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks.

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

to

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks.

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

[jiaxi, 2017-01-06 06:03:52]

jiaxi@google.com changed reviewers:
+ angelayang@google.com, calamity@chromium.org, tsergeant@chromium.org

[jiaxi, 2017-01-06 06:03:53]

https://codereview.chromium.org/2617943002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/list.html (right):

https://codereview.chromium.org/2617943002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/list.html:18: 
According to Tim's comment:  `the cr-dialog and action-button styles are shared
in many different places.`

So I think for now we don't have to touch it? @Chris

https://codereview.chromium.org/2617943002/diff/1/chrome/test/data/webui/md_b...
File chrome/test/data/webui/md_bookmarks/store_test.js (right):

https://codereview.chromium.org/2617943002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/store_test.js:191: });
As discussed offline, integration test with something mocks the reactions of API
will be in another patch.

[tsergeant, 2017-01-09 00:21:05]

Cool! I like how easily the store part of this works.

https://codereview.chromium.org/2617943002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/list.html (right):

https://codereview.chromium.org/2617943002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/list.html:18: 
On 2017/01/06 06:03:53, jiaxi wrote:
> According to Tim's comment:  `the cr-dialog and action-button styles are
shared
> in many different places.`
> 
> So I think for now we don't have to touch it? @Chris

What you've got now looks right to me.

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/list.js (right):

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/list.js:33: onEditTap_: function() {
I think you should probably assert or if-guard to check that menuItem_ is not a
folder.

If you select edit bookmark on a folder, you can set a URL on it, which goofs up
the UI in weird ways.

Obviously, you'll change that by changing the menu for folders, but it's still
good to be defensive.

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/store.js (right):

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/store.js:64: // bookmarks-store, private:
👍 I like this lots

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/store.js:150: 
Just one blank line here

[jiaxi, 2017-01-09 01:59:28]

Patchset #3 (id:40001) has been deleted

[jiaxi, 2017-01-09 02:17:46]

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/list.js (right):

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/list.js:33: onEditTap_: function() {
On 2017/01/09 00:21:04, tsergeant wrote:
> I think you should probably assert or if-guard to check that menuItem_ is not
a
> folder.
> 
> If you select edit bookmark on a folder, you can set a URL on it, which goofs
up
> the UI in weird ways.
> 
> Obviously, you'll change that by changing the menu for folders, but it's still
> good to be defensive.

Done.

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/store.js (right):

https://codereview.chromium.org/2617943002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/store.js:150: 
On 2017/01/09 00:21:04, tsergeant wrote:
> Just one blank line here

Done.

[calamity, 2017-01-09 03:19:11]

Looks great!

#nittinfromnippon

https://codereview.chromium.org/2617943002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/list.html (right):

https://codereview.chromium.org/2617943002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/list.html:19: #bookmarksCard {
Why did this change?

[jiaxi, 2017-01-09 03:23:13]

https://codereview.chromium.org/2617943002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/list.html (right):

https://codereview.chromium.org/2617943002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/list.html:19: #bookmarksCard {
On 2017/01/09 03:19:11, calamity wrote:
> Why did this change?

Hi Chris, how's the holiday? 

Style guide is to use camelCase for ids.

[tsergeant, 2017-01-09 03:27:33]

https://codereview.chromium.org/2617943002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/list.html (right):

https://codereview.chromium.org/2617943002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/list.html:19: #bookmarksCard {
On 2017/01/09 03:23:13, jiaxi wrote:
> On 2017/01/09 03:19:11, calamity wrote:
> > Why did this change?
> 
> Hi Chris, how's the holiday? 
> 
> Style guide is to use camelCase for ids.

Style guide actually says to use either, but I think camelCase is preferred for
Polymer. Let's be consistent with camelCase in bookmarks.

[tsergeant, 2017-01-09 03:29:15]

lgtm

[jiaxi, 2017-01-09 03:39:46]

Description was changed from

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks.

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

to

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

[jiaxi, 2017-01-09 03:40:42]

Description was changed from

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

to

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

[jiaxi, 2017-01-09 05:59:42]

jiaxi@google.com changed reviewers:
+ blundell@chromium.org

[jiaxi, 2017-01-09 05:59:42]

+ blundell@chromium.org for the small string change in
components/bookmark_component_strings.grdp

[blundell, 2017-01-09 12:32:43]

lgtm

[jiaxi, 2017-01-09 23:02:46]

The CQ bit was checked by jiaxi@google.com

[jiaxi, 2017-01-09 23:02:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org
Link to the patchset: https://codereview.chromium.org/2617943002/#ps100001
(title: "merge")

[commit-bot: I haz the power, 2017-01-09 23:03:59]

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-01-10 00:24:28]

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

[commit-bot: I haz the power, 2017-01-10 00:24:28]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jiaxi, 2017-01-10 00:39:54]

The CQ bit was checked by jiaxi@google.com

[jiaxi, 2017-01-10 00:39:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2617943002/#ps120001
(title: "test fix")

[commit-bot: I haz the power, 2017-01-10 00:40:41]

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-01-10 02:43:42]

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

[commit-bot: I haz the power, 2017-01-10 02:43:43]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)

[tsergeant, 2017-01-10 02:54:58]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2017-01-10 02:55:57]

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-01-10 04:37:50]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1484016898029070,
"parent_rev": "75219898c08d3019046d34d47150277d484f9bd4", "commit_rev":
"f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa"}

[commit-bot: I haz the power, 2017-01-10 04:38:19]

Description was changed from

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

to

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

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

[commit-bot: I haz the power, 2017-01-10 04:38:20]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/f0fbe3213ad51e6d6404c10f6de9...

[jiaxi, 2017-01-10 23:04:30]

Description was changed from

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

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

to

==========
[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

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

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