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

@Import Google Charts api Styles #38

Closed
wants to merge 4 commits into from
Closed

@Import Google Charts api Styles #38

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9424be
observe styles from the Google Chart JS api and @import them in the s…
SmokyBob Mar 20, 2015
90c267a
added table to the docs
SmokyBob Mar 20, 2015
3be074b
added gauge type to the docs
SmokyBob Mar 20, 2015
5bae7be
fixed formatting following giudelines
SmokyBob Mar 26, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion google-chart.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@
*
* Should be one of:
* - `area`, `bar`, `bubble`, `candlestick`, `column`, `combo`, `geo`,
* `histogram`, `line`, `pie`, `scatter`, `stepped-area`
* `histogram`, `line`, `pie`, `scatter`, `stepped-area`, `table`, `gauge`
*
*
*
* See <a href="https://google-developers.appspot.com/chart/interactive/docs/gallery">Google Visualization API reference (Chart Gallery)</a> for details.
*
Expand Down Expand Up @@ -197,6 +199,25 @@
this.options = {};
this.rows = [];
this.dataTable = null;

var head = document.querySelector('head');
var element = this;
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(mutation) {
for (var i = 0; i < mutation.addedNodes.length; i++) {
var newNode = mutation.addedNodes[i];
if (newNode.rel && newNode.rel == 'stylesheet') {
//Add the stylesheet as an @import
var cssImport = document.createElement('style');
cssImport.innerHTML = "@import '" + newNode.href + "';";
element.shadowRoot.appendChild(cssImport);
Copy link
Contributor

Choose a reason for hiding this comment

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

this should check that the stylesheet is from the correct url, when one/they are found, the MO should disconnect.

Copy link
Contributor

Choose a reason for hiding this comment

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

@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.

Copy link
Member

@wesalvaro wesalvaro Jun 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hence my comment about better checking that the stylesheet that was added is actually from the origin you care about.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

}
}
});
});

observer.observe(head, { subtree: false, characterData: false, childList: true });

},

readyForAction: function(e, detail, sender) {
Expand Down