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

Add support for Shadow DOM #5585

Merged
merged 4 commits into from
Jul 7, 2018
Merged

Add support for Shadow DOM #5585

merged 4 commits into from
Jul 7, 2018

Conversation

reda-alaoui
Copy link
Contributor

@reda-alaoui reda-alaoui commented Jun 21, 2018

@etimberg etimberg requested a review from simonbrunel June 23, 2018 14:50
@simonbrunel simonbrunel changed the title [BUG] Shadow DOM & Root: Failed to execute 'getComputedStyle' on 'Window' Add support for Shadow DOM Jun 24, 2018
Copy link
Member

@simonbrunel simonbrunel left a comment

Choose a reason for hiding this comment

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

Is it the only changes needed to support shadow DOM?

@@ -481,8 +481,15 @@ module.exports = function() {

return padding.indexOf('%') > -1 ? parentDimension / parseInt(padding, 10) : parseInt(padding, 10);
};
helpers.getParentNode = function(domNode) {
Copy link
Member

Choose a reason for hiding this comment

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

This method should be private (and at some point will be moved in the dom platform implementation):

/**
 * @private
 */
helpers._getParentNode = function(node) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return;
}

var shadow = div.attachShadow({mode: 'closed'});
Copy link
Member

Choose a reason for hiding this comment

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

Do we reach this point when tests are executed by our CI / Travis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes in Chrome, not in Firefox.

@reda-alaoui
Copy link
Contributor Author

reda-alaoui commented Jul 5, 2018

Is it the only changes needed to support shadow DOM?

IMO, yes since other features are working correctly in my app.
@simonbrunel, can you review again please?

@simonbrunel simonbrunel merged commit 387a23d into chartjs:master Jul 7, 2018
@simonbrunel
Copy link
Member

Thanks @reda-alaoui

@simonbrunel simonbrunel added this to the Version 2.8 milestone Jul 7, 2018
@reda-alaoui reda-alaoui deleted the bugfix/5583 branch July 7, 2018 16:23
@royvelich
Copy link

Is this fix already available in the latest stable version?

gofreddo added a commit to gofreddo/vue-chartjs that referenced this pull request Oct 31, 2018
Update package.json.  Bump chart.js version to 2.7.3.  

We're using vue-chartjs in a web component.   Shadow DOM support was added in Version 2.7.3 chartjs/Chart.js#5585 .
apertureless pushed a commit to apertureless/vue-chartjs that referenced this pull request Nov 8, 2018
Update package.json.  Bump chart.js version to 2.7.3.  

We're using vue-chartjs in a web component.   Shadow DOM support was added in Version 2.7.3 chartjs/Chart.js#5585 .



### Fix or Enhancement?


- [ ] All tests passed


### Environment
- OS: Write here
- NPM Version: Write here
onlinehub0808 added a commit to onlinehub0808/vueChartjs that referenced this pull request Feb 2, 2023
Update package.json.  Bump chart.js version to 2.7.3.  

We're using vue-chartjs in a web component.   Shadow DOM support was added in Version 2.7.3 chartjs/Chart.js#5585 .



### Fix or Enhancement?


- [ ] All tests passed


### Environment
- OS: Write here
- NPM Version: Write here
lu0415mc added a commit to lu0415mc/vue-chartjs that referenced this pull request Feb 24, 2024
Update package.json.  Bump chart.js version to 2.7.3.  

We're using vue-chartjs in a web component.   Shadow DOM support was added in Version 2.7.3 chartjs/Chart.js#5585 .



### Fix or Enhancement?


- [ ] All tests passed


### Environment
- OS: Write here
- NPM Version: Write here
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.

3 participants