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.

Sign up for GitHub

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

Jump to bottom

Validator: support shadow slots #10345

Closed
dvoytenko opened this issue Jul 11, 2017 · 9 comments
Closed

Validator: support shadow slots #10345

dvoytenko opened this issue Jul 11, 2017 · 9 comments
Assignees
@powdercloud
Labels
Category: PWA P1: High Priority WG: caching
Milestone
Prioritized FRs

Comments

@dvoytenko
Copy link
Contributor

See https://developers.google.com/web/fundamentals/getting-started/primers/shadowdom

An example:

<slot name="abc">
  ...
</slot>

The slot element is much like a div. All global attributes are allowed, such as id and on. The only specific attribute to slot element is the optional name attribute.
@dvoytenko dvoytenko added this to the Prioritized FRs milestone Jul 11, 2017
@Gregable
Copy link
Member

Only allowed in AMP, not AMP4ADS.

@Gregable Gregable assigned powdercloud and unassigned Gregable Jul 11, 2017
@powdercloud
Copy link
Contributor

What about the slot attribute? I see it there on the page... Is it needed as well?

@powdercloud
Copy link
Contributor

Also it would be useful to have a more complete example of how it's going to be used in AMPHTML. E.g., I see the example in the doc under "Flattened DOM tree" (https://developers.google.com/web/fundamentals/getting-started/primers/shadowdom#lightdom) but there are obviously other things that aren't allowed either, e.g. putting some custom element name (in the example "better-button") that's not known to the validator.
So I'd like a more complete example for the test please...

@jridgewell jridgewell mentioned this issue Jul 11, 2017
Closed
@dvoytenko
Copy link
Contributor Author

The slot attribute is not strictly needed at this time, so I'd rather skip. You can see an example here: https://github.com/ampproject/amphtml/pull/10346/files#diff-40013838fcacfcd8aaef22101639e190

but there are obviously other things that aren't allowed either, e.g. putting some custom element name (in the example "better-button") that's not known to the validator.

Not sure I understand this point. Could you clarify? The name of the element is slot, so it can't be named anything else...

@powdercloud
Copy link
Contributor

powdercloud commented Jul 11, 2017 •
edited

Cool. I was just looking for a complete example and was referring to a snippet in the shadowdom primer. I'm happy with wahtever is needed.
Please note that the content tag is currently also disallowed, but it's used in the example you're pointing at. Should we allow it? If so, should the select attribute on a content tag be allowed?
(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/content).

@dvoytenko
Copy link
Contributor Author

I see. Yeah, outside of AMP the app would use the slot attribute. Structurally it'd look like this:

<div class="host">
  #shadow-root <
     AMP IS HERE. Our validator rules apply.
     <body>
       <slot name="slot1">
          This is shown if no slot can be filled.
       </slot>
     </body>
  >
  <div slot="slot1">
     This will replace slot[name=slot1] in the AMP doc.
   </div>
</div>

@dvoytenko
Copy link
Contributor Author

RE: content tag. This has been deprecated and I'd be in favor of not allowing it at this time.

@Gregable Gregable mentioned this issue Jul 14, 2017
Merged
@dvoytenko
Copy link
Contributor Author

Thanks for the fast turnaround!

@Gregable
Copy link
Member

This should be live in validation in about an hour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@powdercloud powdercloud

Labels
Category: PWA P1: High Priority WG: caching
Projects
None yet
Milestone
Prioritized FRs
Development

No branches or pull requests
4 participants
@Gregable @dvoytenko @honeybadgerdontcare @powdercloud