more cleanup for optional data label attributes

6 files changed, 34 insertions, 29 deletions

diff --git a/lib/axlsx/drawing/bar_3D_chart.rb b/lib/axlsx/drawing/bar_3D_chart.rb
index f3eb77e3..50dbecb2 100644
--- a/lib/axlsx/drawing/bar_3D_chart.rb
+++ b/lib/axlsx/drawing/bar_3D_chart.rb
@@ -120,7 +120,6 @@ module Axlsx
     # @param [String] str
     # @return [String]
     def to_xml_string(str = '')
-      remove_invalid_d_lbls_attributes
       super(str) do |str_inner|
         str_inner << '<c:bar3DChart>'
         str_inner << '<c:barDir val="' << bar_dir.to_s << '"/>'
@@ -139,12 +138,5 @@ module Axlsx
         @val_axis.to_xml_string str_inner
       end
     end
-
-    def remove_invalid_d_lbls_attributes
-      return unless @d_lbls
-      @d_lbls.instance_eval{ @d_lbl_pos = nil
-                             @show_leader_lines = nil
-      }
-    end
   end
 end
diff --git a/lib/axlsx/drawing/chart.rb b/lib/axlsx/drawing/chart.rb
index c99d4878..d0c3cd13 100644
--- a/lib/axlsx/drawing/chart.rb
+++ b/lib/axlsx/drawing/chart.rb
@@ -47,9 +47,10 @@ module Axlsx
 
     #TODO data labels!
     def d_lbls
-      @d_lbls ||= DLbls.new
+      @d_lbls ||= DLbls.new(self.class)
     end
 
+
     # The title object for the chart.
     # @return [Title]
     attr_reader :title
diff --git a/lib/axlsx/drawing/d_lbls.rb b/lib/axlsx/drawing/d_lbls.rb
index 13882056..f7126a67 100644
--- a/lib/axlsx/drawing/d_lbls.rb
+++ b/lib/axlsx/drawing/d_lbls.rb
@@ -24,17 +24,17 @@ module Axlsx
     #   Bar3DChart and Line3DChart and ScatterChart do not support d_lbl_pos or show_leader_lines
     #    
     BOOLEAN_ATTRIBUTES = [:show_legend_key, :show_val, :show_cat_name, :show_ser_name, :show_percent, :show_bubble_size, :show_leader_lines]
-    
+
     # creates a new DLbls object
-    def initialize(options={})
-      @d_lbl_pos = :bestFit
+    def initialize(chart_type, options={})
+      raise ArgumentError, 'chart_type must inherit from Chart' unless chart_type.superclass == Chart
+      @chart_type = chart_type
       initialize_defaults
       options.each do |o|
         self.send("#{o[0]}=", o[1]) if self.respond_to? "#{o[0]}="
       end
-
     end
-    
+
     # Initialize all the values to false as Excel requires them to
     # explicitly be disabled or all will show.
     def initialize_defaults
@@ -43,22 +43,31 @@ module Axlsx
       end
     end
 
+    # The chart type that is using this data lables instance. 
+    # This affects the xml output as not all chart types support the
+    # same data label attributes. 
+    attr_reader :chart_type
+
     # The position of the data labels in the chart
     # @see d_lbl_pos= for a list of allowed values
     # @return [Symbol]
-    attr_reader :d_lbl_pos
+    def d_lbl_pos
+      return unless @chart_type == Pie3DChart
+      @d_lbl_pos ||= :bestFit
+    end
 
     # @see DLbls#d_lbl_pos
     # Assigns the label postion for this data labels on this chart.
     # Allowed positions are :bestFilt, :b, :ctr, :inBase, :inEnd, :l,
     # :outEnd, :r and :t
-    # The default is :best_fit
+    # The default is :bestFit
     # @param [Symbol] label_postion the postion you want to use.
     def d_lbl_pos=(label_position)
