-
Notifications
You must be signed in to change notification settings - Fork 378
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
Need "slotchange" event #288
Comments
I vote v2 or at least not the MVP we can ship when we're ready aligning with all decisions made thus far. |
For interoperability, should we finish the following task (mentioned in #128):
to guarantee when such an event is fired? I also vote v2. I'd like to be careful about exposing the timing of running the slotting algorithm to developers. |
In my web components I use a MutationObserver to watch for changes under the host node. It seems to do what you are asking. |
MutationObserver doesn't work since a slot may be the child of an element with a shadowRoot. In this case, the child list of the element may not change but the elements that are rendered in place of the element's slot do change. |
Having worked with Shadow DOM a lot, not having this feature in Chrome's existing implementation is very cumbersome. I'd really like to see this api included in v1 if possible. If it's ever important to be able to manage your children (menu, tabs, stack of pages, carousel, etc), then you need to know when the elements distributed to your slots have changed. MutationObservers are quite cumbersome to use for this: To use MO, you have to do something like: (1) MO your own childList, (2) whenever a slot is added to you, (3) find that slot's host element (recurse up the tree to find the shadowRoot.host) and MO that host's childList, and (4) repeat step 2 for that element. This is just to see childList changes. To get the actual distribution changes to a given slot, you have to observe attributes on every one of these children and then, perhaps, do a dirty checking scheme to detect what the changes are. The platform has all this information as a result of distributing. For the user to do this correctly is both complicated and requires significant duplication of effort. |
@sorvell you'd need to propose a concrete processing model for this event and get buy-in from everyone. I'd really rather not put Shadow DOM back in a state where not everything has buy-in from everyone. Shipping across browsers is more important. |
I definitely do not want to jeopardize buy-in. The momentum that Shadow DOM has right now is great. I'm hoping this is a relatively easy addition, especially with the simplified slots model that has been adopted. I think it could be a non-bubbling event fired when MutationObservers are. Perhaps it could just be an addition to MutationObservers, and therefore decoupled from the Shadow DOM spec. |
Preferably this would just be an async notification that your distribution changed, perhaps even just MutationObserver can have a type of "distributedChildList" ? |
Let me add label v1 to this issue to get more attention. |
This is about getting notified when the final distribution is changed or when assigned nodes are changed? If former, I'm opposed to it at least in v1. |
Generally speaking, I'm excited to see Shadow DOM v1 ship sooner rather than later, even if it means living without the ability to detect changes in slot assignment as discussed here. That said, I want to make clear that, without this feature, I believe it's prohibitively difficult to build components that can be composed as people expect. We would like people to be able to build custom elements that are every bit as reliable as the standard HTML elements, and have proposed a set of criteria for evaluating that level of quality called the Gold Standard checklist. One of the checklist items, Content Reprojection explicitly encourages component authors to properly handle reprojected content, even when that content changes. The example described on that page — counting the number of children in the final distribution — is trivial, but this issue has come up in practice time and again. As a more practical example, the Basic Web Components project includes a Again, I'm not saying Shadow DOM v1 should be held up for this feature. (I'd rather having something than nothing.) All I want to do is make plain the very real limitations of not having this feature. |
@JanMiksovsky : Do you want an event to detect when the "distributed" (i.e. after unwrapping / flattening slots) nodes are changed or when assigned nodes are changed? Also, do you want this event be synchronous or asynchronous? |
As a component author, I want to be careful to focus on the scenarios I encounter while writing production components, and leave questions about the actual API design to browser engineers. With that in mind, I only want to call out the component requirements I see here that bear on this question. In our basic-carousel example, the carousel wants to know how many children (e.g., img elements) it is showing, in order that it might show the correct number of dots. As I understand the distributed/assigned distinction, I believe that basic-carousel wants to know about changes to the set of nodes distributed to its default slot. If basic-carousel only learned when the set of nodes assigned its default slot had changed, that would seem insufficient to support scenarios in which basic-carousel is composed into other components. For example, some users of basic-carousel want to compose a basic-carousel instance into a component of their own. Let’s call their new component outer-carousel. They create a default (If I’ve misunderstood things here, or there’s some other way in which basic-carousel could achieve its composability objectives, please let me know.) Regarding timing, I don’t think basic-carousel presents any requirement for synchronous timing; asynchronous notification would be fine. Ideally, it would learn about distribution changes before the changes have rendered (so that there’s no visible lag between the appearance of a new image in the carousel and the appearance of a new dot for it). But beyond that, I don’t see any special timing needs. |
After discussing with @JanMiksovsky in person today, I'm pretty convinced that we need to add this event. I think the question is the timing (sync vs. async vs. end-of-mirotask/nanotask) as well as the name. I would like to stay away from the term "distribution" as much as possible. |
+1. I'm super positive to add this feature. |
I'm also very welcome any concrete proposal if someone can write it. I need a help from an expert, if we are going to use MutationObserver. |
It's great to hear 2 major vendors being open to this feature. The Polymer library recently added support for this type of notification. It is a lot of code to manage and the performance degrades in complex cases due to the need to manage sets of Mutation Observers. Polymer is fine with an asynchronous notification, ideally with the same basic behavior as Mutation Observers. |
If this follows |
+1 for adding to MutationObserver records |
This was discussed at the recent custom elements F2F meeting. I believe there was general agreement that this a critical feature needed for webcomponents 'v1' implementations and that
|
Here's a concrete API proposal: Add On a mutation record of this type: We can add a new step to each DOM mutation such as the concept insert to queue a mutation record of type Alternatively, we can store the list of distributed nodes for each slot element at the beginning of each micro task, and queue a mutation record if the list of nodes had changed at the end of the micro task. |
Would the user facing api by like this?
If so, this seems fine. Thanks! |
Is there a reason not to just use an event for this? Creating a new MutationRecord type that contains none of the usual MutationRecord fields seems odd. The trickiest part of using an event is getting it to fire at microtask timing, but I suspect we already have enough spec machinery to do that. |
Yeah, now that I'm thinking about this more, it seems more natural to add an event on HTMLSlotElement whenever the distributed nodes change. |
I'm afraid that an event will be too spammy if we define the timing of "distributed node change" naively. The trickiest part is how we can define the timing of "distribution nodes change" so that we do not have an interoperability issue. In the current spec, UA can delay the calculation of distributed nodes until it is actually demanded as long as developers can not see the difference. |
We can just check that at the end of each micro task. |
That might be unacceptable because it could be super expensive. To make the topic more understandable, suppose that if we had an imaginary "ComputedStyle Change" event, when should we dispatch such a "ComputedStyle change" event? UA do not want to calculate ComputedStyle at the end of each micro task, I think. Does someone have any idea? I am aware that supporting this feature request, "Slotting change event", is very important for developers. I need a workable solution. |
No, checking this condition is pretty easy & efficient since you can determine whether a node belongs to a given slot or not in O(1). Essentially, each shadow root needs to store the names of all slots its tree contains in a hash set/map. When its host' children changes, it can notify the shadow root that the slot assignments have changed for a specific name / default slot (keep repeating this process if the slot's parent also has a shadow root if you wanted to get notified of changes in distributed nodes). Now, at the end of micro task, you visit all those shadow roots that have been notified of such potential changes and those that do have relevant event listeners. If the slot element had been inserted or removed from a given shadow root, you may need to resolve the first slot for each slot name in O(n). Otherwise, dispatch the event for each slot for which the shadow root had been notified of the change in its node assignments. Having said that, I'm not opposed to running this at the end of the task (before painting essentially) although that might be painfully late for some use cases. Exposing style resolution timing is not acceptable for us. |
So you want to fire
That sounds right. You DO need to recursively check |
That makes sense. You want to perform slotting though so you know if the next insert/remove changes things.
And all of them get an event, right? Thank you for mentioning that. |
Right.
Yup. |
"Insert" means this timing, https://dom.spec.whatwg.org/#concept-node-insert, right?
It looks we should remove the condition of "node's (new) root is a shadow root" from there. e.g. document tree: <x-foo>
<slot id=s1>
<div id=inserted></div> <--- inserted
</slot>
</x-foo> x-foo' shadow tree: <slot id=s2></slot> #s2's "slotchange" should be fired if #inserted is inserted as a child of #s1. In addition to insert and remove, we have to check the change of element's "slot" attribute and slot's "name" attribute. Let me take another look later. |
@hayatoito "insertion steps" run at roughly the same time as "insert", they're part of the same atomic operation anyway. |
@hayatoito your example is a case I missed by the way, that would be a "node is inserted into a parent that is a slot element". If nothing is assigned to the node, that would be the fallback that will get assigned. |
Here is some pseudo-code I plan to adopt for the DOM Standard next week. Hopefully folks can take a look at it and hopefully it's understandable. I'm assuming that slotables have pointers to slots and slots hold a list of pointers to slotables and both need updating during mutations. Flattening is not calculated as that does not seem necessary. CSS and the
Obviously there's a bit of copypasta here that can be (and will be in the spec) reduced with extra algorithms. |
Thanks. I have reviewed "insert" part. Let me review other parts later. It looks that there is an assumption that we preserve the result of assgnSlottables(slot) somewhere.
How slotchange(slot) can be defined? I think that should be recursive, like:
Does this match your expectation? Ops. I am afraid that this slotchanged(slot) is still not enough because a slot might be a fallback content of another slot. That makes the situation complex. We need additional check:
The node is never assigned to the parent slot, according to the definition.
This should be in tree order (which is implicit?) to make sure that assignSlottables(slot-A) is called before assignSlottables(slot-B-which-is-a-descendant-of-A) is called. That is required to handle the fallback contents correctly. |
Yes. A slot has assigned nodes (a list of slotables). A slotable has an assigned slot ( a slot). The assign slotables algorithm updates both those internal variables. (Therefore the node can end up being assigned to the parent when it is inserted into a slot. I don't see a reason to special case fallback content. It should just end up being assigned if there only is fallback content.) The "slotchange" algorithm (going to call it signal a slot change most likely) is indeed recursive, but does not need "findSlot", it can simply use the assigned slot pointer for recursion. |
Yeah, I am feeling that "Not considering fallback contents as assigned nodes" is making the algorithms around here complicated. However, that's the conclusion in #317. I am open to either option:
@rniwa , WDTY? |
Oh, I missed that |
The fallback case means remove needs to run "slotchange(parent)" when a node is removed from a parent that is a slot with assigned nodes being empty. |
Yes, that's exactly same to what I am about to say for "remove part"
|
By the way, |
Fixes WICG/webcomponents#288. This defines the slotchange event in response to remove/insert operations, changes to a slot’s name attribute, and changes to an element’s slot attribute. This also introduces the assigned nodes concept for slots, and assigned slot concept for slotables. Slotables now also have a name, rather than a “get name”. The slotchange event dispatches just after mutation observers.
I posted a PR at whatwg/dom#229 that I'd appreciate feedback on. I think there's some potential for more cleanup to what I did and I'm going to study it more now, but it should meet all requirements as-is. |
Shadow: define slotchange event Fixes WICG/webcomponents#288. This defines the slotchange event in response to remove/insert operations, changes to a slot’s name attribute, and changes to an element’s slot attribute. This also introduces the assigned nodes concept for slots, and assigned slot concept for slotables. Slotables now also have a name, rather than a “get name”. The slotchange event dispatches just after mutation observers have their callbacks invoked. The slotchange event is not yet marked scoped as scoped events are not yet defined in the DOM Standard. Note: although the original issue did not mention it, this also suppresses signaling when slots are removed from a tree. Not just when they are inserted.
* Add more legacy event types for createEvent() Approximately as requested at https://bugzilla.mozilla.org/show_bug.cgi?id=1251198#c7. This is the list of events supported in createEvent() by at least two of Firefox, Chrome, and IE 11. The one exception is I omitted MutationEvent, which all three support, because apparently spec-land has decided to deny its existence in the hope that it will go away. Bikeshed does not know about all of the added interface names, hopefully that will sort itself out over time. * Meta: improve pull request instructions for DOM See whatwg/fetch#276 for context. * Enable an event listener to be invoked just once * Editorial: web compatibility typically remains relevant Fixes whatwg#210. * Shadow: define attachShadow() for custom elements * Meta: make it easier to reference participate in a tree PR: whatwg#216 * Define node document for new Text nodes Fixes whatwg#224 and part of whatwg#212. Also fix part of whatwg#209 by stating these algorithms can rethrow exceptions. * Use a single concept for attribute changes This setup is still a little sketchy I think, but not more so than the insertion and removing steps. * SVGEvent is only Gecko, Blink also has SVGEvents As pointed out by zcorpan in whatwg#227. * Set createDocument()'s content type based on namespace Fixes whatwg#217. PR: whatwg#218 * Make document.createEvent("touchevent") sometimes throw Browsers typically disable touch events on non-touch devices, and there exists web content that detects this difference using document.createEvent(). Fixes whatwg#227. * Shadow: define slotchange event Shadow: define slotchange event Fixes WICG/webcomponents#288. This defines the slotchange event in response to remove/insert operations, changes to a slot’s name attribute, and changes to an element’s slot attribute. This also introduces the assigned nodes concept for slots, and assigned slot concept for slotables. Slotables now also have a name, rather than a “get name”. The slotchange event dispatches just after mutation observers have their callbacks invoked. The slotchange event is not yet marked scoped as scoped events are not yet defined in the DOM Standard. Note: although the original issue did not mention it, this also suppresses signaling when slots are removed from a tree. Not just when they are inserted. * Editorial: update custom element cross-spec references * Editorial: fix a few cross-linking missteps * Editorial: make "is" and "prefix" optional in "create an element" * Use "create an element" in createHTMLDocument Takes care of part of whatwg#212. * Editorial: align exception language with IDL * Editorial: introduce more shadow-including terms for CSS Fixes whatwg#225. * Editorial: distributed -> flattened * Meta: export more terms Fixes whatwg#233. * Editorial: add "shadow host" and "assigned" as terms This makes a couple of non-null checks read better and enshrines a term we had already been using. * Editorial: flip non-null/otherwise conditions PR: whatwg#234 * Shadow: <slot> is now defined in HTML * Remove passive as event listener key This changes makes passive no longer contribute to the uniqueness of an event listener. It therefore also no longer needs to be supported as part of removeEventListener(). Fixes WICG/EventListenerOptions#27. PR: whatwg#236 * Meta: point out event's timeStamp is likely to change See whatwg#23 for details. * Add [CEReactions] annotations to mutating methods Part of WICG/webcomponents#186, and furthering whatwg/html@27aa7bc. Linking [CEREactions] will happen once speced/bikeshed#677 is fixed. * Editorial: check stop propagation flag at start of invoke * Editorial: deduplicate Veli Şenol * Editorial: define defaults for EventListenerOptions Although this is also done in prose, this nonetheless simplifies the prose a bit and makes it clearer to those skimming the standard what is going on (although skimming is not recommended). Fixes whatwg#239. * Meta: link to Japanese translation See triple-underscore/triple-underscore.github.io#1 for more details.
As an end-user, having a With the current @rniwa said,
I do that myself by deferring my render logic to an animation frame. But, not all users may be adept enough to do that, and those users may not necessarily care about animation anyways. In fact, those users probably will never touch MutationObserver. @annevk said,
What are the use cases where people want to listen to @rniwa said,
Yes, please. I myself at least don't have any other reason to listen to slotchange other than to get the list of nodes that got inserted and removed. Is it coincidence that the first time I need to use
If we use MutationObserver, with an opt-in option, how is that prohibitively more expensive than having to do it manually in JavaScript? It's not expensive at all if users don't opt-in similarly to childList, O(1). @treshugart said,
For my lib to be able to keep track of the final flat tree (for rendering in WebGL), this is needed. When nodes are assigned into slots, they have new parents with respect to the WebGL render tree (i.e. they are transformed relative to their slot parent). My lib keeps track of two things: when nodes are "possibly distributed" in which case they are not rendered relative to their original parents, and when those same nodes are "assigned", in which case they are rendered relative to their slot parents. So, when adding a shadow root to a host, the host's children automatically become "possibly distributed" and will not be rendered unless they happen to be distributed to a slot, and the rendering happens relative to that new location. For more perspective, I believe that if a library like http://aframe.io (which renders Custom HTML Elements to WebGL via Three.js) wants to be compatible with ShadowDOM, then that library will also have to know how nodes are distributed (i.e. keep track of the flat tree), and the most obvious way to do that is keeping track of nodes that are assigned/unassigned to slots. Using MutationObserver for this would be much cleaner than making us do it in user-land. The problem is, we need some way to determine the final position of a distributed node. For example, if a slot S1 is distributed to a slot S2, which is distributed to S3, which is distributed to S4, then a node assigned to S1 would be finally distributed to S4 and would render relative to the parent of S4. This is what want to keep track of so that I can render to WebGL. If all the shadow trees are open, then this is no problem: I can check Besides
Does that |
Revised my previous comment. I have a clearer picture after some testing. It might be nice to have a method (f.e. For example, here's a jsfiddle showing S1 through S4 assigned nodes flattened: https://jsfiddle.net/trusktr/9bs9mryt/ The output looks like this:
If we had a new method for getting strictly finally distributed nodes (
where we see the final position of the
Get what I mean? I think that I believe that that would be useful for my case where I want to know the final positions where nodes are distributed. I believe that I can currently achieve this by hijacking Made a new issue for the idea: #611 |
https://bugs.webkit.org/show_bug.cgi?id=155424 <rdar://problem/24997534> Reviewed by Antti Koivisto. Source/WebCore: Added `slotchange` event as discussed on WICG/webcomponents#288. While the exact semantics of it could still evolve over time, this patch implements as an asynchronous event that fires on a slot element whenever its distributed nodes change (flattened assigned nodes): http://w3c.github.io/webcomponents/spec/shadow/#dfn-distributed-nodes Since inserting or removing an element from a shadow host could needs to enqueue this event on the right slot element, this patch moves the invalidation point of element removals and insertions from Element::childrenChanged to Element::insertedInto and Element::removedFrom. Text nodes are still invalidated at Element::childrenChanged for performance reasons since it could only appear within a default slot element. Because this more fine-grained invalidation needs to be overridden by HTMLDetailsElement, we now subclass SlotAssignment in HTMLDetailsElement instead of passing in a std::function. Test: fast/shadow-dom/slotchange-event.html * dom/Document.cpp: (WebCore::Document::enqueueSlotchangeEvent): Added. * dom/Document.h: * dom/Element.cpp: (WebCore::Element::attributeChanged): Call hostChildElementDidChangeSlotAttr. (WebCore::Element::insertedInto): Call hostChildElementDidChange. (WebCore::Element::removedFrom): Ditto. (WebCore::Element::childrenChanged): Don't invalidate the slots on ElementInserted and ElementRemoved since they're now done in Element::insertedInto and Element::removedFrom. * dom/Event.cpp: (WebCore::Event::scoped): slotchange event is scoped. * dom/EventNames.h: Added eventNames().slotchange. * dom/ShadowRoot.cpp: (WebCore::ShadowRoot::invalidateSlotAssignments): Deleted. (WebCore::ShadowRoot::invalidateDefaultSlotAssignments): Deleted. * dom/ShadowRoot.h: (ShadowRoot): Added more fine-grained invalidators, mirroring changes to SlotAssignment. * dom/SlotAssignment.cpp: (WebCore::SlotAssignment::SlotAssignment): Removed a variant that takes SlotNameFunction since HTMLDetailsElement now subclasses SlotAssignment. (WebCore::SlotAssignment::~SlotAssignment): Added now that the class is virtual. (WebCore::recursivelyFireSlotChangeEvent): Added. (WebCore::SlotAssignment::didChangeSlot): Added. Invalidates the style tree only if there is a corresponding slot element, and fires slotchange event. When the slot element we found in this shadow tree is assigned to a slot element inside an inner shadow tree, recursively fire slotchange event on each such inner slots. (WebCore::SlotAssignment::hostChildElementDidChange): Added. Update the matching slot when an element is inserted or removed under a shadow host. (WebCore::SlotAssignment::assignedNodesForSlot): Removed the superfluous early exit to an release assert since addSlotElementByName should always create a SlotInfo for each element. (WebCore::SlotAssignment::slotNameForHostChild): Added. This is the equivalent of old m_slotNameFunction which DetailsSlotAssignment overrides. (WebCore::SlotAssignment::invalidateDefaultSlot): Deleted. (WebCore::SlotAssignment::findFirstSlotElement): Added an assertion. slotInfo.element must be nullptr if elementCount is 0, and elementCount must be 0 if slotInfo.element is nullptr after calling resolveAllSlotElements, which traverses the entire shadow tree to find all slot elements. (WebCore::SlotAssignment::assignSlots): * dom/SlotAssignment.h: Implemented inline functions of ShadowRoot here to avoid including SlotAssignment.h in ShadowRoot.h. Not inlining them results in extra function calls for all builtin elements with shadow root without slot elements, which impacts performance. (WebCore::ShadowRoot::didRemoveAllChildrenOfShadowHost): Added. (WebCore::ShadowRoot::didChangeDefaultSlot): Added. (WebCore::ShadowRoot::hostChildElementDidChange): Added. (WebCore::ShadowRoot::hostChildElementDidChangeSlotAttribute): Added. (WebCore::ShadowRoot::innerSlotDidChange): * html/HTMLDetailsElement.cpp: (WebCore::DetailsSlotAssignment): Added. Subclasses SlotAssignment to override hostChildElementDidChange and slotNameForHostChild. (WebCore::DetailsSlotAssignment::hostChildElementDidChange): Added. We don't check if this is the first summary element since we don't know the answer when this function is called inside Element::removedFrom. (WebCore::DetailsSlotAssignment::slotNameForHostChild): Renamed from slotNameFunction. Also removed the code to return nullAtom when details element is not open as that messes up new fine-grained invalidation. Insert/remove the slot element in parseAttribute instead. (WebCore::HTMLDetailsElement::didAddUserAgentShadowRoot): Don't insert the slot element for the summary since the details element is not open now. (WebCore::HTMLDetailsElement::parseAttribute): Remove and insert the slot element for the summary here instead of changing the behavior of slotNameForHostChild. * html/HTMLDetailsElement.h: * html/HTMLSlotElement.cpp: (WebCore::HTMLSlotElement::enqueueSlotChangeEvent): Added. Enqueues a new slotchange event if we haven't done so for this element yet. (WebCore::HTMLSlotElement::dispatchEvent): Added. Clear m_hasEnqueuedSlotChangeEvent when dispatching a slotchange event so that a subsequent call to enqueueSlotChangeEvent would enqueue a new event. Note scripts call EventTarget::dispatchEventForBindings instead. * html/HTMLSlotElement.h: LayoutTests: Added a W3C style testharness.js test. * fast/shadow-dom/ShadowRoot-interface-expected.txt: * fast/shadow-dom/ShadowRoot-interface.html: Don't import testharness.css from svn.webkit.org. * fast/shadow-dom/slotchange-event-expected.txt: Added. * fast/shadow-dom/slotchange-event.html: Added. Canonical link: https://commits.webkit.org/173540@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@198115 268f45cc-cd09-0410-ab3c-d52691b4dbfc
For developers, it will be good to have an event that is fired whenever slotting algorithm runs within a shadow tree. Maybe runs and produces changes compared to the previous result? Don't know. Also don't know if this is v1 or v2.
The text was updated successfully, but these errors were encountered: