Skip to content

Conversation

@e111077
Copy link
Contributor

@e111077 e111077 commented Nov 28, 2018

google-chart never worked in shadow dom. I have cloned the global link[rel="stylesheet"]#load-css-* into each instance root. This should make the styles available to each shadow root. Demos now work.

#load-css-* because the format of google chart styles are #load-css-0, #load-css-1, etc.

@e111077 e111077 requested a review from aomarks November 28, 2018 20:56
@aomarks
Copy link

aomarks commented Nov 28, 2018

Nice!

@e111077
Copy link
Contributor Author

e111077 commented Nov 28, 2018

Put it all under one for loop and moved it all to a function and made sure it only does it once since it is in a _typeChanged function


var stylesheetsArray = Array.from(stylesheets);

for (var i = 0; i < stylesheetsArray.length; i++) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.forEach should be safe for both Array and NodeList

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh not for IE11 though maybe?

@aomarks
Copy link

aomarks commented Nov 28, 2018

Put it all under one for loop and moved it all to a function and made sure it only does it once since it is in a _typeChanged function

Nice, looks great.

* Queries global document head for google charts link#load-css-* and clones
* them into the local root's div#styles element for shadow dom support.
*/
_localizeGlobalStylesheets: function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged this on behalf of an internal request from a blocked user. I'd 100% be willing to review your import CL over this implementation.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants