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

isEncapsulated naming #107

Closed
domenic opened this issue Jun 5, 2015 · 18 comments
Closed

isEncapsulated naming #107

domenic opened this issue Jun 5, 2015 · 18 comments

Comments

@domenic
Copy link
Collaborator

domenic commented Jun 5, 2015

adae25b introduced the isEncapsulated flag to solve #61. It seems pretty great!

However, the naming is a bit weird.

  • No other booleans on the platform that I know of use an "is" prefix. And, Event itself definitely does not: bubbles and cancelable are the models to follow here. So, encapsulated makes more sense.
  • The term "encapsulation" is a new one that doesn't appear in other shadow-related APIs, as far as I know. Maybe there is something better? I cannot think of anything that is not overly verbose though (like "stopsAtShadowBoundaries"). But I thought others might have opinions, e.g. @annevk.
@travisleithead
Copy link
Member

Well, there's precedent in "isTrusted", but I'd prefer the non-is-prefixed name :)

@hayatoito
Copy link
Contributor

Yeah, I welcome any opinion. Actually, I've spent only a few minutes to give it a name. :)
I had a similar idea, stopsAtShadowRoot, but I chose isEncapsulated without a strong reason because I saw isTrusted in the spec, as Travis mentioned.

Because is-prefix is less-loved recently, I've changed it encapsulated at e91e7f0.

I think we still need a better idea. I don't want to expose a term of 'encapsulation' to the APIs.

The candidates:

  • stopsAtShadowBoundaries
  • stopsAtShadowRoot
  • stopsPropagationAtShadowBoundaries
  • Any better ideas.

What makes the naming difficult is that the event which is encapsulated doesn't always stop at a shadow root if distribution is involved in the event path.

e.g.
In the example of http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example, if an event happens on L, the event doesn't stop at Q or N. It stops at J.

Event path in this case: [L, P, R, Q, O, N, K, J]

@annevk
Copy link
Collaborator

annevk commented Jun 9, 2015

Isn't the semantic that the event is scoped to a subtree? scopedToSubtree or some such?

@travisleithead
Copy link
Member

I like sticking to the semantic. If this is true, then +1 to @annevk

@hayatoito
Copy link
Contributor

Thank you for the feedback.

I'm afraid that it might be ambiguous what subtree means here.
The term of subtree is widely used in the context of the node tree, IMO.

Here, we're scoping the event to sub-tree-of-trees.

@annevk
Copy link
Collaborator

annevk commented Jun 10, 2015

scopedToDeepSubtree?

@hayatoito
Copy link
Contributor

Thanks, however, I'm still reluctant to expose the term of subtree.

DeepSubtree sounds an attractive candidate, but I have a concern that developers have a wrong impression that the event is not dispatched to the parent node of the event.target by the name of subtree.

I welcome any wording which doesn't use subtree, though I have no idea. :(
If I am allowed to use a long name, I might name it:

  • event.doesNotLeakIntoAncestorTrees
  • event.scopedToInclusiveDescendantTrees.

I'm aware that both are not good name.

@annevk
Copy link
Collaborator

annevk commented Jun 11, 2015

The second one of those seems somewhat reasonable. Maybe excludesAncestorTrees?

@rniwa
Copy link
Collaborator

rniwa commented Jun 11, 2015

I think we should add propagatesAcrossShadowBoundary or propagatesToShadowHost or some variant thereof which returns true for non-encapsulated event instead. encapsulated is ambiguous as to within what or against what it's encapsulated.

@annevk
Copy link
Collaborator

annevk commented Jun 11, 2015

Propagates does not make sense for the capturing phase I think. But maybe includesShadowHost or some such if we want to avoid the tree terminology (which I think works too).

@rniwa
Copy link
Collaborator

rniwa commented Jun 11, 2015

True, includesShadowHost sounds good to me.

@hayatoito
Copy link
Contributor

Thanks for the feedback. I appreciate it.
I like the bikeshed. :)

I thought the same thing, like a 'event.xxxShadowHost', though I don't remember 'XXX'.

However, I abandoned it because I thought that it might impress developers that event is dispatched to only the shadow host (+ ancestor chain in the current node tree, of course), excluding the ancestor nodes of the shadow host. Am I worrying too much?

In addition to that, includes should be used for event.path, IMO.
If it is used for the attributes of the event, that might be weird.

event.includesShadowHostInPath might be better, but it is too long. :(

@hayatoito
Copy link
Contributor

At the same time, as @annevk mentioned, I want to avoid the tree terminology here as much as possible.
Most of the terminology are intended to be used internally in the spec. That's not intended to be exposed as the name of API. Using the tree terminology is a last resort.

@rniwa
Copy link
Collaborator

rniwa commented Jun 11, 2015

I don't think includesShadowHostInPath makes much sense because the path of an event isn't determined until an event is dispatched. I'm now leaning towards escapesShadowTree because that's what really this flag tells us. If we wanted the negation of that, I'd go with confinedToShadowTree.

I don't think there's any reason to avoid the word tree here.

@hayatoito
Copy link
Contributor

Thanks. The term of the shadow tree is okay to use. I just wanted to avoid to use other term such as "ancestorTress", "inclusiveDescendantTrees" because it should be "ancestorNodeTrees", "inclusiveDescendantNodeTrees" in a strict meaning. I omitted "Node" in the spec.

Yeah, I prefer the naming which is false by default.

WDYT about scopedToShadowTree? Which is better between confinedToShadowTree or scopedToShadowTree? Is confined a familiar term for developers?

@hayatoito
Copy link
Contributor

Otherwise, simply, how about scoped? Is this strange?

@rniwa
Copy link
Collaborator

rniwa commented Jun 11, 2015

I see. I think scopedToShadowTree is fine too. We could even go shorter and just say shadowScoped or shadowTreeScoped, or even scoped like you suggested. scoped sounds a little too generic but perhaps that isn't an issue because there isn't any other way for an event to be scoped like this.

@hayatoito
Copy link
Contributor

Thanks. Let's change it to scoped and see how people will complain about it.

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