MD Bookmarks: Focus sidebar nodes on click without showing an outline

MD Bookmarks: Focus sidebar nodes on click without showing an outline

Previously, clicking on a node in the MD Bookmarks sidebar would select
that folder without actually focusing the element. It was necessary to
click a second time to actually give focus and allow keyboard navigation.

This CL fixes this by pulling out the code from <bookmarks-item> that
prevents outlines from showing on click into a shared behavior.
Implementing this behavior improves keyboard navigation in the sidebar
without affecting mouse-only users.

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

Review-Url: https://codereview.chromium.org/2920413002
Cr-Commit-Position: refs/heads/master@{#477916}
Committed: https://chromium.googlesource.com/chromium/src/+/3c550cc85921ba59fdb1ef2655a8bd861f351174

[tsergeant, 2017-06-06 03:17:25]

Description was changed from

==========
MD Bookmarks: Focus sidebar nodes on click without showing an outline

Previously, clicking on a node in the MD Bookmarks sidebar would select
that folder without actually focusing the element. It was necessary to
click a second time to actually give focus and allow keyboard navigation.

This CL fixes this by pulling out the code from <bookmarks-item> that
prevents outlines from showing on click into a shared behavior.
Implementing this behavior improves keyboard navigation in the sidebar
without affecting mouse-only users.

BUG=692844
==========

to

==========
MD Bookmarks: Focus sidebar nodes on click without showing an outline

Previously, clicking on a node in the MD Bookmarks sidebar would select
that folder without actually focusing the element. It was necessary to
click a second time to actually give focus and allow keyboard navigation.

This CL fixes this by pulling out the code from <bookmarks-item> that
prevents outlines from showing on click into a shared behavior.
Implementing this behavior improves keyboard navigation in the sidebar
without affecting mouse-only users.

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

[tsergeant, 2017-06-06 03:30:00]

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

[tsergeant, 2017-06-06 03:30:10]

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

[commit-bot: I haz the power, 2017-06-06 03:30:47]

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-06 05:21:38]

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

[commit-bot: I haz the power, 2017-06-06 05:21:40]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-06-08 03:34:19]

lg % a question.

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

https://codereview.chromium.org/2920413002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/folder_node.js:73: /** @return
{!HTMLElement} */
@override

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

https://codereview.chromium.org/2920413002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js:1: // Copyright
2016 The Chromium Authors. All rights reserved.
7

https://codereview.chromium.org/2920413002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js:19:
target.addEventListener('blur', this.boundClearMouseFocus_, true);
Why is capture true here?

[tsergeant, 2017-06-08 05:03:19]

Patchset #3 (id:40001) has been deleted

[tsergeant, 2017-06-08 05:03:40]

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

https://codereview.chromium.org/2920413002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/folder_node.js:73: /** @return
{!HTMLElement} */
On 2017/06/08 03:34:18, calamity wrote:
> @override

Done.

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

https://codereview.chromium.org/2920413002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js:1: // Copyright
2016 The Chromium Authors. All rights reserved.
On 2017/06/08 03:34:19, calamity wrote:
> 7

ooooops

https://codereview.chromium.org/2920413002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js:19:
target.addEventListener('blur', this.boundClearMouseFocus_, true);
On 2017/06/08 03:34:19, calamity wrote:
> Why is capture true here?

Ah, this was leftover from experimenting with trying to remove the extra call to
clearMouseFocus() from <bookmarks-item>. It's unnecessary now, removed.

[calamity, 2017-06-08 05:07:11]

lgtm

https://codereview.chromium.org/2920413002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/folder_node.js (right):

https://codereview.chromium.org/2920413002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/folder_node.js:75: * @private
lol. @private != @override

[tsergeant, 2017-06-08 05:16:30]

https://codereview.chromium.org/2920413002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/folder_node.js (right):

https://codereview.chromium.org/2920413002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/folder_node.js:75: * @private
On 2017/06/08 05:07:11, calamity wrote:
> lol. @private != @override

Oh geez, my brain is running on autopilot a little bit. This is one of the cases
where override doesn't work because a behavior is not actual inheritance, but
I've added a comment for it.

[tsergeant, 2017-06-08 05:23:00]

Patchset #5 (id:100001) has been deleted

[tsergeant, 2017-06-08 05:23:10]

The CQ bit was checked by tsergeant@chromium.org

[tsergeant, 2017-06-08 05:23:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
calamity@chromium.org
Link to the patchset: https://codereview.chromium.org/2920413002/#ps120001
(title: "Fix override comment")

[commit-bot: I haz the power, 2017-06-08 05:23:27]

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-08 06:43:03]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1496899390560000,
"parent_rev": "d18e61ab5e01b85ff9160c4ab143cba6cd64d7be", "commit_rev":
"3c550cc85921ba59fdb1ef2655a8bd861f351174"}

[commit-bot: I haz the power, 2017-06-08 06:43:14]

Description was changed from

==========
MD Bookmarks: Focus sidebar nodes on click without showing an outline

Previously, clicking on a node in the MD Bookmarks sidebar would select
that folder without actually focusing the element. It was necessary to
click a second time to actually give focus and allow keyboard navigation.

This CL fixes this by pulling out the code from <bookmarks-item> that
prevents outlines from showing on click into a shared behavior.
Implementing this behavior improves keyboard navigation in the sidebar
without affecting mouse-only users.

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

to

==========
MD Bookmarks: Focus sidebar nodes on click without showing an outline

Previously, clicking on a node in the MD Bookmarks sidebar would select
that folder without actually focusing the element. It was necessary to
click a second time to actually give focus and allow keyboard navigation.

This CL fixes this by pulling out the code from <bookmarks-item> that
prevents outlines from showing on click into a shared behavior.
Implementing this behavior improves keyboard navigation in the sidebar
without affecting mouse-only users.

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

Review-Url: https://codereview.chromium.org/2920413002
Cr-Commit-Position: refs/heads/master@{#477916}
Committed:
https://chromium.googlesource.com/chromium/src/+/3c550cc85921ba59fdb1ef2655a8...
==========

[commit-bot: I haz the power, 2017-06-08 06:43:16]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/3c550cc85921ba59fdb1ef2655a8...