Skip to content

feat: add IE filter support (AlphaImageLoader) #638

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

Closed
wants to merge 3 commits into from

Conversation

SmileSmith
Copy link

What kind of change does this PR introduce?
support ie filter.

since IE not supprt background-size. we usually use filter to polyfill background image;
background: url('../images/home@1x.png') no-repeat center; background-size: 100% 100%; filter: progid:DXImageTransform.Microsoft.AlphaImageLoader(src='../images/home@1x.png', sizingMethod='scale');

If relevant, did you update the README?

Summary
for css-loader to support ie filter

Does this PR introduce a breaking change?
No

Other information

@jsf-clabot
Copy link

jsf-clabot commented Dec 3, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Dec 3, 2017

Codecov Report

Merging #638 into master will decrease coverage by 2.33%.
The diff coverage is 10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #638      +/-   ##
==========================================
- Coverage   98.92%   96.58%   -2.34%     
==========================================
  Files          10       10              
  Lines         371      381      +10     
  Branches       87       90       +3     
==========================================
+ Hits          367      368       +1     
- Misses          4       13       +9
Impacted Files Coverage Δ
lib/processCss.js 93.16% <10%> (-5.51%) ⬇️

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 05c36db...9478f5e. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 3, 2017

Codecov Report

Merging #638 into master will decrease coverage by 0.49%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #638     +/-   ##
=========================================
- Coverage   98.92%   98.42%   -0.5%     
=========================================
  Files          10       10             
  Lines         371      381     +10     
  Branches       87       90      +3     
=========================================
+ Hits          367      375      +8     
- Misses          4        6      +2
Impacted Files Coverage Δ
lib/processCss.js 97.51% <80%> (-1.16%) ⬇️

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 05c36db...8644dc2. Read the comment docs.

@SmileSmith SmileSmith force-pushed the master branch 3 times, most recently from 44d2b63 to 11d81df Compare December 3, 2017 08:59
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@alexander-akait
Copy link
Member

@michael-ciniawsky friendly ping

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

I have mixed feelings about adding this tbh

@SmileSmith Which version of IE do you need to support atm (is this needed for) ?

@@ -97,6 +97,20 @@ var parserPlugin = postcss.plugin("css-loader-parser", function(options) {
item.nodes.forEach(processNode);
break;
case "item":
if (item.name.indexOf('progid:DXImageTransform.Microsoft.AlphaImageLoader') >= 0){
delete item.innerSpacingBefore;
delete item.innerSpacingAfter;
Copy link
Member

Choose a reason for hiding this comment

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

\n

if (item.name.indexOf('progid:DXImageTransform.Microsoft.AlphaImageLoader') >= 0){
delete item.innerSpacingBefore;
delete item.innerSpacingAfter;
var matchSrc = item.name.match(/src=['"]([^'"]+)['"]/);
Copy link
Member

Choose a reason for hiding this comment

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

matchSrc => match && \n

delete item.innerSpacingBefore;
delete item.innerSpacingAfter;
var matchSrc = item.name.match(/src=['"]([^'"]+)['"]/);
if (!matchSrc) break;
Copy link
Member

Choose a reason for hiding this comment

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

\n

delete item.innerSpacingAfter;
var matchSrc = item.name.match(/src=['"]([^'"]+)['"]/);
if (!matchSrc) break;
var srcUrl = matchSrc[1];
Copy link
Member

Choose a reason for hiding this comment

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

srcUrl => url && \n

var srcUrl = matchSrc[1];
if (options.url && !/^#/.test(srcUrl) && (isAlias(srcUrl) || loaderUtils.isUrlRequest(srcUrl, options.root))) {
item.name = item.name.replace(srcUrl, "___CSS_LOADER_URL___" + urlItems.length + "___");
urlItems.push({
Copy link
Member

Choose a reason for hiding this comment

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

urlItems.push({ url: url })

urlItems.push({
url: srcUrl
});
}
Copy link
Member

Choose a reason for hiding this comment

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

\n

@@ -97,6 +97,20 @@ var parserPlugin = postcss.plugin("css-loader-parser", function(options) {
item.nodes.forEach(processNode);
break;
case "item":
if (item.name.indexOf('progid:DXImageTransform.Microsoft.AlphaImageLoader') >= 0){
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment (in brief) here, describing what the following code/case is doing

@SmileSmith
Copy link
Author

@michael-ciniawsky thanks,

AlphaImageLoader support Sizing background in IE-8 and below
https://msdn.microsoft.com/en-us/library/ms532920(v=vs.85).aspx
https://caniuse.com/#search=background-size

and these code will process it's 'src:...' like bacdground: url(...)

in IE9 and above, we can use background: url();backgroud-size: .. ; to achieve

@michael-ciniawsky michael-ciniawsky changed the title support ie filter of AlphaImageLoader feat: add IE filter support (AlphaImageLoader) Jan 4, 2018
@michael-ciniawsky
Copy link
Member

Sry but I have to declined this since css-loader v1.0.0 will be a complete overhaul of the loader starting with a minimal design

@michael-ciniawsky michael-ciniawsky removed this from the 0.29.0 milestone Jan 4, 2018
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.

4 participants