Views: Implement keyboard shortcuts for the remove and edit button in the bookmark bubble.

Views: Implement keyboard shortcuts for the remove and edit button in the bookmark bubble.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287706

[tfarina, 2014-08-03 22:20:26]

Why do we want to do this? I remember removing something like that from other
dialog I think long time ago.

+msw just in case.

[Lei Zhang, 2014-08-04 04:14:04]

On 2014/08/03 22:20:26, tfarina wrote:
> Why do we want to do this? I remember removing something like that from other
> dialog I think long time ago.
> 
> +msw just in case.

Because I like to be able to press: ctrl+d, alt+r without ever touching the
keyboard.

[sky, 2014-08-04 17:12:03]

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
File chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc (right):

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc:129: if (key_code ==
ui::VKEY_RETURN) {
I think we should keep return as we had it.

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc:132: } else if
(key_code == ui::VKEY_ESCAPE) {
nit: no else after a return (here and your new code).

[Lei Zhang, 2014-08-04 18:59:57]

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
File chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc (right):

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc:129: if (key_code ==
ui::VKEY_RETURN) {
On 2014/08/04 17:12:03, sky wrote:
> I think we should keep return as we had it.

The "if (edit_button_->HasFocus())" block doesn't seem to do get reached. Even
if I remove it, tabbing to the edit button and pressing return still works.

[Lei Zhang, 2014-08-04 19:05:55]

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
File chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc (right):

https://codereview.chromium.org/441593002/diff/1/chrome/browser/ui/views/book...
chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc:132: } else if
(key_code == ui::VKEY_ESCAPE) {
On 2014/08/04 17:12:03, sky wrote:
> nit: no else after a return (here and your new code).

Done.

[sky, 2014-08-04 20:22:05]

Ok, LGTM

[Lei Zhang, 2014-08-05 23:04:31]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-05 23:06:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/441593002/20001

[commit-bot: I haz the power, 2014-08-06 04:05:20]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-06 06:27:13]

Change committed as 287706