@Import Google Charts api Styles #38
Conversation
|
Is this PR obsolete? |
| //Add the stylesheet as an @import | ||
| var cssImport = document.createElement('style'); | ||
| cssImport.innerHTML = "@import '" + newNode.href + "';"; | ||
| element.shadowRoot.appendChild(cssImport); |
There was a problem hiding this comment.
this should check that the stylesheet is from the correct url, when one/they are found, the MO should disconnect.
There was a problem hiding this comment.
@philipwalton wdyt about this approach instead of your own PR that ads the styles internal to the shadow dom. What I like about this (despite being a bit hacky) is that the styles will always be up to date from the server.
There was a problem hiding this comment.
I like the idea of keeping them up-to-date. I almost find this to be less hacky than the other approach. However, it could use some more organization.
There was a problem hiding this comment.
Doesn't this just append any dynamically added stylesheet to the element's shadow DOM? I don't think this is a viable option unless we can reliably identify gviz stylesheets only.
Also, I think gviz only adds the stylesheets once, for the first chart. This means the mutation observer for <google-charts> added after the gviz style are added to the <head> won't pick up these styles.
There was a problem hiding this comment.
Hence my comment about better checking that the stylesheet that was added is actually from the origin you care about.
There was a problem hiding this comment.
Hence my comment about better checking that the stylesheet that was added is actually from the origin you care about.
Ahh, yes, didn't see that comment...
If a known path that only applies to gviz stylesheets can work, I think the best approach would be to create a single style manager object that keeps track of what gviz styles exists in the DOM, and then the individual charts instances could query that info from the style manager on attached and subscribe to changes.
There was a problem hiding this comment.
Another option would be to work the the gviz team to have them expose the stylesheet URLs associated with each individual chart type. This is probably the best long-term strategy, as it guarantees we'd always been in sync.
There could also be a load configuration option that tells gviz libraries not to load their own styles, and to let consumer take care of it. This would prevent unnecessary styles from existing on the page.
|
So, I just realized this is very similar to how I just implemented here. However, a mutation observer isn't necessary. From my investigation into the gViz library code, the loaded callback runs after the CSS styles have been added to the head. That means that on each load callback, we should be in the clear to import the styles. In my change, I currently do the import when the chart fires the ready event. This means that chart type changes are also taken into account (however, is seems the ready event is firing long after the chart has been drawn...). I'm identifying gviz CSS with a selector at that point. |
|
It has been fixed in #223 Tested by adding |
required for styling of #26
Additionally updated the docs with table and gauge as possible chart type