MD Bookmarks: Add full support for cut/copy/paste keyboard shortcuts

MD Bookmarks: Add full support for cut/copy/paste keyboard shortcuts

This adds keyboard shortcuts for Cut/Paste, and expands the keyboard
shortcut for Copy to work with any selection (not just a single
bookmark).

Copying a bookmark shows a toast indicating how many items were copied,
since there is no other visual indicator that the copy was successful.

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

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

[tsergeant, 2017-06-21 07:38:58]

Description was changed from

==========
MD Bookmarks: Add full support for cut/copy/paste keyboard shortcuts

This adds keyboard shortcuts for Cut/Paste, and expands the keyboard
shortcut for Copy to work with any selection (not just a single
bookmark).

Copying a bookmark shows a toast indicating how many items were copied,
since there is no other visual indicator that the copy was successful.

BUG=729858
==========

to

==========
MD Bookmarks: Add full support for cut/copy/paste keyboard shortcuts

This adds keyboard shortcuts for Cut/Paste, and expands the keyboard
shortcut for Copy to work with any selection (not just a single
bookmark).

Copying a bookmark shows a toast indicating how many items were copied,
since there is no other visual indicator that the copy was successful.

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

[tsergeant, 2017-06-21 07:43:13]

Patchset #1 (id:1) has been deleted

[tsergeant, 2017-06-21 07:43:25]

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

[commit-bot: I haz the power, 2017-06-21 07:43:41]

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-06-21 09:21:07]

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

[commit-bot: I haz the power, 2017-06-21 09:21:08]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-06-28 00:14:51]

Patchset #2 (id:40001) has been deleted

[tsergeant, 2017-06-28 00:15:02]

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

[commit-bot: I haz the power, 2017-06-28 00:15:50]

Dry run: CQ is trying da patch.

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

[tsergeant, 2017-06-28 00:21:40]

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

[tsergeant, 2017-06-28 00:21:41]

oops, forgot to mail this last week.

For the moment, this adds a toast for Ctrl-C only. We can revisit whether we
also want a toast for Ctrl-X and Ctrl-V later.

[commit-bot: I haz the power, 2017-06-28 02:10:41]

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

[commit-bot: I haz the power, 2017-06-28 02:10:42]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-07-03 03:40:05]

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:300:
chrome.bookmarkManagerPrivate.cut(idList);
nit: inline var.

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:306: selectedFolder,
Array.from(selectedItems));
We should scroll to the pasted node here.

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/toast_manager.html (right):

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/toast_manager.html:28: flex: 1;
I thought we got rid of this?

[tsergeant, 2017-07-03 05:28:29]

I also discovered a little bug with the toast layout which wasn't visible until
now that we've got a long toast message with no undo button.

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:300:
chrome.bookmarkManagerPrivate.cut(idList);
On 2017/07/03 03:40:05, calamity wrote:
> nit: inline var.

Done.

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:306: selectedFolder,
Array.from(selectedItems));
On 2017/07/03 03:40:05, calamity wrote:
> We should scroll to the pasted node here.

This little comment reveals a slightly unkempt yak.

We actually have the same problem when adding a new item -- it can be created
offscreen without any indication of success.

The old BMM has a nifty system (the relevant function is
"selectItemsAfterUserAction") for selecting and scrolling after anything which
can add new items to the list (paste, create, drop).

Do you mind if I file a bug and follow-up separately on implementing something
like that for MD Bookmarks? Solving this problem well might be a reasonably
sized task by itself.

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/toast_manager.html (right):

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/toast_manager.html:28: flex: 1;
On 2017/07/03 03:40:05, calamity wrote:
> I thought we got rid of this?

oops, merge flub

[calamity, 2017-07-03 06:45:34]

lgtm. Shave at your leisure.

[tsergeant, 2017-07-04 01:32:11]

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2948943002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:306: selectedFolder,
Array.from(selectedItems));
On 2017/07/03 05:28:29, tsergeant wrote:
> On 2017/07/03 03:40:05, calamity wrote:
> > We should scroll to the pasted node here.
> 
> This little comment reveals a slightly unkempt yak.
> 
> We actually have the same problem when adding a new item -- it can be created
> offscreen without any indication of success.
> 
> The old BMM has a nifty system (the relevant function is
> "selectItemsAfterUserAction") for selecting and scrolling after anything which
> can add new items to the list (paste, create, drop).
> 
> Do you mind if I file a bug and follow-up separately on implementing something
> like that for MD Bookmarks? Solving this problem well might be a reasonably
> sized task by itself.

Filed https://crbug.com/738958

[tsergeant, 2017-07-04 01:32:16]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2017-07-04 01:32:21]

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-07-04 02:59:24]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1499131936159550,
"parent_rev": "9875876f9414aa1cf1db2629ce3d79263c129e1c", "commit_rev":
"b7253e9c12c66f12bf67f95be8c6aeea808dce12"}

[commit-bot: I haz the power, 2017-07-04 02:59:38]

Description was changed from

==========
MD Bookmarks: Add full support for cut/copy/paste keyboard shortcuts

This adds keyboard shortcuts for Cut/Paste, and expands the keyboard
shortcut for Copy to work with any selection (not just a single
bookmark).

Copying a bookmark shows a toast indicating how many items were copied,
since there is no other visual indicator that the copy was successful.

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

to

==========
MD Bookmarks: Add full support for cut/copy/paste keyboard shortcuts

This adds keyboard shortcuts for Cut/Paste, and expands the keyboard
shortcut for Copy to work with any selection (not just a single
bookmark).

Copying a bookmark shows a toast indicating how many items were copied,
since there is no other visual indicator that the copy was successful.

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

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

[commit-bot: I haz the power, 2017-07-04 02:59:39]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/b7253e9c12c66f12bf67f95be8c6...