+      return unless @chart_type == Pie3DChart
       Axlsx::RestrictionValidator.validate 'DLbls#d_lbl_pos',  [:bestFit, :b, :ctr, :inBase, :inEnd, :l, :outEnd, :r, :t], label_position
       @d_lbl_pos = label_position
     end
-   
+
     # Dynamically create accessors for boolean attriubtes
     BOOLEAN_ATTRIBUTES.each do |attr|    
       class_eval %{
@@ -80,10 +89,19 @@ module Axlsx
     # serializes the data labels
     # @return [String]
     def to_xml_string(str = '')
+      validate_attributes_for_chart_type
       str << '<c:dLbls>'
       instance_values.each { |name, value| str << "<c:#{Axlsx::camel(name, false)} val='#{value}' />" }
       str << '</c:dLbls>'
     end
+
+    # nills out d_lbl_pos and show_leader_lines as these attributes, while valid in the spec actually chrash excel for any chart type other than pie charts.
+    def validate_attributes_for_chart_type
+      return if @chart_type == Pie3DChart
+      @d_lbl_pos = nil
+      @show_leader_lines = nil
+    end
+
+
   end
 end
-
diff --git a/lib/axlsx/drawing/line_3D_chart.rb b/lib/axlsx/drawing/line_3D_chart.rb
index 2268a6dc..b601c02b 100644
--- a/lib/axlsx/drawing/line_3D_chart.rb
+++ b/lib/axlsx/drawing/line_3D_chart.rb
@@ -90,7 +90,6 @@ module Axlsx
     # @param [String] str
     # @return [String]
     def to_xml_string(str = '')
-      remove_invalid_d_lbls_attributes
       super(str) do |str_inner|
         str_inner << '<c:line3DChart>'
         str_inner << '<c:grouping val="' << grouping.to_s << '"/>'
@@ -107,14 +106,5 @@ module Axlsx
         @serAxis.to_xml_string str_inner
       end
     end
-    #
-    # nills out d_lbls attributes that are not allowed in this chart type
-    def remove_invalid_d_lbls_attributes
-      return unless @d_lbls
-      @d_lbls.instance_eval{ @d_lbl_pos = nil
-                             @show_leader_lines = nil
-      }
-    end
-
   end
 end
diff --git a/lib/axlsx/drawing/scatter_chart.rb b/lib/axlsx/drawing/scatter_chart.rb
index 7b9230d5..b3cd8c29 100644
--- a/lib/axlsx/drawing/scatter_chart.rb
+++ b/lib/axlsx/drawing/scatter_chart.rb
@@ -29,6 +29,7 @@ module Axlsx
       @yValAxis = ValAxis.new(@yValAxId, @xValAxId)
       super(frame, options)
       @series_type = ScatterSeries
+      @d_lbls = nil
       options.each do |o|
         self.send("#{o[0]}=", o[1]) if self.respond_to? "#{o[0]}="
       end
@@ -49,6 +50,9 @@ module Axlsx
         str_inner << '<c:scatterStyle val="' << scatterStyle.to_s << '"/>'
         str_inner << '<c:varyColors val="1"/>'
         @series.each { |ser| ser.to_xml_string(str_inner) }
+        if @d_lbls
+          @d_lbls.to_xml_string(str)
+        end
         str_inner << '<c:dLbls>'
         str_inner << '<c:showLegendKey val="0"/>'
         str_inner << '<c:showVal val="0"/>'
diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb
index e81c31c7..abb6d48b 100644
--- a/lib/axlsx/workbook/worksheet/worksheet.rb
+++ b/lib/axlsx/workbook/worksheet/worksheet.rb
@@ -610,7 +610,7 @@ module Axlsx
 
     def workbook=(v) DataTypeValidator.validate "Worksheet.workbook", Workbook, v; @workbook = v; end
 
-     def update_column_info(cells, widths=[])
+    def update_column_info(cells, widths=[])
       cells.each_with_index do |cell, index|
         col = find_or_create_column_info(index)
         next if widths[index] == :ignore