Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce "delegatesFocus" flag to shadow root #126

Closed
TakayoshiKochi opened this issue Jun 30, 2015 · 16 comments
Closed

Introduce "delegatesFocus" flag to shadow root #126

TakayoshiKochi opened this issue Jun 30, 2015 · 16 comments

Comments

@TakayoshiKochi
Copy link
Member

To fix focusability and focus navigation order issue (W3C 25473, Chromium 380445), introduce delegatesFocus flag under shadow root, to address some shortcomings that
the default focus navigation behavior (defined in the Shadow DOM spec) has.

Detailed doc is here:
https://github.com/TakayoshiKochi/tabindex-focus-navigation-explainer/blob/master/TabindexFocusNavigationExplainer.md

I'll post the proposal doc soon.
This is experimentally implmented in Blink Chromium 496005

@rniwa
Copy link
Collaborator

rniwa commented Jul 11, 2015

Why can't we always use autofocus attribute to infer it? That is, behave as if delegatesFocus as true if and only if the shadow DOM contains an element with autofocus attribute but don't expose delegatesFocus.

@rniwa
Copy link
Collaborator

rniwa commented Jul 11, 2015

@annevk

@annevk
Copy link
Collaborator

annevk commented Jul 12, 2015

I don't see how that solves the problem of skipping the host element adequately.

@rniwa
Copy link
Collaborator

rniwa commented Jul 12, 2015

I'm saying that skip the host element if its shadow DOM contains an element with autofocus attribute.

@annevk
Copy link
Collaborator

annevk commented Jul 12, 2015

But if it only contains a single <input> you would not skip it? Why would a component always want to use autofocus?

@TakayoshiKochi
Copy link
Member Author

Let me summarize the status of this issue to get the ball rolling again.

  1. @rniwa's question about autofocus (Introduce "delegatesFocus" flag to shadow root #126) - as @annevk already replied, why autofocus would suffice for what delegatesFocus tries to solve is unclear - @rniwa, could you add more comment on this?
  2. ([Shadow] Focus on shadow host should slide to its inner focusable node (delegatesFocus) #105) - about auto focus-sliding into shadow tree behavior. I think my use case is understood by @annevk but @rniwa's comment that delegatesFocus is unnecessary isn't well understood by me or @annevk. @rniwa, could you add more comment?
  3. (delegatesFocus #276) - focus processing model is still unclear - I will have to come up with more clear model of focus processing as a patch to current HTML spec, am I understanding your concern right? @annevk

@annevk
Copy link
Collaborator

annevk commented Jul 28, 2015

Yes, I would like to see how shadow DOM affects the focus model we have today (and even that likely has holes still). After that is explained it would be easier to reason about any changes to it.

@hayatoito
Copy link
Contributor

@TakayoshiKochi , could you summarize the status of this issue? This is a blocking issue for v1.

@rniwa
Copy link
Collaborator

rniwa commented Feb 17, 2016

Never mind about my proposal. I was misunderstanding the problem at the time.

Now that I understand the problem better, I think it's fine to have a flag. If we're going to have setForceFocusable though, we should probably call this flag focusable. Flipping the boolean in the init dictionary and the method argument is obnoxious.

@rniwa
Copy link
Collaborator

rniwa commented Feb 17, 2016

Also, do we really need setForceFocusable? What are use cases in which whether the shadow root should/can be focused change dynamically?

@TakayoshiKochi
Copy link
Member Author

The focusable or setForceFocusable were introduced to the document trying to explain some of the
native elements via shadow DOM.

For example <a name=...> isn't focusable but <a href=...> is focusable, without having to add/remove tabindex attribute in HTML. So it could be used for emulating such behavior, but I don't think it is a viable use case, and we can omit it at least for v1.

@TakayoshiKochi
Copy link
Member Author

Thus, the changes I would like to introduce is basically listed here,
by giving delegatesFocus: true in attachShadow() will affect:

  1. if shadow host has 0 or positive tabindex attribute value, the sequential navigation order honors that value, but actual focus goes into an element in its shadow (depending on navigation order, first or last focusable element in shadow will be picked)
  2. focus() on a shadow host will also delegate focus to its shadow
  3. autofocus attribute on a shadow host will also delegate focus to its shadow
  4. If non-focusable area in shadow is clicked, shadow host gets focus but then delegates focus to its shadow
  5. CSS :focus pseudo class on shadow host should match when its shadow or its descendant has focus

@rniwa @travisleithead @annevk any opinion for each item above?

@annevk
Copy link
Collaborator

annevk commented Feb 24, 2016

I think 5 should also be recursive, no?

Will we do something similar for slots?

@TakayoshiKochi
Copy link
Member Author

Re :focus matching, it should be recursive. e.g. in the following example,
div#host1:focus should match when either input#outer or input#inner is focused.
Of course div#host2:focus only matches when input#inner is focused.

<div#host1>
  <:shadow>
     <div#host2>
        <:shadow>
           <input#inner>
        </:shadow>
    <input#outer>
  </:shadow>
</div>

Because a slot is not focusable, I don't think identical mechanism is necessary, but regarding to :focus matching I think it depends on the result of how we treat focus in slot for .activeElement (#358).

@rniwa
Copy link
Collaborator

rniwa commented Feb 26, 2016

Let's not block this issue on the issue #358 or the issue #308. I think the proposal in #126 (comment) is sound.

@TakayoshiKochi
Copy link
Member Author

I start working on updating the specs.

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

No branches or pull requests

4 participants