MD Bookmarks: Pull context menu into separate element

chrome/browser/resources/md_bookmarks/command_manager.html

Index: chrome/browser/resources/md_bookmarks/command_manager.html
diff --git a/chrome/browser/resources/md_bookmarks/command_manager.html b/chrome/browser/resources/md_bookmarks/command_manager.html
new file mode 100644
index 0000000000000000000000000000000000000000..bc1e5737e4ee4c59436a1ac8259f36d4f24bd006
--- /dev/null
+++ b/chrome/browser/resources/md_bookmarks/command_manager.html
@@ -0,0 +1,37 @@
+<link rel="import" href="chrome://resources/html/polymer.html">
+<link rel="import" href="chrome://resources/cr_elements/cr_action_menu/cr_action_menu.html">
+<link rel="import" href="chrome://resources/cr_elements/cr_lazy_render/cr_lazy_render.html">
+<link rel="import" href="chrome://bookmarks/edit_dialog.html">
+<link rel="import" href="chrome://bookmarks/shared_style.html">
+<link rel="import" href="chrome://bookmarks/store_client.html">
+
+<dom-module id="bookmarks-command-manager">
+  <template>
+    <style include="shared-style">
+    </style>

[calamity, 2017/04/24 05:20:35]

nit: Unnecessary?

[tsergeant, 2017/04/24 07:28:23]

Oops, done.

+    <dialog is="cr-action-menu" id="dropdown" on-mousedown="onMenuMousedown_">
+      <button class="dropdown-item"
+          command="edit"
+          hidden$="[[!canEdit(menuIds_)]]"
+          on-tap="onCommandClick_">
+        [[getEditActionLabel_(menuIds_)]]
+      </button>
+      <button class="dropdown-item"
+          command="copy"
+          hidden$="[[!canCopy(menuIds_)]]"

[calamity, 2017/04/24 05:20:35]

I'm conflicted about whether I want this to be canCopy or canExecute('copy').
When keyboard shortcuts come in, it'll need to be able to ask if certain things
are do-able, so it might be nicer to have something generic? Dunno, we can cross
that bridge when we get to it I suppose.

[tsergeant, 2017/04/24 07:28:23]

I actually used to have it like canExecute('copy'). I'm not too attached to
either method, but I think I find this one nicer to read.

My intention for keyboard shortcuts is that there'll be a list of registered
commands with their canExecute/handle functions which we can use to figure that
stuff out.

I've sent you a short design doc I was working on for this -- feel free to make
suggestions about the design there.

[calamity, 2017/04/26 03:43:44]

Yeah, just saw it this morning. Sorry.

+          on-tap="onCommandClick_">
+        $i18n{menuCopyURL}
+      </button>
+      <button class="dropdown-item"
+          command="delete"
+          hidden$="[[!canDelete(menuIds_)]]"
+          on-tap="onCommandClick_">
+        $i18n{menuDelete}
+      </button>

[calamity, 2017/04/24 05:20:35]

I mean, looking at this, there's some part of me that wants to believe in a
dom-repeat solution, where given a bunch of commands to put in the menu, the
dropdown menu would generate and correctly figure out what to hide and show at
any given time.

This isn't terrible though, and is probably much simpler.

[tsergeant, 2017/04/24 07:28:23]

The biggest thing that gets in the way of the dom-repeat solution is that we're
going to have a <hr> separator in the middle of the context menu. Doing it this
way is much simpler, even though it's pretty verbose.

[calamity, 2017/04/26 03:43:44]

Fair enough.

+    </dialog>
+    <template is="cr-lazy-render" id="editDialog">
+      <bookmarks-edit-dialog></bookmarks-edit-dialog>
+    </template>
+  </template>
+  <script src="chrome://bookmarks/command_manager.js"></script>
+</dom-module>