Add support for distributed children in app-grid #396

Open
wants to merge 1 commit into
from

Projects

None yet

3 participants

@ahaasler
ahaasler commented Jan 6, 2017

This fixes #394 by adding :host ::content to the app-grid-style selectors following instructions in https://www.polymer-project.org/1.0/docs/devguide/styling#styling-distributed-children-content.

I had to keep the previous selectors because tests didn't pass otherwise. If someone wants to check it or knows why that's happening my unsquashed commits are at https://github.com/ahaasler/app-layout/commits/distributed-children-app-grid-style. The first commit causes the app-grid tests to fail.

@ahaasler ahaasler Add support for distributed children in app-grid
Distributed children test and demo has been added.
c9e354b
@googlebot googlebot added the cla: yes label Jan 6, 2017
@@ -121,7 +121,8 @@
<template>
<style>
- :host {
+ :host,
+ :host ::content {
@blasten
blasten Jan 6, 2017 edited Member

It can just be ::content. Similar comment for the rest of the selectors.

@ahaasler
ahaasler Jan 7, 2017

While I was working on the PR I discovered that using ::content without :root works, but should it?

In the Polymer docs (link above) this can be seen:

Under shady DOM, the <content> tag doesn't appear in the DOM tree. Styles are rewritten to remove the ::content pseudo-element, and any combinator immediately to the left of ::content.

This implies:

  • You must have a selector to the left of the ::content pseudo-element.

    :host ::content div
    

Also, in some code samples there's this clarification:

/* styling elements distributed to content (via ::content) requires */
/* selecting the parent of the <content> element for compatibility with */
/* shady DOM . This can be :host or a wrapper element. */

I already have this change in my workspace, and the tests and demos all work as expected, but should I really submit this change?

@blasten

Thanks for the PR @ahaasler. Just a minor comment.

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