From 4aa052d9e43c1fac142b0c8ce4be1ae388274ca3 Mon Sep 17 00:00:00 2001 From: Scott Davis Date: Fri, 12 Aug 2011 15:12:31 -0400 Subject: [PATCH] horizontal layout now respects positions and spacing correctly --- Gemfile.lock | 2 +- lib/compass/sass_extensions/sprites/image.rb | 23 +++--- .../sass_extensions/sprites/layout_methods.rb | 2 +- test/units/sprites/image_test.rb | 75 ++++++++----------- test/units/sprites/sprite_map_test.rb | 24 +++++- 5 files changed, 69 insertions(+), 57 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d9559e14..a4e40f94 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - compass (0.12.0.alpha.0.e49581c) + compass (0.12.0.alpha.0.d8e43bf) chunky_png (~> 1.2) fssm (>= 0.2.7) sass (~> 3.1) diff --git a/lib/compass/sass_extensions/sprites/image.rb b/lib/compass/sass_extensions/sprites/image.rb index fc842b3d..1b41b18f 100644 --- a/lib/compass/sass_extensions/sprites/image.rb +++ b/lib/compass/sass_extensions/sprites/image.rb @@ -48,29 +48,28 @@ module Compass File.basename(relative_file, '.png') end - # Value of $#{name}-repeat or $repeat - def repeat - [ "#{name}-repeat", "repeat" ].each { |which| - if var = options.get_var(which) - return var.value - end - } - "no-repeat" + def get_var_file(var) + options.get_var "#{name}_#{var}" end - # Value of $#{name}-position or $position defaults o 0px + # Value of $#{name}-repeat or $repeat + def repeat + (get_var_file("repeat") || options.get_var("repeat") || Sass::Script::String.new("no-repeat")).value + end + + # Value of $#{name}-position or $position defaults to 0px def position - options.get_var("#{name}-position") || options.get_var("position") || Sass::Script::Number.new(0, ["px"]) + get_var_file("position") || options.get_var("position") || Sass::Script::Number.new(0, ["px"]) end # Offset within the sprite def offset (position.unitless? || position.unit_str == "px") ? position.value : 0 end - + # Spacing between this image and the next def spacing - (options.get_var("#{name}-spacing") || options.get_var("spacing") || Sass::Script::Number.new(0)).value + (get_var_file("spacing") || options.get_var("spacing") || Sass::Script::Number.new(0, ['px'])).value end # MD5 hash of this file diff --git a/lib/compass/sass_extensions/sprites/layout_methods.rb b/lib/compass/sass_extensions/sprites/layout_methods.rb index 2b6a6142..60da675c 100644 --- a/lib/compass/sass_extensions/sprites/layout_methods.rb +++ b/lib/compass/sass_extensions/sprites/layout_methods.rb @@ -81,7 +81,7 @@ module Compass image.top = image.position.unit_str == '%' ? (@height - image.height) * (image.position.value / 100.0) : image.position.value next if index == 0 last_image = @images[index-1] - image.left = last_image.left + last_image.width + [image.offset, last_image.offset].max + image.left = last_image.left + last_image.width + [image.spacing, last_image.spacing].max end end diff --git a/test/units/sprites/image_test.rb b/test/units/sprites/image_test.rb index a3342138..ffac255d 100644 --- a/test/units/sprites/image_test.rb +++ b/test/units/sprites/image_test.rb @@ -6,41 +6,36 @@ class SpritesImageTest < Test::Unit::TestCase include SpriteHelper def setup create_sprite_temp - file = StringIO.new("images_path = #{@images_src_path.inspect}\n") - Compass.add_configuration(file, "sprite_config") - @repeat = 'no-repeat' - @spacing = 0 - @position = 100 - @offset = 100 end - let(:sprite_filename) { 'squares/ten-by-ten.png' } - let(:sprite_path) { File.join(@images_tmp_path, sprite_filename) } - let(:sprite_name) { File.basename(sprite_filename, '.png') } + SPRITE_FILENAME = 'selectors/ten-by-ten.png' - - def options - options = {:offset => @offset} - options.stubs(:get_var).with(anything).returns(nil) - ::OpenStruct.any_instance.stubs(:unitless?).returns(true) - options.stubs(:get_var).with("#{sprite_name}-repeat").returns(::OpenStruct.new(:value => @repeat)) - options.stubs(:get_var).with("#{sprite_name}-spacing").returns(::OpenStruct.new(:value => @spacing)) - options.stubs(:get_var).with("#{sprite_name}-position").returns(::OpenStruct.new(:value => @position)) - options.stubs(:get_var).with("layout").returns(::OpenStruct.new(:value => 'vertical')) - options + def sprite_path + File.join(@images_tmp_path, SPRITE_FILENAME) end - + def sprite_name + File.basename(SPRITE_FILENAME, '.png') + end - let(:digest) { Digest::MD5.file(sprite_path).hexdigest } - - - let(:image) { Compass::SassExtensions::Sprites::Image.new(sprite_map_test(options), File.join(sprite_filename), options)} + def digest + Digest::MD5.file(sprite_path).hexdigest + end + + def test_map(options ={}) + options = {'cleanup' => Sass::Script::Bool.new(true), 'layout' => Sass::Script::String.new('vertical')}.merge(options) + map = sprite_map_test(options) + end + + def test_image(options ={}) + test_map(options).images.first + end test 'initialize' do + image = test_image assert_equal sprite_name, image.name assert_equal sprite_path, image.file - assert_equal sprite_filename, image.relative_file + assert_equal SPRITE_FILENAME, image.relative_file assert_equal 10, image.width assert_equal 10, image.height assert_equal digest, image.digest @@ -49,44 +44,40 @@ class SpritesImageTest < Test::Unit::TestCase end test 'hover' do - assert_equal 'ten-by-ten_hover', image.hover.name + assert_equal 'ten-by-ten_hover', test_image.hover.name end test 'no parent' do - assert_nil image.parent + assert_nil test_image.parent end - - test 'image type is nil' do - @repeat = nil - assert_nil image.repeat - end - test 'image type is "global"' do - @repeat = 'global' - assert_equal @repeat, image.repeat + image = test_image "ten_by_ten_repeat" => Sass::Script::String.new('global') + assert_equal 'global', image.repeat end test 'image type is "no-repeat"' do - assert_equal 'no-repeat', image.repeat + assert_equal 'no-repeat', test_image.repeat end test 'image position' do - assert_equal Sass::Script::Number.new(100, ["px"]).value, image.position.value + image = test_image "ten_by_ten_position" => Sass::Script::Number.new(100, ["px"]) + assert_equal 100, image.position.value end test 'image spacing' do @spacing = 10 - assert_equal @spacing, image.spacing + image = test_image "spacing" => Sass::Script::Number.new(100, ["px"]) + assert_equal 100, image.spacing end test 'offset' do - assert_equal @offset, image.offset + image = test_image "ten_by_ten_position" => Sass::Script::Number.new(100, ["px"]) + assert_equal 100, image.offset end test 'neither, uses 0' do - @offset = 0 - img = image + img = test_image img.position.stubs(:unitless?).returns(false) assert_equal 0, img.offset end @@ -105,7 +96,7 @@ class SpritesImageTest < Test::Unit::TestCase config.images_path = @images_tmp_path config.sprite_load_path = [@images_tmp_path, other_folder] Compass.add_configuration(config, "sprite_config") - image = Compass::SassExtensions::Sprites::Image.new(sprite_map_test(options), "foo/my.png", options) + image = Compass::SassExtensions::Sprites::Image.new(test_map, "foo/my.png", {}) assert_equal File.join(other_folder, 'foo/my.png'), image.file assert_equal 0, image.size FileUtils.rm_rf other_folder diff --git a/test/units/sprites/sprite_map_test.rb b/test/units/sprites/sprite_map_test.rb index 30b601b9..b9f4a0ef 100644 --- a/test/units/sprites/sprite_map_test.rb +++ b/test/units/sprites/sprite_map_test.rb @@ -86,6 +86,16 @@ class SpriteMapTest < Test::Unit::TestCase assert_equal [0, 0, 0, 0], @base.images.map(&:left) end + it "should have a vertical layout with spacing" do + base = sprite_map_test(@options.merge({"spacing" => Sass::Script::Number.new(10, ['px'])})) + assert_equal [0, 20, 40, 60], base.images.map(&:top) + end + + it "should layout vertical with position" do + base = sprite_map_test("ten_by_ten_active_position" => Sass::Script::Number.new(10, ['px'])) + assert_equal [0, 10, 0, 0], base.images.map(&:left) + end + def smart options = @options.merge("layout" => Sass::Script::String.new('smart')) importer = Compass::SpriteImporter.new @@ -122,8 +132,9 @@ class SpriteMapTest < Test::Unit::TestCase end # Horizontal tests - def horizontal + def horizontal(options= {}) opts = @options.merge("layout" => Sass::Script::String.new('horizontal')) + opts.merge!(options) sprite_map_test(opts) end @@ -139,6 +150,17 @@ class SpriteMapTest < Test::Unit::TestCase assert_equal [0, 0, 0, 0], base.images.map(&:top) end + it "should layout horizontaly with spacing" do + base = horizontal("spacing" => Sass::Script::Number.new(10, ['px'])) + assert_equal [0, 20, 40, 60], base.images.map(&:left) + assert_equal [0, 0, 0, 0], base.images.map(&:top) + end + + it "should layout horizontaly with position" do + base = horizontal("ten_by_ten_active_position" => Sass::Script::Number.new(10, ['px'])) + assert_equal [0, 10, 0, 0], base.images.map(&:top) + end + it "should generate a horrizontal sprite" do base = horizontal base.generate