Update lib/compass/sass_extensions/functions/gradient_support.rb #1161

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

dalgard commented Feb 5, 2013

Correction: Resulted in a gradient 100px wide instead of 100%

Update lib/compass/sass_extensions/functions/gradient_support.rb
Correction: Resulted in a gradient 100px wide instead of 100%
Owner

scottdavis commented Feb 22, 2013

You doing to have to be more detailed about what caused this change and what the expected output was. The fact that there is not a test case makes me nervous.

dalgard commented Feb 28, 2013

Sorry for the delay. The module generates the SVG based gradients used for (experimental) compatibility with IE9 and less. Radial-gradients is a special case; it is probably not used much, which must be why this very small error has gone unnoticed.

As you can see in the diff, I've just made a tiny correction - it is totally clear from the code that this was supposed to be a percentage, like in the other routines, and the change is only reflected in the SVG string directly (which has that character added, making the gradient fill up the entire image instead of an absolute value of 100px, which doesn't make sense).

If you ask the person who has written this part of the code, they will tell you right away that this is how it's supposed to be. This may be the most trivial correction ever submitted on GitHub and as a contribution it looks kinda silly, but it did help me in my project :)

I'm running into this problem and can confirm that dalgard's patch fixes it.

Owner

scottdavis commented Apr 19, 2013

This has been rewritten in the latest alpha

Sent from my iPhone

On Apr 19, 2013, at 6:57 PM, andymesa notifications@github.com wrote:

I'm running into this problem and can confirm that dalgard's patch fixes it. I really hope you guys accept this, because my team is against me patching compass (as minuscule a change as it might be).


Reply to this email directly or view it on GitHub.

Do you mean in the next alpha? Because it's still an issue in 0.13.alpha.4 (that is fixed by adding back the %)

Also, thanks for the quick response!

Owner

scottdavis commented Apr 19, 2013

Ok

Sent from my iPhone

On Apr 19, 2013, at 7:10 PM, andymesa notifications@github.com wrote:

Do you mean in the next alpha? Because it's still an issue in 0.13.alpha.4 (that is fixed by adding back the %)


Reply to this email directly or view it on GitHub.

Owner

chriseppstein commented Dec 4, 2013

I think this patch should have fixed it: chriseppstein#1436

it landed a couple days ago. Please confirm. closing for now.

@dalgard dalgard deleted the dalgard:patch-1 branch Dec 8, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment