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

Finalize the slots proposal #128

Closed
3 tasks
dglazkov opened this issue Jul 1, 2015 · 12 comments
Closed
3 tasks

Finalize the slots proposal #128

dglazkov opened this issue Jul 1, 2015 · 12 comments
Assignees

Comments

@dglazkov
Copy link
Contributor

dglazkov commented Jul 1, 2015

The proposal is here: https://github.com/w3c/webcomponents/blob/gh-pages/proposals/Slots-Proposal.md

Task list:

  • Review slotting algorithm
  • Review get distributed nodes algorithm and rename it to get composed nodes algorithm
  • Enumerate all DOM operations that will trigger the slotting algorithm. This will be defined as a synchronous action.
@dglazkov dglazkov added the v1 label Jul 1, 2015
@dglazkov dglazkov self-assigned this Jul 1, 2015
@hayatoito
Copy link
Contributor

The proposal looks good to me at first glance. I'll take another look later.

Quick question:

Enumerate all DOM operations that will trigger the slotting algorithm. This will be defined as a synchronous action.

What is the motivation for making this list? Do we expose the new API to access the result of the slotting algorithm?

@domenic
Copy link
Collaborator

domenic commented Jul 2, 2015

Enumerate all DOM operations that will trigger the slotting algorithm. This will be defined as a synchronous action.

Hmm, is there a reason we can't use this list to trigger an imperative distribution API?

@dglazkov
Copy link
Contributor Author

dglazkov commented Jul 2, 2015

What is the motivation for making this list? Do we expose the new API to access the result of the slotting algorithm?

Interoperability. A precisely defined list ensures that we don't have to hunt down subtle bugs across multiple implementations.

Hmm, is there a reason we can't use this list to trigger an imperative distribution API?

Sadly, no. The synchronous action is mostly a spec construct to precisely describe the cause and effect. It is unlikely that browser implementations will even use the exact algorithm for implementations.

@hayatoito
Copy link
Contributor

Interoperability. A precisely defined list ensures that we don't have to hunt down subtle bugs across multiple implementations.

I see, however, this list might not be enough for interoperability. What we need is a list which will trigger the distribution resolution algorithm, I think.

Most of exposed states depends on the result of distribution resolution algorithm, rather than the result of the slotting algorithm. At first glance, a list which will trigger the distribution resolution might be a strict super set of a list which will trigger the slotting algorithm.

Let me take another look later.

@hayatoito
Copy link
Contributor

Now I can feel that I understand the intention of the new proposal.
Because all browser vendors seem to have a rough consensus about adapting the Slots Proposal, let me start to spec "the Slots proposal" soon. I'll fix minor issues in the proposal too.

I'll remove the current <content> insertion points from the spec entirely.
For the backward compatibility in Blink, I'll write another doc about the unified distribution algorithm, which will be used in Blink to support co-existence of both models.

See #130 also.

@hayatoito
Copy link
Contributor

In the proposal,

When each node is assigned to a slot, this slot is also added to the node's destination insertion points list.

I'm afraid that this is imcomplete because each node is assigned to at-most one slot in the proposal.
A node's destination insertion points should be a list of nodes, rather than a single insertion point.

@hayatoito hayatoito mentioned this issue Jul 29, 2015
@dglazkov
Copy link
Contributor Author

Yep, you're right. In the slot world, there will be only one node in that list, so it doesn't need to be a list.

@hayatoito
Copy link
Contributor

Yeah. Instead of the current getDistinationInsertionPoints(), I have a plan to add:

partial interface Node {
  [nullable] Element assignedSlot;  // a tentative name
}

I haven't decided whether assignedSlot returns a directly assigned slot or the final destination slot, in the case of re-distribution.
Note that users can implement the latter on the top of the former, by calling it recursively.

@hober
Copy link

hober commented Jul 30, 2015

Given the entirely-local nature of distribution in the slots world, assignedSlot should return the directly assigned slot. The fact that that slot happened to be distributed into another slot is not relevant.

@hayatoito
Copy link
Contributor

Yeah, but we should be aware that main use case of getDestinationInsertionPoints() is the last element of the return value of it, called final destination insertion point. There, the node is rendered. That's not the local nature, even in the slots world. The situation hasn't changed at all, in terms of rendering.

If we provide only assignedSlot which returns a directly assigned slot, I'm afraid that we are forcing web developers to write a utility function, such as:

function slotWhereNodeIsRendered(node) {
  var slot = node.assignedSlot;
  while (slot.assignedSlot) {
    slot = slot.assignedSlot;
  }
  return slot;
}

This takes O(N), but I'm sure that the platform can return the same result in O(1), though it depends on the implementation of user-agent.

I guess the performance doesn't matter here. N might be small enough in most cases. Providing only assignedSlot might be okay at first. That's a primitive.

@annevk
Copy link
Collaborator

annevk commented Jul 31, 2015

Yeah, if it's actually common for developers to write utility methods for the composed tree, we can consider adding them. Let's just make v1 the minimum viable thing.

@hayatoito
Copy link
Contributor

Let me close this bug since I've finished the spec-ing of slots. I hope I can say that the current spec has the processing equivalence to the proposal.
Although some of tasks in the first comment are not finished, I don't think that's a blocking issue to implement Shadow DOM.

Please feel free re-open this bug (or file a new bug) if we are to address some of the task list.

kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
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

5 participants