Allow ::slotted(selectors) #113

JamesXNelson · 2017-02-27T10:37:20Z

::slotted selectors are necessary for ShadowDOM V1

It may make sense to allow ::anyname(selectors) instead of cherry-picking individual named selectors, but I do not know the code well enough to make such a judgement.

"If" tests are desired, let me know where to add them, and I can add that to PR.

iflan · 2017-03-01T20:13:51Z

Hi James,

Looking at ::slotted, it seems to have pretty specific semantics. Generally, from what I see, it's pretty hard to predict what should go in a pseudo element, so I don't think your generic proposal is a good fit yet.

It looks like Shadow DOM is also introducing a new selector, >>>, the shadow-piercing descendant combinator.

As far as the PR goes, I will need tests for it. I don't have time to do that in depth tonight, but I'll try to get to it soon.

Ian

iflan · 2017-03-01T20:50:12Z

(And by "do that", I meant "review the code".)

JamesXNelson · 2017-03-05T05:18:27Z

Heya.

As I continued to work with shadow DOM, I also needed :host(selectors > here), and :host-context(selectors > here) as well.

If you have any existing css infrastructure testing, I would be happy to put together an automated test. I see GssParserTest is probably good enough for this?

I think there is also support for ::slotted(div)::before as well, so it might start getting a touch more complicated if we want to hit the moving target that is shadow dom css. Let me know how I can be of assistance to get some (useful) code merged!

JamesXNelson · 2017-03-05T07:04:40Z

Note, the current working draft says that >>> combinators will only apply to querySelector(), and not css styling.

So, unless/until that changes, the most you'd want to do is issue a warning to developers that attempting to use this in a stylesheet will not work.

Also note, the draft shows ::slotted as containing complex selectors, but the discussion it is based on in the web component spec authors' github repo, the consensus was to support simple selectors only, as ::slotted(div.foo) with ::slotted(.foo)::before also supported; the idea being that you can only match the top-level children of a slot, so no combinator would ever make sense.

I made the PR match the recommendations I was able to grok from the still evolving spec:

"Note: ::slotted() can only represent the elements assigned to the slot. Slots can also be assigned text nodes, which can’t be selected by ::slotted(). The only way to style assigned text nodes is by styling the slot and relying on inheritance."

If we want to be pedantic, the only usable prefixes of ::slotted is empty ::slotted, slot::slotted or *::slotted; any other combination won't work, because only <slot /> elements can have a slotted pseudo.

iflan

Thanks for doing this! I think it looks good, generally, but I have a few comments.

If only simple selectors are allowed, then reusing the code we have for :not(selector), as you have done, seems right to me. :host(selectors > here) and :host-context(...) obviously need a special case, too. It would be great, though, if we could make the tokens "generic" and have just one for functions that take simple selectors and another for functions that take compound selectors.

I don't know if I want to try to enforce things like enforcing the usable prefixes of ::slotted in the grammar. Instead, I think that should be in a compiler pass were we to want it. If we do that, we can also warn about /deep/, which is currently supported.

You don't have to make that change in this PR unless you'd like to.

Also, if >>> isn't supported in rules, then I think I wouldn't even bother mentioning it.

Also, you can add tests here:

https://github.com/google/closure-stylesheets/blob/master/tests/com/google/common/css/compiler/ast/GssParserTest.java

Thanks!

Ian

iflan · 2017-03-06T07:00:21Z

src/com/google/common/css/compiler/ast/GssParserCC.jj

@@ -707,6 +707,8 @@ PARSER_END(GssParserCC)
  // Special handling needed because ':not' takes simple selectors as


Should we just generalize this to two tokens: SIMPLE_SELECTOR_FUNCTION and SELECTOR_FUNCTION? It seems like a bunch of duplicate code in the pseudo expansion would go away.

Aye. I agree that the duplication here is less than stellar.

However, I think the semantics of :not is slightly different.

:not() takes only a simple selector; one tag name, class name or id. I tested this in browser console, and it did not like any compound selectors ( :not(elementName.className) did not work)

The other three, ::slotted, :host and :host-context all support a compound selector (what simpleSelector() actually matches). ::slotted(div.clazz) is valid here.

I think simpleSelector should be renamed to compoundSelector, and then a new simpleSelector:

elementName() | id() | className() | pseudo()

I will test tonight that :host-context does not also support combinators.

not supports compounding only through duplication:

div:not(.class1):not(.class2)

iflan · 2017-03-06T07:02:17Z

src/com/google/common/css/compiler/ast/GssParserCC.jj

 }
 {
  t = <COLON>  { beginLocation = this.getLocation(); tokens.add(t); }
  ( ( t = <IDENTIFIER> { pseudo = t.image; tokens.add(t); } )
    |
+    ( // ::slotted( simple_selector )


Please update the docs above under pseudo.

iflan · 2017-03-06T07:10:01Z

closure-stylesheets.iml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>


I'm not sure that I really want this to be checked in. What's the advantage?

No advantage. It's an intellij project file.
Will remove this file, and perhaps add a .gitignore for it.

Unlike ::slotted(), which only allows simple selectors inside the parens, :host and :host-context allow complex selectors to be placed inside the parens.

JamesXNelson · 2017-11-20T02:05:07Z

Hi @iflan

Sorry I kind of forgot about this pull request.

I updated the comment per your request in the BNF.

I don't think we decided on any naming for splitting up the pseudo definition. I'd be happy to break it up if you decide what you want and let me know. Thanks!

JamesXNelson · 2017-11-20T02:07:54Z

I'll add some tests shortly.

Jauhen · 2018-04-16T21:50:21Z

It would be great if you also add support for ::cue() pseudo selector:
https://w3c.github.io/webvtt/#css-extensions-introduction

ghost · 2020-05-22T10:07:48Z

Custom Elements we're the perfect solution for me, until i couldn't style the children.. I do not get how custom elements are of any use without that.

googlebot added the cla: yes label Feb 27, 2017

JamesXNelson mentioned this pull request Feb 27, 2017

Allow ::slotted(selectors) #112

Open

JamesXNelson force-pushed the allow-slotted-selector branch from 74146a2 to f952d9d Compare March 5, 2017 04:48

iflan suggested changes Mar 6, 2017

View reviewed changes

JamesXNelson force-pushed the allow-slotted-selector branch 2 times, most recently from 943a638 to 4ef475c Compare November 20, 2017 02:02

JamesXNelson added 2 commits November 19, 2017 18:02

Allow ::slotted(selectors)

c5e73e8

Allow :host() and :host-context() selectors.

d5fbf75

Unlike ::slotted(), which only allows simple selectors inside the parens, :host and :host-context allow complex selectors to be placed inside the parens.

JamesXNelson force-pushed the allow-slotted-selector branch from 4ef475c to d5fbf75 Compare November 20, 2017 02:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow ::slotted(selectors) #113

Allow ::slotted(selectors) #113

JamesXNelson commented Feb 27, 2017

iflan commented Mar 1, 2017

iflan commented Mar 1, 2017

JamesXNelson commented Mar 5, 2017

JamesXNelson commented Mar 5, 2017 •

edited

Loading

iflan left a comment

iflan Mar 6, 2017

JamesXNelson Mar 8, 2017 •

edited

Loading

JamesXNelson Mar 8, 2017

iflan Mar 6, 2017

iflan Mar 6, 2017

JamesXNelson Mar 8, 2017

JamesXNelson commented Nov 20, 2017

JamesXNelson commented Nov 20, 2017

Jauhen commented Apr 16, 2018

ghost commented May 22, 2020

		@@ -707,6 +707,8 @@ PARSER_END(GssParserCC)
		// Special handling needed because ':not' takes simple selectors as

Allow ::slotted(selectors) #113

Are you sure you want to change the base?

Allow ::slotted(selectors) #113

Conversation

JamesXNelson commented Feb 27, 2017

iflan commented Mar 1, 2017

iflan commented Mar 1, 2017

JamesXNelson commented Mar 5, 2017

JamesXNelson commented Mar 5, 2017 • edited Loading

iflan left a comment

Choose a reason for hiding this comment

iflan Mar 6, 2017

Choose a reason for hiding this comment

JamesXNelson Mar 8, 2017 • edited Loading

Choose a reason for hiding this comment

JamesXNelson Mar 8, 2017

Choose a reason for hiding this comment

iflan Mar 6, 2017

Choose a reason for hiding this comment

iflan Mar 6, 2017

Choose a reason for hiding this comment

JamesXNelson Mar 8, 2017

Choose a reason for hiding this comment

JamesXNelson commented Nov 20, 2017

JamesXNelson commented Nov 20, 2017

Jauhen commented Apr 16, 2018

ghost commented May 22, 2020

JamesXNelson commented Mar 5, 2017 •

edited

Loading

JamesXNelson Mar 8, 2017 •

edited

Loading