MD Bookmarks: Extract bookmark editing into a <bookmark-edit-dialog> element

chrome/browser/resources/md_bookmarks/edit_dialog.js

Index: chrome/browser/resources/md_bookmarks/edit_dialog.js
diff --git a/chrome/browser/resources/md_bookmarks/edit_dialog.js b/chrome/browser/resources/md_bookmarks/edit_dialog.js
new file mode 100644
index 0000000000000000000000000000000000000000..05c9c9c954249d484e0d355551dd997286aef0df
--- /dev/null
+++ b/chrome/browser/resources/md_bookmarks/edit_dialog.js
@@ -0,0 +1,59 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.

[calamity, 2017/03/20 05:04:50]

Sooo last year.

[tsergeant, 2017/03/20 06:03:18]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+Polymer({
+  is: 'bookmarks-edit-dialog',
+
+  properties: {
+    /** @private {BookmarkNode} */
+    editItem_: Object,
+
+    /** @private */
+    isFolder_: Boolean,
+
+    /** @private */
+    titleValue_: String,
+
+    /** @private */
+    urlValue_: String,
+  },
+
+  /** @param {BookmarkNode} editItem */
+  editNode: function(editItem) {

[calamity, 2017/03/20 05:04:50]

I think I'd prefer showForNode. It's not obvious that editNode does a UI thing.

[tsergeant, 2017/03/20 06:03:18]

The difficulty with naming this is that there'll eventually be a way to add a
new bookmark using this element. The intention was that there'd be an addNode()
method to do that.

You're right, though, that this doesn't come across as interactive. I've changed
it to 'showEditDialog' (so we can have 'showAddDialog' later).

WDYT?

[calamity, 2017/03/22 00:36:50]

Oh, I didn't realize this would be getting the add functionality as well. Are we
keeping the strings for Cancel and Save? Is it worth changing the name of this
to add_edit_dialog? Can we change onSaveEditTap_ to onSaveButtonTap_?

[tsergeant, 2017/03/22 00:53:22]

-> Cancel and Save are the button names for the native Add dialog, so I think we
can keep them
-> I wanted to keep the element name short-ish, so I'd prefer not to have both
verbs. Happy to revisit during the CL for Add
-> Done, renamed button event handlers.

+    this.editItem_ = editItem;
+    this.isFolder_ = !editItem.url;
+
+    this.titleValue_ = editItem.title;
+    if (!this.isFolder_)
+      this.urlValue_ = assert(editItem.url);
+
+    this.$.dialog.showModal();
+  },
+
+  /**
+   * @param {boolean} isFolder
+   * @return {string}
+   * @private
+   */
+  getDialogTitle_: function(isFolder) {
+    return loadTimeData.getString(
+        isFolder ? 'renameFolderTitle' : 'editBookmarkTitle');
+  },
+
+  /** @private */
+  onSaveEditTap_: function() {
+    // TODO(tsergeant): Verify values.
+    var edit = {'title': this.titleValue_};
+    if (!this.isFolder_)
+      edit['url'] = this.urlValue_;
+
+    chrome.bookmarks.update(this.editItem_.id, edit);
+    this.$.dialog.close();
+  },
+
+  /** @private */
+  onCancelEditTap_: function() {
+    this.$.dialog.cancel();
+  },

[calamity, 2017/03/20 05:04:50]

Can has Press enter to save?

[tsergeant, 2017/03/20 06:03:18]

Yup, this constantly annoys me.

I've added a TODO, so we can keep this CL to a fairly straightforward move with
no behavior changes.

[calamity, 2017/03/22 00:36:50]

Acknowledged.

+});