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

Possibility to specify type attribute #163

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BilyachenkoOY
Copy link

@BilyachenkoOY BilyachenkoOY commented Jun 25, 2019

Now it will be possible to specify type attribute of <style>.
This will allow to fix warning from W3C validator "The type attribute for the style element is not needed and should be omitted.".

Now it will be possible to specify or type attribute of <style>
@codecov-io
Copy link

codecov-io commented Jun 25, 2019

Codecov Report

Merging #163 into master will increase coverage by 0.29%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #163      +/-   ##
==========================================
+ Coverage   80.95%   81.25%   +0.29%     
==========================================
  Files           4        4              
  Lines          63       64       +1     
  Branches       15       17       +2     
==========================================
+ Hits           51       52       +1     
  Misses         11       11              
  Partials        1        1
Impacted Files Coverage Δ
src/insertCss.js 75.6% <100%> (+0.6%) ⬆️

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 a990d94...4c2e321. Read the comment docs.

@frenzzy
Copy link
Member

frenzzy commented Jun 26, 2019

Why not just remove this line at all?

elem.setAttribute('type', 'text/css')

@BilyachenkoOY
Copy link
Author

BilyachenkoOY commented Jun 26, 2019

For backward compatibility at least

@BilyachenkoOY
Copy link
Author

@frenzzy Any news on this PR?

@frenzzy
Copy link
Member

frenzzy commented Aug 9, 2019

@BilyachenkoOY sorry, I am not interested in maintenance isomorphic-style-loader anymore since I switched to using Emotion.

@BilyachenkoOY
Copy link
Author

@koistya can you help with this? :)

Copy link
Collaborator

@dazlious dazlious left a comment

Choose a reason for hiding this comment

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

Just a minor change regarding the code format. Please add at least one test for this in order to allow future maintenance

Comment on lines +69 to +70
if (type === undefined || typeof type === 'string')
elem.setAttribute('type', type || 'text/css')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (type === undefined || typeof type === 'string')
elem.setAttribute('type', type || 'text/css')
if (!type || typeof type === 'string') {
elem.setAttribute('type', type || 'text/css')
}

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.

4 participants