Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

iron-overlay-behavior does not autofocus inputs in nested custom elements #260

Open
aarongable opened this issue Jan 31, 2018 · 3 comments

Comments

@aarongable
Copy link

aarongable commented Jan 31, 2018

Description

The iron-overlay-element by default sets focus to any child of itself which has the "autofocus" attribute. However, it does not do so for descendants which are within another custom element.

Steps to reproduce

  1. Have a custom element which contains an autofocus input (such as https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/8b6bbfc8104e5015735e36c577b072bdfbb7326d/src/main/resources/static/cr-tryjob-picker.html#65)
  2. Make that element the child of an element which has the IronOverlayBehavior (such as https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/8b6bbfc8104e5015735e36c577b072bdfbb7326d/src/main/resources/static/cr-buildbucket-view.html#84)
  3. Open, close, and then re-open the overlay

Expected outcome

Every time the overlay opens, the autofocus input element gets focus.

Actual outcome

The autofocus input element only gets focus the first time the overlay opens (due to native browser support for autofocus, not due to actions taken by iron-overlay-behavior). The element does not get focus subsequent times.

Live demo

https://crrev.com/c/868827; sign in and then click "Choose Tryjobs"

Suggested action

The following patch is a working fix:

$ diff iron-overlay-behavior.html iron-overlay-behavior.html.patched 
143c143
<       return this._focusedChild || Polymer.dom(this).querySelector('[autofocus]') || this;
---
>       return this._focusedChild || this.querySelector('[autofocus]') || this;

This causes the querySelector to crawl all descendents of the element, even those within the shadow dom of a child element, rather than only considering the custom elements themselves. However, I suspect that Polymer.dom(this) is used for a reason and this may not be the desired approach, so I haven't directly submitted a pull request.

@valdrinkoshi
Copy link
Member

Hi @aarongable, the suggested change will not work with native ShadowDOM, as this.querySelector will have visibility only on the light dom of the overlay (in your case, <gr-overlay> will see only <cr-tryjob-picker>).

You'll need to set autofocus on <cr-tryjob-picker>, make it focusable but not tabbable (set tabindex=-1), and handle focus redirection to the <input> in the shadowRoot.

@aarongable
Copy link
Author

Thanks, I was able to follow your advice and get it working with this change: https://chromium-review.googlesource.com/c/infra/gerrit-plugins/buildbucket/+/895845

I still think it would be useful for iron-overlay-behavior to be able to autofocus an element that is in the shadow dom of one of its descendants, especially for cases where the devloper is composing a lot of pre-made elements and wants to focus an input contained in (e.g.) a pre-made mailing address form element.

@valdrinkoshi
Copy link
Member

I believe iron-overlay-behavior should not pierce into children's shadowRoots, but just handle the light dom. A shadowRoot might be closed, hence iron-overlay-behavior wouldn't even be able to pierce those shadowRoots.

Declaring a shadowRoot with delegatesFocus: true addresses this issue; a custom element that delegates focus forwards the focus to its focusable content by default, and if no content is present, it keeps the focus on itself - see this explainer for more info.
This behavior is not polyfilled tho, and I believe it's currently implemented only on Chrome...

In Polymer, you can pass configuration options to attachShadow by overriding _attachDom method, see Polymer/polymer#3988 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants