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

Remove unit from 0rem #186

Closed
XhmikosR opened this issue Nov 25, 2013 · 15 comments
Closed

Remove unit from 0rem #186

XhmikosR opened this issue Nov 25, 2013 · 15 comments
Assignees
Milestone

Comments

@XhmikosR
Copy link
Contributor

I just noticed this, we need to test it works as expected.

@GoalSmashers
Copy link
Contributor

It should be fine but let's check it (+ how it behaves in IE<9 which do not support rems).

@ghost ghost assigned GoalSmashers Nov 26, 2013
@tomByrer
Copy link
Contributor

tomByrer commented Dec 4, 2013

note: If you happen to also remove % from 0%, please ensure it does not drop the symbol from 0% { transform:. It will cause world-flooding (ok just some confused/mad programmers)

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 4, 2013

@tomByrer: it's irrelevant to this... Plus, #182

@GoalSmashers
Copy link
Contributor

@tomByrer we'll make sure!

@GoalSmashers
Copy link
Contributor

@XhmikosR the scenario re IE<9 I was thinking about is as follows:

div { margin: 100px }
…
div { margin: 0rem }

on IE<9 2nd declaration will be dropped thus div will have a margin of 100px, however if we turn 0rem to 0 the 2nd declaration will be taken into account thus margin will be 0.

This is unlikely however we may need a switch to disable it. Thoughts?

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Dec 4, 2013

That's an edge case indeed; a switch sounds good. But in order not to bloat the options, how about renaming selectors-merge-mode to a more universal name and put this in there too?

@GoalSmashers
Copy link
Contributor

Yup, we should one general - "IE<9 compatibility mode".

@GoalSmashers
Copy link
Contributor

@XhmikosR feel free to give f-rems-186 branch a try. It should be all good.

@XhmikosR
Copy link
Contributor Author

Looks good but maybe you could make an improvement for the compatibility option.

If no fallback is specified, remove the unit as usual.

@GoalSmashers
Copy link
Contributor

@XhmikosR Maybe I'm missing something but if no compatibility option is given then units will be removed as before.

@XhmikosR
Copy link
Contributor Author

I mean the case when compatibility is specified and there's no fallback. In that case it makes sense to remove the unit, right?

@GoalSmashers
Copy link
Contributor

Good point, there should be.

@GoalSmashers
Copy link
Contributor

This should be fine now.

@XhmikosR
Copy link
Contributor Author

I can try it tomorrow again and let you know, but if it works for you I see why it wouldn't for me too 👍

@GoalSmashers
Copy link
Contributor

That's a minor thing so let me merge it. Thanks for taking a look at the code!

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

No branches or pull requests

3 participants