Fixed and issue with the options keyword in the Sprite::Base class. it was casing the passed options to be reset to an empty hash. I renamed it to kwargs for consistancy. I also refactored my previous changes to be unobtrusive to people currently using the sprite-map function without an @import.

This commit is contained in:
Scott Davis 2011-05-10 21:04:57 -04:00
parent 4321fc0cf5
commit 325306dcec
6 changed files with 23 additions and 21 deletions

View File

@ -19,11 +19,11 @@ module Compass::SassExtensions::Functions::Sprites
# #
# The sprite map object will generate the sprite map image, if necessary, # The sprite map object will generate the sprite map image, if necessary,
# the first time it is converted to a url. Simply constructing it has no side-effects. # the first time it is converted to a url. Simply constructing it has no side-effects.
def sprite_map(glob, cleanup, kwargs = {}) def sprite_map(glob, kwargs = {})
kwargs.extend VariableReader kwargs.extend VariableReader
Compass::SassExtensions::Sprites::Base.from_uri(glob, self, kwargs, cleanup) Compass::SassExtensions::Sprites::Base.from_uri(glob, self, kwargs)
end end
Sass::Script::Functions.declare :sprite_map, [:glob, :cleanup], :var_kwargs => true Sass::Script::Functions.declare :sprite_map, [:glob], :var_kwargs => true
# Returns the image and background position for use in a single shorthand property: # Returns the image and background position for use in a single shorthand property:
# #

View File

@ -6,13 +6,12 @@ module Compass
# Initialize a new aprite object from a relative file path # Initialize a new aprite object from a relative file path
# the path is relative to the <tt>images_path</tt> confguration option # the path is relative to the <tt>images_path</tt> confguration option
def self.from_uri(uri, context, kwargs, cleanup = Sass::Script::Bool.new(true)) def self.from_uri(uri, context, kwargs)
sprite_map = ::Compass::SpriteMap.new(uri.value, {}) sprite_map = ::Compass::SpriteMap.new(uri.value, {})
sprites = sprite_map.files.map do |sprite| sprites = sprite_map.files.map do |sprite|
sprite.gsub(Compass.configuration.images_path+"/", "") sprite.gsub(Compass.configuration.images_path+"/", "")
end end
new(sprites, sprite_map, context, cleanup, kwargs) new(sprites, sprite_map, context, kwargs)
end end
# Loads the sprite engine # Loads the sprite engine
@ -24,18 +23,22 @@ module Compass
# We should do so only when the packing algorithm changes # We should do so only when the packing algorithm changes
SPRITE_VERSION = "1" SPRITE_VERSION = "1"
attr_accessor :image_names, :path, :name, :options, :map, :cleanup attr_accessor :image_names, :path, :name, :map, :kwargs
attr_accessor :images, :width, :height attr_accessor :images, :width, :height
def initialize(image_names, map, context, cleanup, options) def initialize(sprites, sprite_map, context, kwargs)
require_engine! require_engine!
@image_names, @path, @name, @options, @cleanup = image_names, map.path, map.name, options, cleanup @image_names = sprites
@path = sprite_map.path
@name = sprite_map.name
@kwargs = kwargs
@kwargs['cleanup'] ||= Sass::Script::Bool.new(true)
@images = nil @images = nil
@width = nil @width = nil
@height = nil @height = nil
@evaluation_context = context @evaluation_context = context
@map = map @map = sprite_map
validate! validate!
compute_image_metadata! compute_image_metadata!
end end
@ -58,7 +61,7 @@ module Compass
# Creates the Sprite::Image objects for each image and calculates the width # Creates the Sprite::Image objects for each image and calculates the width
def init_images def init_images
@images = image_names.collect do |relative_file| @images = image_names.collect do |relative_file|
image = Compass::SassExtensions::Sprites::Image.new(self, relative_file, options) image = Compass::SassExtensions::Sprites::Image.new(self, relative_file, kwargs)
@width = [ @width, image.width + image.offset ].max @width = [ @width, image.width + image.offset ].max
image image
end end
@ -118,7 +121,7 @@ module Compass
# Generate a sprite image if necessary # Generate a sprite image if necessary
def generate def generate
if generation_required? if generation_required?
if @cleanup.value if kwargs.get_var('cleanup').value
cleanup_old_sprites cleanup_old_sprites
end end
sprite_data = construct_sprite sprite_data = construct_sprite
@ -183,7 +186,7 @@ module Compass
to_s to_s
end end
def to_s(options = self.options) def to_s(kwargs = self.kwargs)
sprite_url(self).value sprite_url(self).value
end end

View File

@ -76,7 +76,7 @@ $#{name}-repeat: no-repeat !default;
$#{name}-prefix: '' !default; $#{name}-prefix: '' !default;
$#{name}-clean-up: true !default; $#{name}-clean-up: true !default;
#{skip_overrides ? "$#{name}-sprites: sprite-map(\"#{uri}\", $#{name}-clean-up);" : generate_overrides } #{skip_overrides ? "$#{name}-sprites: sprite-map(\"#{uri}\", $cleanup: $#{name}-clean-up);" : generate_overrides }
// All sprites should extend this class // All sprites should extend this class
// The #{name}-sprite mixin will do so for you. // The #{name}-sprite mixin will do so for you.
@ -130,7 +130,7 @@ $#{name}-#{sprite_name}-repeat: $#{name}-repeat !default;
SCSS SCSS
end.join end.join
content += "\n$#{name}-sprites: sprite-map(\"#{uri}\", $#{name}-clean-up,\n" content += "\n$#{name}-sprites: sprite-map(\"#{uri}\",\n$cleanup: $#{name}-clean-up,\n"
content += sprite_names.map do |sprite_name| content += sprite_names.map do |sprite_name|
%Q{ $#{sprite_name}-position: $#{name}-#{sprite_name}-position, %Q{ $#{sprite_name}-position: $#{name}-#{sprite_name}-position,
$#{sprite_name}-spacing: $#{name}-#{sprite_name}-spacing, $#{sprite_name}-spacing: $#{name}-#{sprite_name}-spacing,

View File

@ -259,7 +259,7 @@ class SpritesTest < Test::Unit::TestCase
it "should use position adjustments in functions" do it "should use position adjustments in functions" do
css = render <<-SCSS css = render <<-SCSS
$squares: sprite-map("squares/*.png", true, $position: 100%); $squares: sprite-map("squares/*.png", $position: 100%);
.squares-sprite { .squares-sprite {
background: $squares no-repeat; background: $squares no-repeat;
} }
@ -411,7 +411,7 @@ class SpritesTest < Test::Unit::TestCase
it "should work even if @import is missing" do it "should work even if @import is missing" do
css = render <<-SCSS css = render <<-SCSS
.squares { .squares {
background: sprite(sprite-map("squares/*.png", true), twenty-by-twenty) no-repeat; background: sprite(sprite-map("squares/*.png"), twenty-by-twenty) no-repeat;
} }
SCSS SCSS
assert_correct css, <<-CSS assert_correct css, <<-CSS

View File

@ -11,13 +11,13 @@ class SpritesBaseTest < Test::Unit::TestCase
config.images_path = @images_tmp_path config.images_path = @images_tmp_path
Compass.add_configuration(config) Compass.add_configuration(config)
Compass.configure_sass_plugin! Compass.configure_sass_plugin!
@options = {} @options = {'cleanup' => Sass::Script::Bool.new(true)}
setup_map setup_map
end end
def setup_map def setup_map
@map = Compass::SpriteMap.new("selectors/*.png", @options) @map = Compass::SpriteMap.new("selectors/*.png", @options)
@base = Compass::SassExtensions::Sprites::Base.new(@map.sprite_names.map{|n| "selectors/#{n}.png"}, @map, @map.sass_engine, Sass::Script::Bool.new(true), @map.options) @base = Compass::SassExtensions::Sprites::Base.new(@map.sprite_names.map{|n| "selectors/#{n}.png"}, @map, @map.sass_engine, @map.options)
end end
def teardown def teardown
@ -82,7 +82,6 @@ class SpritesBaseTest < Test::Unit::TestCase
file_to_remove = File.join(@images_tmp_path, 'selectors', 'ten-by-ten.png') file_to_remove = File.join(@images_tmp_path, 'selectors', 'ten-by-ten.png')
FileUtils.rm file_to_remove FileUtils.rm file_to_remove
assert !File.exists?(file_to_remove), "Failed to remove sprite file" assert !File.exists?(file_to_remove), "Failed to remove sprite file"
@options["cleanup"] = true
setup_map setup_map
@base.generate @base.generate
assert !File.exists?(file), "Sprite file did not get removed" assert !File.exists?(file), "Sprite file did not get removed"

View File

@ -20,7 +20,7 @@ class SpritesImageTest < Test::Unit::TestCase
def parent def parent
map = Compass::SpriteMap.new("selectors/*.png", options) map = Compass::SpriteMap.new("selectors/*.png", options)
@parent ||= Compass::SassExtensions::Sprites::Base.new(map.sprite_names.map{|n| "selectors/#{n}.png"}, map, map.sass_engine, Sass::Script::Bool.new(true), map.options) @parent ||= Compass::SassExtensions::Sprites::Base.new(map.sprite_names.map{|n| "selectors/#{n}.png"}, map, map.sass_engine, map.options)
end end
let(:options) do let(:options) do