Fixing positioning issue #873

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@vbichkovsky
Contributor

When there are some repeat-x images in the sprite and some other images have position set to 100%, there is a gap between sprite's right side and images with position = 100% (because width-fix was applied after they had been positioned).
I tried to figure out how to write a test for such case and I must say that your tests are difficult to comprehend ;)

@scottdavis

awesome thanks! can you write a test for this? also in the future could you make pull requests against the master branch!

Owner

Well, I just made a quick fix for myself and decided I should share it this way.
I can formulate the test - it should use a directory with several x-repeated images of different width + some regular wide image and check if sprite map's width is correct. But I'm not sure where to place this test in your test infrastructure.
As for the master branch - sprite map width is calculated in some other place there, and my fix is for version 0.11.5. (which I am currently using).

so to be clear what your doing here is bumping the total with to be a factor of the image your repeating? need to implement this in master

@chriseppstein
Member

@scottdavis can you review and merge when convenient?

@scottdavis scottdavis commented on the diff May 28, 2012
lib/compass/sass_extensions/sprites/sprite_methods.rb
@@ -1,3 +1,5 @@
+require 'rational'
@scottdavis
scottdavis May 28, 2012 Member

Do we need this require?

@vbichkovsky
vbichkovsky May 29, 2012 Contributor

Nope, it just happened that my commit contained these changes and I created a pull request from it. You only need to change layout_methods.rb

@scottdavis
scottdavis May 31, 2012 Member

manually changing something kinda makes using pull requests pointless =p

@chriseppstein
Member

@scottdavis what's the status here? Are you holding this up so that a single require can be removed? >_<

@scottdavis
Member

No I'm holding it up because I'm busy I'll get to it this week I'm going to cherry-pick it into master

Sent from my iPhone

On Jun 4, 2012, at 2:28 AM, Chris Eppsteinreply@reply.github.com wrote:

@scottdavis what's the status here? Are you holding this up so that a single require can be removed? >_<


Reply to this email directly or view it on GitHub:
#873 (comment)

@scottdavis
Member

merged 5e42b8f

@scottdavis scottdavis closed this Jun 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment