WebUI: Return focus to anchor element when cr-action-menu is closed.

WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own behavior that matches user's focus
expectations, instead of just relying on <dialog>'s behavior.

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

Committed: https://crrev.com/140f168199593fafe11b6a568cdfa99b4d1c9bda
Cr-Commit-Position: refs/heads/master@{#429468}

[dpapad, 2016-10-31 21:00:46]

Description was changed from

==========
MD Settings: Return focus to anchor element when cr-action-menu is closed.

BUG=659567
==========

to

==========
MD Settings: Return focus to anchor element when cr-action-menu is closed.

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

[dpapad, 2016-10-31 21:11:28]

Description was changed from

==========
MD Settings: Return focus to anchor element when cr-action-menu is closed.

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

to

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

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

[dpapad, 2016-10-31 21:29:26]

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

[dpapad, 2016-10-31 21:29:59]

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

[dpapad, 2016-10-31 21:30:03]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-10-31 21:30:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dpapad, 2016-10-31 21:30:34]

Patchset #2 (id:40001) has been deleted

[dpapad, 2016-10-31 21:32:32]

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

[commit-bot: I haz the power, 2016-10-31 21:33:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dpapad, 2016-10-31 21:33:58]

Description was changed from

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

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

to

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own restore on focus behavior, instead
of just relying on <dialog>'s behavior.

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

[commit-bot: I haz the power, 2016-10-31 21:46:03]

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

[commit-bot: I haz the power, 2016-10-31 21:46:04]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[dpapad, 2016-10-31 23:14:37]

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

[commit-bot: I haz the power, 2016-10-31 23:15:27]

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, 2016-11-01 00:27:52]

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

[commit-bot: I haz the power, 2016-11-01 00:27:53]

Dry run: This issue passed the CQ dry run.

[dpapad, 2016-11-01 16:50:38]

Description was changed from

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own restore on focus behavior, instead
of just relying on <dialog>'s behavior.

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

to

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own restore that matches user's focus
expectations on focus behavior, instead of just relying on <dialog>'s behavior.

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

[dpapad, 2016-11-01 16:52:21]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org, michaelpg@chromium.org

[dpapad, 2016-11-01 16:52:22]

@dbeam, @michaelpg: I think this makes much more sense from the user's
perspective. See https://bugs.chromium.org/p/chromium/issues/detail?id=659567#c8
for a screencast.

[michaelpg, 2016-11-01 19:46:21]

On 2016/11/01 16:52:22, dpapad wrote:
> @dbeam, @michaelpg: I think this makes much more sense from the user's
> perspective. See
https://bugs.chromium.org/p/chromium/issues/detail?id=659567#c8
> for a screencast.

How does this work when the anchor element is removed from the DOM? Example in
chrome://md-settings/languages:

Removing an item in the middle of the list:
A1. Add French and German, with French above German.
A2. Tap action menu for French. Choose Remove.

Expected: ?

Removing an item at the end of the list:
B1. Add French and German, with French above German.
B2. Tap action menu for German. Choose Remove.

Expected: ?

Also worth testing with an <iron-list> (which languages isn't).

[dpapad, 2016-11-01 20:45:57]

On 2016/11/01 at 19:46:21, michaelpg wrote:
> On 2016/11/01 16:52:22, dpapad wrote:
> > @dbeam, @michaelpg: I think this makes much more sense from the user's
> > perspective. See
https://bugs.chromium.org/p/chromium/issues/detail?id=659567#c8
> > for a screencast.
> 
> How does this work when the anchor element is removed from the DOM? Example in
chrome://md-settings/languages:
> 
> Removing an item in the middle of the list:
> A1. Add French and German, with French above German.
> A2. Tap action menu for French. Choose Remove.
> 
> Expected: ?
Focus moves to the next item, in this case would be the '...' next to German.


> 
> Removing an item at the end of the list:
> B1. Add French and German, with French above German.
> B2. Tap action menu for German. Choose Remove.
> 
> Expected: ?
Nothing is focused. Hitting tab goes to the 1st item after the list, shift+tab
goes to the newly last '...' icon.


> 
> Also worth testing with an <iron-list> (which languages isn't).
Tried with search engines iron-list. Same behavior as above.

[michaelpg, 2016-11-01 21:30:03]

On 2016/11/01 20:45:57, dpapad wrote:
> On 2016/11/01 at 19:46:21, michaelpg wrote:
> > On 2016/11/01 16:52:22, dpapad wrote:
> > > @dbeam, @michaelpg: I think this makes much more sense from the user's
> > > perspective. See
> https://bugs.chromium.org/p/chromium/issues/detail?id=659567#c8
> > > for a screencast.
> > 
> > How does this work when the anchor element is removed from the DOM? Example
in
> chrome://md-settings/languages:
> > 
> > Removing an item in the middle of the list:
> > A1. Add French and German, with French above German.
> > A2. Tap action menu for French. Choose Remove.
> > 
> > Expected: ?
> Focus moves to the next item, in this case would be the '...' next to German.
> 
> 
> > 
> > Removing an item at the end of the list:
> > B1. Add French and German, with French above German.
> > B2. Tap action menu for German. Choose Remove.
> > 
> > Expected: ?
> Nothing is focused. Hitting tab goes to the 1st item after the list, shift+tab
> goes to the newly last '...' icon.

OK, that's what I was checking. I guess that's what happens when you call
focus() on an element that is no longer in the DOM, right? So it's okay that
anchorElement_ is now a "dangling" element (not in the dom).

> 
> 
> > 
> > Also worth testing with an <iron-list> (which languages isn't).
> Tried with search engines iron-list. Same behavior as above.

[dpapad, 2016-11-01 21:38:13]

> OK, that's what I was checking. I guess that's what happens when you call
focus() on an element that is no longer in the DOM, right? So it's okay that
anchorElement_ is now a "dangling" element (not in the dom).

Added a statement to set anchorElement_ back to null. It is always being
initialized in showAt() so there was no reason to keep the reference alive after
close().

[dpapad, 2016-11-01 21:38:50]

On 2016/11/01 at 21:38:13, dpapad wrote:
> > OK, that's what I was checking. I guess that's what happens when you call
focus() on an element that is no longer in the DOM, right? So it's okay that
anchorElement_ is now a "dangling" element (not in the dom).
> 
> Added a statement to set anchorElement_ back to null. It is always being
initialized in showAt() so there was no reason to keep the reference alive after
close().

And yes, I think the observed behavior happens because the call to focus() the
element not in the DOM is ignored.

[Dan Beam, 2016-11-01 23:03:04]

lgtm

[dpapad, 2016-11-01 23:05:50]

Description was changed from

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own restore that matches user's focus
expectations on focus behavior, instead of just relying on <dialog>'s behavior.

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

to

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own behavior that matches user's focus
expectations, instead of just relying on <dialog>'s behavior.

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

[michaelpg, 2016-11-01 23:17:15]

lgtm 2

[dpapad, 2016-11-01 23:43:33]

On 2016/11/01 at 23:17:15, michaelpg wrote:
> lgtm 2

Thanks. I have to wait for the parent CL
https://codereview.chromium.org/2461113002 to land before landing this though.

[michaelpg, 2016-11-02 01:05:18]

On 2016/11/01 23:43:33, dpapad wrote:
> On 2016/11/01 at 23:17:15, michaelpg wrote:
> > lgtm 2
> 
> Thanks. I have to wait for the parent CL
> https://codereview.chromium.org/2461113002 to land before landing this though.

that's fine. just sayin, since I was a reviewer.

[dpapad, 2016-11-02 23:05:36]

Patchset #6 (id:140001) has been deleted

[dpapad, 2016-11-02 23:06:22]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-11-02 23:06:22]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2464873003/#ps160001
(title: "Resolve conflicts.")

[commit-bot: I haz the power, 2016-11-02 23:07:14]

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, 2016-11-03 00:14:10]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-11-03 00:16:34]

Description was changed from

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own behavior that matches user's focus
expectations, instead of just relying on <dialog>'s behavior.

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

to

==========
WebUI: Return focus to anchor element when cr-action-menu is closed.

The default <dialog> restore focus behavior is not very intuitive (or clear)
and certainly does not match the user's expectation when an action menu is
closed. Action menu should define its own behavior that matches user's focus
expectations, instead of just relying on <dialog>'s behavior.

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

Committed: https://crrev.com/140f168199593fafe11b6a568cdfa99b4d1c9bda
Cr-Commit-Position: refs/heads/master@{#429468}
==========

[commit-bot: I haz the power, 2016-11-03 00:16:35]

Patchset 6 (id:??) landed as
https://crrev.com/140f168199593fafe11b6a568cdfa99b4d1c9bda
Cr-Commit-Position: refs/heads/master@{#429468}