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.

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

Inject styles into Shadow DOM when inside Shadow DOM (#5763) #6556

Merged

Conversation

Simon-Tang
Copy link
Contributor

@Simon-Tang Simon-Tang commented Oct 6, 2019

  • Injects the inline <style> used by ChartJS into document.head and each Shadow Root as needed
  • Adds a node[EXPANDO]: { containsStyles: boolean } to document.head and each Shadow Root that contains a ChartJS instance
  • Removes the flags platform._style and platform._loaded since we now need to keep track of multiple <style> insertions

To-do(?):

  • Karma tests to test responsive behaviour inside Web Components
  • Update documentation to include the need for manually including the <script> inside Shadow DOMs in the case of disableCSSInjection: true - example workaround

Closes: #5763

@kurkle
Copy link
Member

kurkle commented Oct 7, 2019

Looks good!

Can you add a test pen where the functionality can be verified? (Fork https://stackblitz.com/edit/polymer310-testcase-chartjs for example)

@Simon-Tang
Copy link
Contributor Author

Thanks @kurkle. I'm not sure what's up with the error I'm getting when using customElements but here is what I've been using for testing:

https://stackblitz.com/edit/polymer310-testcase-chartjs-zxndkp

I copied my exact testing page into index2.html in the above. I'm getting this error:

Error: Failed to construct 'HTMLElement': Please use the 'new' operator, this DOM object constructor cannot be called as a function.

If any reviewer has the patience to help out with this I would appreciate it a lot! I'm not really sure if I'm missing something obvious to make this work. However I would expect the pen you linked (https://stackblitz.com/edit/polymer310-testcase-chartjs) to also work with the changes in this PR.

Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

@etimberg etimberg merged commit 0b62f28 into chartjs:master Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chart does not resize inside shadow DOM (e.g. Polymer)
3 participants