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

allow SVG elements to have scoped CSS #1226

Merged
merged 3 commits into from
Mar 12, 2018
Merged

allow SVG elements to have scoped CSS #1226

merged 3 commits into from
Mar 12, 2018

Conversation

Rich-Harris
Copy link
Member

fixes #1224

@codecov-io
Copy link

codecov-io commented Mar 12, 2018

Codecov Report

Merging #1226 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1226      +/-   ##
==========================================
+ Coverage   91.67%   91.67%   +<.01%     
==========================================
  Files         126      126              
  Lines        4575     4576       +1     
  Branches     1501     1502       +1     
==========================================
+ Hits         4194     4195       +1     
  Misses        158      158              
  Partials      223      223
Impacted Files Coverage Δ
src/generators/nodes/Attribute.ts 85.64% <100%> (ø) ⬆️
src/generators/nodes/Element.ts 94.62% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9136d9...6131963. Read the comment docs.

@@ -216,7 +216,9 @@ export default class Element extends Node {
if (this._needsCssAttribute && !this.generator.customElement) {
if (!this.attributes.find(a => a.type === 'Attribute' && a.name === 'class')) {
block.builders.hydrate.addLine(
`${name}.className = "${this.generator.stylesheet.id}";`
this.namespace === namespaces.svg
Copy link
Member

Choose a reason for hiding this comment

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

Elsewhere I think we always use setAttribute if any namespace is set at all, not just if the SVG one is set. I'm really not sure what the practical difference is, but do we want to do the same thing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, you're right — I don't think it does make any practical difference (idk, MathML?) but consistency is valuable. updated

@Rich-Harris Rich-Harris merged commit 26c5982 into master Mar 12, 2018
@Rich-Harris Rich-Harris deleted the gh-1224 branch March 12, 2018 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.className can't be used to set classes on SVG elements
3 participants