From 4d7dc7ad305a2681783619df84aaf6f67832897d Mon Sep 17 00:00:00 2001
From: Scott Davis <jetviper21@gmail.com>
Date: Fri, 25 Mar 2011 17:17:27 -0400
Subject: [PATCH] refactored sprite selectors

---
 Gemfile.lock                                  |  2 +-
 .../compass/utilities/sprites/_base.scss      | 37 +++++++++---------
 .../utilities/sprites/_sprite-img.scss        |  2 +
 .../sass_extensions/functions/sprites.rb      | 38 ++++++++-----------
 lib/compass/sass_extensions/sprites/image.rb  | 15 ++++++--
 .../sass_extensions/sprites/image_spec.rb     | 23 ++++++++++-
 spec/sprites_spec.rb                          | 23 ++++-------
 7 files changed, 78 insertions(+), 62 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 1a8ba66c..09e8eb4c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -7,7 +7,7 @@ GIT
 PATH
   remote: .
   specs:
-    compass (0.11.beta.4.e47fccd)
+    compass (0.11.beta.4.9962a37)
       chunky_png (~> 1.1.0)
       sass (>= 3.1.0.alpha.249)
 
diff --git a/frameworks/compass/stylesheets/compass/utilities/sprites/_base.scss b/frameworks/compass/stylesheets/compass/utilities/sprites/_base.scss
index 002419f8..e033d85f 100644
--- a/frameworks/compass/stylesheets/compass/utilities/sprites/_base.scss
+++ b/frameworks/compass/stylesheets/compass/utilities/sprites/_base.scss
@@ -24,6 +24,19 @@
   }
 }
 
+// Include the selectors for the `$sprite` given the `$map` and the 
+// `$full-sprite-name`
+// @private
+@mixin sprite_selectors($map, $sprite-name, $full-sprite-name) {
+  @each $selector in $sprite-selectors {
+    @if sprite_has_selector($map, $sprite-name, $selector) {
+      .#{$full-sprite-name}:#{$selector}, .#{$full-sprite-name}_#{$selector}, .#{$full-sprite-name}-#{$selector} {
+        @include sprite($map, "#{$sprite-name}_#{$selector}");
+      }
+    }
+  }
+}
+
 // Generates a class for each space separated name in `$sprite-names`.
 // The class will be of the form .<map-name>-<sprite-name>.
 //
@@ -32,25 +45,13 @@
 // If `$dimensions` is `true`, the sprite dimensions will specified.
 @mixin sprites($map, $sprite-names, $base-class: false, $dimensions: false, $prefix: sprite-map-name($map)) {
   @each $sprite-name in $sprite-names {
-    $full-sprite-name: if($prefix and $prefix != "", "#{$prefix}-#{$sprite-name}", $sprite-name);
-    .#{$full-sprite-name} {
-      @if sprite_has_hover($map, $sprite-name) {
-        &:hover {
-          @include sprite($map, "#{$sprite-name}_hover")
-        }
+    @if sprite_does_not_have_parent($map, $sprite-name) {
+      $full-sprite-name: "#{$prefix}-#{$sprite-name}";
+      .#{$full-sprite-name} {
+        @if $base-class { @extend #{$base-class}; }
+        @include sprite($map, $sprite-name, $dimensions);
       }
-      @if sprite_has_target($map, $sprite-name) {
-        &:target {
-          @include sprite($map, "#{$sprite-name}_target")
-        }
-      }
-      @if sprite_has_active($map, $sprite-name) {
-        &:active {
-          @include sprite($map, "#{$sprite-name}_active")
-        }
-      }
-      @if $base-class { @extend #{$base-class}; }
-      @include sprite($map, $sprite-name, $dimensions);
+      @include sprite_selectors($map, $sprite-name, $full-sprite-name);
     }
   }
 }
\ No newline at end of file
diff --git a/frameworks/compass/stylesheets/compass/utilities/sprites/_sprite-img.scss b/frameworks/compass/stylesheets/compass/utilities/sprites/_sprite-img.scss
index b14536be..4cbfb359 100644
--- a/frameworks/compass/stylesheets/compass/utilities/sprites/_sprite-img.scss
+++ b/frameworks/compass/stylesheets/compass/utilities/sprites/_sprite-img.scss
@@ -24,6 +24,8 @@ $sprite-image-default-width: $sprite-default-size !default;
 
 $sprite-image-default-height: $sprite-default-size !default;
 
+$sprite-selectors: hover, target, active !default;
+
 // Sets all the rules for a sprite from a given sprite image to show just one of the sprites.
 // To reduce duplication use a sprite-bg mixin for common properties and a sprite-select mixin for positioning.
 @mixin sprite-img($img, $col, $row: 1, $width: $sprite-image-default-width, $height: $sprite-image-default-height, $margin: $sprite-default-margin) {
diff --git a/lib/compass/sass_extensions/functions/sprites.rb b/lib/compass/sass_extensions/functions/sprites.rb
index 9b3637c9..18ca2009 100644
--- a/lib/compass/sass_extensions/functions/sprites.rb
+++ b/lib/compass/sass_extensions/functions/sprites.rb
@@ -1,6 +1,6 @@
 module Compass::SassExtensions::Functions::Sprites
   ZERO = Sass::Script::Number::new(0)
-
+  VALID_SELECTORS = %w(hover active target)
   # Provides a consistent interface for getting a variable in ruby
   # from a keyword argument hash that accounts for underscores/dash equivalence
   # and allows the caller to pass a symbol instead of a string.
@@ -66,32 +66,26 @@ module Compass::SassExtensions::Functions::Sprites
   end
   Sass::Script::Functions.declare :sprite_file, [:map, :sprite]
 
-  # Returns boolean is sprite image has a hover selector
-  def sprite_has_hover(map, sprite)
-    verify_map(map)
-    verify_sprite(sprite)
-    Sass::Script::Bool.new map.has_hover?(sprite)
+  # Returns voolean if sprite has a parent
+  def sprite_does_not_have_parent(map, sprite)
+    verify_map map
+    verify_sprite sprite
+    Sass::Script::Bool.new map.image_for(sprite.value).parent.nil?
   end
   
-  Sass::Script::Functions.declare :sprite_has_hover, [:map, :sprite]
+  Sass::Script::Functions.declare :sprite_does_not_have_parent, [:map, :sprite]
 
-  # Returns boolean is sprite image has a target selector
-  def sprite_has_target(map, sprite)
-    verify_map(map)
-    verify_sprite(sprite)
-     Sass::Script::Bool.new map.has_target?(sprite)
+  # Returns boolean if sprite has the selector
+  def sprite_has_selector(map, sprite, selector)
+    verify_map map
+    verify_sprite sprite
+    unless VALID_SELECTORS.include?(selector.value)
+      raise Sass::SyntaxError, "Invalid Selctor did you mean one of: #{VALID_SELECTORS.join(', ')}"
+    end
+    Sass::Script::Bool.new map.send(:"has_#{selector.value}?", sprite)
   end
   
-  Sass::Script::Functions.declare :sprite_has_target, [:map, :sprite]
-  
-  # Returns boolean is sprite image has a active selector
-  def sprite_has_active(map, sprite)
-    verify_map(map)
-    verify_sprite(sprite)
-     Sass::Script::Bool.new map.has_active?(sprite)
-  end
-  
-  Sass::Script::Functions.declare :sprite_has_active, [:map, :sprite]
+  Sass::Script::Functions.declare :sprite_has_selector, [:map, :sprite, :selector]
 
 
   # Returns a url to the sprite image.
diff --git a/lib/compass/sass_extensions/sprites/image.rb b/lib/compass/sass_extensions/sprites/image.rb
index 4a73ee8f..43a40394 100644
--- a/lib/compass/sass_extensions/sprites/image.rb
+++ b/lib/compass/sass_extensions/sprites/image.rb
@@ -67,7 +67,7 @@ module Compass
         
         # Has hover selector
         def hover?
-          base.has_hover?(name)
+          name[-6..-1] == '_hover'
         end
         
         # Hover selector Image object if exsists
@@ -77,7 +77,7 @@ module Compass
         
         # Has target selector
         def target?
-          base.has_target?(name)
+          name[-7..-1] == '_target'
         end
         
         # Target selector Image object if exsists
@@ -87,13 +87,22 @@ module Compass
         
         # Has active selector
         def active?
-          base.has_active?(name)
+          name[-7..-1] == '_active'
         end
         
         # Active selector Image object if exsists
         def active
           base.image_for("#{name}_active")
         end
+        
+        
+        def parent
+          if [hover?, target?, active?].any?
+            %r{(.+)_(.+)$}.match name
+            base.image_for($1)
+          end
+        end
+        
                 
         private
           def dimensions
diff --git a/spec/compass/sass_extensions/sprites/image_spec.rb b/spec/compass/sass_extensions/sprites/image_spec.rb
index 5c61e0f6..f328a535 100644
--- a/spec/compass/sass_extensions/sprites/image_spec.rb
+++ b/spec/compass/sass_extensions/sprites/image_spec.rb
@@ -5,7 +5,15 @@ describe Compass::SassExtensions::Sprites::Image do
   let(:sprite_filename) { 'squares/ten-by-ten.png' }
   let(:sprite_path) { File.join(images_src_path, sprite_filename) }
   let(:sprite_name) { File.basename(sprite_filename, '.png') }
-  let(:image) { self.class.describes.new(nil, File.join(sprite_filename), options)}
+  let(:parent) do
+    mock
+  end
+  before do
+    parent.stubs(:image_for).with('ten-by-ten').returns(image)
+    parent.stubs(:image_for).with('ten-by-ten_hover').returns(hover_image)
+  end
+  let(:image) { self.class.describes.new(parent, File.join(sprite_filename), options)}
+  let(:hover_image) { self.class.describes.new(parent, File.join('selectors/ten-by-ten_hover.png'), options)}
   let(:digest) { Digest::MD5.file(sprite_path).hexdigest }
   subject { image }
 
@@ -34,7 +42,18 @@ describe Compass::SassExtensions::Sprites::Image do
     options.stubs(:get_var).with(get_var_expects).returns(get_var_return)
     options
   }
-
+  
+  describe '#parent' do
+    context '_hover' do
+      subject { hover_image }
+      its(:parent) { should == image }
+    end
+    context 'no parent' do
+      subject { image }
+      its(:parent) { should be_nil }
+    end
+  end
+  
   describe '#repeat' do
     let(:type) { nil }
     let(:get_var_return) { OpenStruct.new(:value => type) }
diff --git a/spec/sprites_spec.rb b/spec/sprites_spec.rb
index e31a3e68..d5aff351 100644
--- a/spec/sprites_spec.rb
+++ b/spec/sprites_spec.rb
@@ -473,34 +473,25 @@ describe Compass::Sprites do
       @include all-selectors-sprites;
     SCSS
     css.should == <<-CSS
-      .selectors-sprite, .selectors-ten-by-ten, .selectors-ten-by-ten_active, .selectors-ten-by-ten_hover, .selectors-ten-by-ten_target {
+      .selectors-sprite, .selectors-ten-by-ten {
         background: url('/selectors-edfef809e2.png') no-repeat;
       }
       
       .selectors-ten-by-ten {
         background-position: 0 0;
       }
-      .selectors-ten-by-ten:hover {
-        background-position: 0 -20px;
-      }
-      .selectors-ten-by-ten:target {
-        background-position: 0 -30px;
-      }
-      .selectors-ten-by-ten:active {
-        background-position: 0 -10px;
-      }
       
-      .selectors-ten-by-ten_active {
-        background-position: 0 -10px;
-      }
-      
-      .selectors-ten-by-ten_hover {
+      .selectors-ten-by-ten:hover, .selectors-ten-by-ten_hover, .selectors-ten-by-ten-hover {
         background-position: 0 -20px;
       }
       
-      .selectors-ten-by-ten_target {
+      .selectors-ten-by-ten:target, .selectors-ten-by-ten_target, .selectors-ten-by-ten-target {
         background-position: 0 -30px;
       }
+      
+      .selectors-ten-by-ten:active, .selectors-ten-by-ten_active, .selectors-ten-by-ten-active {
+        background-position: 0 -10px;
+      }
     CSS
   end