saeidzebardast / nvd3-elements

NVD3 charts as web components for Polymer.
http://saeidzebardast.github.io/nvd3-elements
MIT License
38 stars 13 forks source link

Styles issues since Polymer 1.7 #12

Closed plequang closed 7 years ago

plequang commented 8 years ago

Hi,

Since Polymer 1.7, some of the styles defined in nvd3-shared-styles do not apply anymore.

To reproduce, run bower update in the nvd3-elements repo, then polyserve and go to http://localhost:8080/components/nvd3-elements/demo/nvd3-line.html

The rule that do not apply and that makes the demo look weird is :

Line 289
      :root svg.nvd3-svg {
        ...
        width: 100%;
        height: 100%;
      }

I think this rule do not apply because nvd3-shared-styles is used in a document level custom-style in nvd3-behavior.html :

<style is="custom-style" include="nvd3-shared-styles"></style>

From the doc https://www.polymer-project.org/1.0/docs/devguide/styling#custom-style :

Document styles defined in a custom-style are shimmed to ensure they do not leak into local DOM when running on browsers without native Shadow DOM.

The svg element is in the local DOM, so it seems normal that the rule does not apply (even though it was working before 1.7).

Other css rules seem to work, because they apply on elements added to the DOM by nvd3, so not under the control of Polymer shady DOM polyfill (none of these elements have the style-scope class), but this is not something we should rely on.

Also, using document level styles rules is probably the cause of issue #11, because styles really do not leak into the local DOM from above under shadow DOM.

I think using scoped styles is the path to solve this issue.

I wrote a PR to achieve this and everything seems to work with shady and shadow DOM. Basically, I've included nvd3-shared-styles in every element local DOM style, and rewritten the CSS rules to target the local DOM.

One problem though is the styling of the tooltip. nvd3 adds the tooltip div at the end of the body, so it still need document level styling.

I've tried to make clear my findings, but I'm still confused with all these DOM (shadow/shady/local/light) things. So don't hesitate to correct me if I wrote something wrong.

saeidzebardast commented 7 years ago

I've tested the PR. It works fine. Thanks for the effort.

saeidzebardast commented 7 years ago

It also fixed #11.