-
Notifications
You must be signed in to change notification settings - Fork 379
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
[Shadow] activeElement behavior seems to break encapsulation #358
Comments
Thanks. Let me add an example, which came from our previous internal discussion, slightly modified so that it uses slot elements: Exampledocument tree: (Updated: I fixed all wrong indentation and missing closing tags) <body>
<focusable1></focusable1>
<x-foo>
<focusable2 slot="slot1"></focusable2> // This is assigned to a slot
<focusable3></focusable3> // This is not assigned to any slots
</x-foo>
</body> x-foo's shadow tree: <slot name="slot1"></slot> // <- focusable2 is assigned to this slot
<focusable4></focusable4> A) The behavior what the current spec says:
B) The proposed change, as far as I understand correctly:
I do not have a strong opinion. I'm fine with B. It looks very simple. Does someone have any use cases where web developers would have trouble with B?? |
I think we disused the desired behavior a couple of weeks ago with Polymer team. @TakayoshiKochi |
Here it is: For In the meantime, if any element in a shadow tree gets focused (from outside the shadow tree), its shadow host should get That way a "focus" event listener on a shadow host can notice when its shadow content gets focused and also dig into which element gets focused by using its shadow root's |
Why don't we make |
Yeah, But the point is that we want "when The second paragraph in my last comment was confusing. Let me explain more. This behavior is for consistency between Without Shadow DOM, If we put an element with shadow DOM, and some element in the shadow gets focus, |
So if we allow |
It can not be allowed. That's the bottom-line, I think. The reason is simple: a slot is not a focusable element. It never participates in the document composed tree. {Document|ShadowRoot}.activeElement should point to a real element which exits in the document composed tree. |
I think what Hayato said - that only focusable elements can be assigned to a Document / ShadowRoot's This is probably outside the scope of this particular bug but it might still be worth considering if there should be a simple(r) way to determine more information about if / where a focused element deeper in the composed tree exists from within a ShadowRoot (without giving away information external to it). To my understanding, the event propagation path makes it possible to capture focus / blur events anywhere deeper in the composed tree than the node you're listening on (even if those events' targets are reprojected from somewhere shallower in the tree-of-trees). Given that, it's kind of odd that this information is first made available within the ShadowRoot but then lost after the event has stopped propagating. But, maybe it should be lost for the same reason as this change to activeElement? |
Why?
I don't see why that's any more rational than focusing the slot for the sake of developer ergonomics. Having to listen to focus/blur events to know where the focus resides is such an annoyance. |
That sounds reasonable also. I don't have a strong opinion on this. |
On a related note, can slots be styled? If so, would this have implications as to whether or not you could use slot:focus {
color: blue;
} |
You can't style by default because it's supposed to have the default style of |
Specifically, any CSS rules that would select a slot element act as if they do not? |
Well, there is no difference in the way CSS rules are applied so you should be able to get the computed style of a slot just like any other element. |
Oh, I misunderstood your earlier comment / #308 then. So styling a slot is like any other element and the UA stylesheet in particular should contain |
In the current spec:
A good analogy for a slot is DocumentFragment, which will disappear once their role ended when the composition finished, in terms of rendering. We discussed this topic in #308 and concluded we should not consider a slot being the composed tree until v2 because there is no browser support for {display: contents} and that will not happen soon. |
We're planning to add |
Can we continue the discussion in #308? |
Even if a slot could be a focusable element, I prefer As @kochi said in #358 (comment), in terms of a document tree, that would cause two active elements in a document tree,
|
Thanks for the clarification on #308. |
Not at all. Just because That is, I'm not suggesting to focus the slot element when the slot contains a focused/active element. I'm simply suggesting to make |
@rniwa if a slot can be // some pseudo code included
focused = root.activeElement;
if (focused.tagName == 'SLOT') {
for (node in focused.getAssignedNodes()) {
if (node is really focused)
return node;
}
}
return focused; (Probably there's a better way than mine :) Or is it enough that a component knows some element under |
s/ |
Hmm, without I don't still understand what's the point of allowing a slot to be an active element. |
In many UI components, it's important to know whether something inside the component is focused or not. e.g. toolbar-pane component may need to style itself differently when a text field inside is focused. |
For styling in the toolbar-pane example, maybe using In that case you need to listen to <toolbar-pane>
<#shadow>
<button id="button">
<slot name="inputs"></slot>
</#shadow>
<input id="x" slot="inputs" button-color="#333">
<input id="y" slot="inputs" button-color="#ff0">
<input id="z" slot="inputs" button-color="#0cc">
</toolbar-pane> Let's say If you are saying that you need an API to determine if focus is inside the component, including distributed nodes in O(1) way, it might be okay. But then what is the notification mechanism that a distributed node gets focused for the component? |
My preference is option B in #358 (comment). I prefer keeping activeElement simple and understandable. I am not a fan of making it too magical one. If we find a use case where we need an intelligent magical API, such as I am afraid that it might be too early optimization to make activeElement too magical. |
Discussed in WICG#358
@annevk @travisleithead : what are you opinions here? |
It seems hard to decide this if we don't have agreement on #308. This fundamentally depends on that I think. My impression was always that slots would not be part of the composed tree. |
After thinking of #358 (comment) more, I come to think if we have I was imagining the following case, and the code is listening <my-menu>
#shadow-root
<my-item>...</my-item>
<my-item>...</my-item>
<!-- optional items go here -->
<slot></slot>
#shadow-root-end
<my-optional-item>...</my-optional-item>
<my-optional-item>...</my-optional-item>
<my-optional-item>...</my-optional-item>
</my-menu> The concern is that to implement What do you think? |
This is why I think it's better for |
This does not sound a good use case because:
BTW, my preference is still option B. If we want to give developers an ability such as ShadowRoot.magicalActiveElement => slot, we can give it as another API, so that we do not break any assumption on existing APIs. Remember that a slot is now always a focusable element, given that it does not generate CSS Box in default. We should honor that the return value from DocumetnOrShadowRoot.activeElement is a focusable element. Please give us a concrete proposal how activeElement should be if a slot does not generate CSS BOX. In addition that,
What happened to this? Please give us a concrete proposal how ':focus' pseudo class and 'focus event' work and the relationships between activeElement. |
Since Furthermore, whether an element generates a CSS box or not is nothing to do with whether an element is focusable or not. For example, elements in the fallback contents of a canvas element can be focusable but they never generate CSS boxes. |
My point is that we should be extremely careful to give the existing API yet another meaning.
Okay, I've added how
My proposal can satisfy the following expectations:
const previousFocusedElement = documentOrShadowRoot.activeElement;
....
previousFocusedElement.focus(); // This makes sure that previousFocusedElement gets focused again. I did not realize that the fallback contents of a canvas is an exception, but I do not think we should spread out such a bad citizen any more. I appreciate if someone give us a concrete proposal other than an option B, so that we can compare proposals in a broader perspective. Please include the formal definition or an algorithm for:
|
There is no meaning or existing API since this is about
It's not bad at all. It's HIGHLY desirable that fallback contents of a canvas element is focusable for accessibility purposes. In general, focusability of an element should be independent of whether it generates a CSS box or not for accessibility purposes. The alternative proposal is as follows:
In the sample,
|
I guess such a definition is not what you want. Your definition is missing cases, I guess. Suppose focusableX is focused in the following example, could you clarify what happens, and update the definition? <body>
<focusable1></focusable1>
<x-foo>
<focusable2 slot="slot1">
<focusableX></focusableX>
</focusable2>
<focusable3></focusable3>
</x-foo>
</body> x-foo's shadow tree: (updated after I got the reply :( ) <x-bar>
<div slot="slot2">
<slot name="slot1"></slot>
</div>
</x-bar>
<focusable4></focusable4> x-bar's shadow tree: <slot name="slot2"></slot> |
In that example, |
Thanks. Now I understand your intention clearly, which matches what I expected. We need a formal definition for that, which is not difficult to define, I think. BTW, I still prefer [(option B) + (New API)]. New API would tell us the exact focused element, instead of a slot. |
By "exact focused element", you mean the lowest shadow-inclusive ancestor of a focused element, which is unclosed to the shadow root? If so, we should just make |
Yeah, that's the option A, I think. But #358 (comment) has a concern about activeElement having such a behavior. That's the reason why I am proposing option B (+ new APIs). |
@bicknellr, do you have any updates on this? Any thought? |
I still think that Maybe this property could be called
IMHO, ShadowRoot should only provide access to things within itself, with the notable exception of the reference back to its host. Slots seem like a better place to access things distributed into the ShadowRoot from outside, but these accesses should be restricted to the portion of the ancestor tree that was assigned to the slot. If someone wants build a component that reaches elements outside of a ShadowRoot's composed tree, it should be up to them to get there by going up through the component's host through something like |
Thanks @bicknellr for the updates in detail! @rniwa, I don't buy the "ShadowRoot.activeElement is a new API and we can do anything On the other hand, I understand it would be nice for a developer to have a way to know |
I also don't buy that returning a slot would be violating the "API semantics". |
@bicknellr. Thank you. The idea looks promising. I like it. I prefer:
If we can agree on this idea, let me update the spec. |
Let me close this with the conclusion of Option B. |
In case someone end up here and need a workaround, this did the job for me: deepActiveElement(root: DocumentOrShadowRoot = document): Element | null {
if (root.activeElement && root.activeElement.shadowRoot && root.activeElement.shadowRoot.activeElement) {
return deepActiveElement(root.activeElement.shadowRoot);
}
return root.activeElement;
} And just call it : |
It seems strange for a ShadowRoot's
activeElement
to point to elements outside of that ShadowRoot. In particular, the spec currently seems to allow a ShadowRoot'sactiveElement
to point not only to elements within the ShadowRoot but also elements in the tree that the ShadowRoot's host participates in (and possibly even further outwards).I think it would be more reasonable if
activeElement
of a ShadowRoot could only point to an element contained within that ShadowRoot (or be null if none was focused). Additionally, if the 'deepest' focused node (i.e. the focused, UA-implemented element without a ShadowRoot) was nested further within descendant ShadowRoots, any given ShadowRoot'sactiveElement
along that chain would point to the most shallow host of that chain contained within that ShadowRoot.One implication of this approach is that determining focused descendants of the ShadowRoot's host means checking
activeElement
of the Document / ShadowRoot containing that host. This situation seems to break encapsulation by implying that the code controlling the ShadowRoot / component needs to look at its surrounding tree. However, I would argue that it's preferable as descendants of the ShadowRoot's host (i.e. those outside of the ShadowRoot) should be the responsibility of the code controlling the tree in which those descendants (and the host) participate. Also, there's precedent for using a tree's root to determine focus within that tree given Document'sactiveElement
property already works this way.An option for providing the ShadowRoot with some insight into what non-shadow descendants of its host might be focused would be to have
activeElement
point at a slot when an element distributed to that slot (or descendant of) is focused. (Maybe even give slotsactiveElement
that point to the focused non-shadow descendant? They seem a lot like ShadowRoot hosts with no descendants anyways.) I don't think this would really be necessary though, given that you could just check if the activeElement of the host's containing Document / ShadowRoot is contained within the host, but it might make working with focus of non-shadow tree descendants within a component easier.In general, I think this would make the scope of responsibility of code watching focus of a component's nodes more closely mirror shadow tree boundaries.
(Also, I get the feeling that a ShadowRoot's encapsulation mode would somehow be relevant to this but I haven't been able to figure out anything about encapsulation modes from the spec other than that a ShadowRoot has one and it's either 'open' or 'closed'.)
The text was updated successfully, but these errors were encountered: