support hide_labels separate from hide_line_markers for bar graphs only

[elrayle]

Partially addresses Issue #139 hiding line markers results in series labels being hidden

A first attempt was made in PR #443 to add support for hide_labels to all graphs, but there were many failing tests. The reason for the failures is that graph types handle labels differently.

This PR limits hide_labels to bar graphs which has a fairly straight forward implementation.

IMPACTS: bar graphs
USAGE:

    g.no_labels = true | false
    g.no_line_markers = true | false

See new tests for usage examples.

This can serve as a pattern for adding this functionality to other graph types.

[elrayle]

Each graph type handles labels differently and can set @hide_line_markers within their initializer. This complicates the transition to handling hiding of labels and hiding of line markers separately.

Using the hide_labels? method allows for a transition of one graph type at a time, keeping the required changes simple and easy to review.

[Watson1978]

I think we need to consider a SideBar too in a condition at least. What do you think about ?

[Watson1978]

If we were to adapt this code to Gruff's style, it would look something like below.

diff --git a/lib/gruff/bar.rb b/lib/gruff/bar.rb
index b27f02e..45a2995 100644
--- a/lib/gruff/bar.rb
+++ b/lib/gruff/bar.rb
@@ -33,12 +33,17 @@ class Gruff::Bar < Gruff::Base
   # Default is +false+.
   attr_writer :show_labels_for_bar_values
 
+  # Prevent drawing of column labels below a bar graph. For other graph types, hide_labels
+  # is ignored and continues to be tied to hide_line_markers. Default is +false+.
+  attr_writer :hide_labels
+
   def initialize_ivars
     super
     @spacing_factor = 0.9
     @group_spacing = 10
     @label_formatting = nil
     @show_labels_for_bar_values = false
+    @hide_labels = false
   end
   private :initialize_ivars
 
@@ -136,4 +141,8 @@ def draw_bars
   def calculate_spacing
     @scale * @group_spacing * (column_count - 1)
   end
+
+  def hide_labels?
+    @hide_labels
+  end
 end
diff --git a/lib/gruff/base.rb b/lib/gruff/base.rb
index c9fb7a7..16b7277 100644
--- a/lib/gruff/base.rb
+++ b/lib/gruff/base.rb
@@ -103,10 +103,6 @@ class Base
     # Prevent drawing of line markers. Default is +false+.
     attr_writer :hide_line_markers
 
-    # Prevent drawing of column labels below a bar graph. For other graph types, hide_labels
-    # is ignored and continues to be tied to hide_line_markers. Default is +false+.
-    attr_writer :hide_labels
-
     # Prevent drawing of the legend. Default is +false+.
     attr_writer :hide_legend
 
@@ -233,7 +229,7 @@ def initialize_ivars
 
       @no_data_message = 'No Data'
 
-      @hide_line_markers = @hide_legend = @hide_title = @hide_line_numbers = @legend_at_bottom = @hide_labels = false
+      @hide_line_markers = @hide_legend = @hide_title = @hide_line_numbers = @legend_at_bottom = false
       @center_labels_over_point = true
       @has_left_labels = false
       @label_stagger_height = 0
@@ -675,8 +671,6 @@ def draw_unique_label(index)
     end
 
     def hide_labels?
-      return @hide_labels if is_a? Gruff::Bar
-
       @hide_line_markers
     end
 
diff --git a/lib/gruff/bar.rb b/lib/gruff/bar.rb
index b27f02e..45a2995 100644
--- a/lib/gruff/bar.rb
+++ b/lib/gruff/bar.rb
@@ -33,12 +33,17 @@ class Gruff::Bar < Gruff::Base
   # Default is +false+.
   attr_writer :show_labels_for_bar_values
 
+  # Prevent drawing of column labels below a bar graph. For other graph types, hide_labels
+  # is ignored and continues to be tied to hide_line_markers. Default is +false+.
+  attr_writer :hide_labels
+
   def initialize_ivars
     super
     @spacing_factor = 0.9
     @group_spacing = 10
     @label_formatting = nil
     @show_labels_for_bar_values = false
+    @hide_labels = false
   end
   private :initialize_ivars
 
@@ -136,4 +141,8 @@ def draw_bars
   def calculate_spacing
     @scale * @group_spacing * (column_count - 1)
   end
+
+  def hide_labels?
+    @hide_labels
+  end
 end
diff --git a/lib/gruff/base.rb b/lib/gruff/base.rb
index c9fb7a7..16b7277 100644
--- a/lib/gruff/base.rb
+++ b/lib/gruff/base.rb
@@ -103,10 +103,6 @@ class Base
     # Prevent drawing of line markers. Default is +false+.
     attr_writer :hide_line_markers
 
-    # Prevent drawing of column labels below a bar graph. For other graph types, hide_labels
-    # is ignored and continues to be tied to hide_line_markers. Default is +false+.
-    attr_writer :hide_labels
-
     # Prevent drawing of the legend. Default is +false+.
     attr_writer :hide_legend
 
@@ -233,7 +229,7 @@ def initialize_ivars
 
       @no_data_message = 'No Data'
 
-      @hide_line_markers = @hide_legend = @hide_title = @hide_line_numbers = @legend_at_bottom = @hide_labels = false
+      @hide_line_markers = @hide_legend = @hide_title = @hide_line_numbers = @legend_at_bottom = false
       @center_labels_over_point = true
       @has_left_labels = false
       @label_stagger_height = 0
@@ -675,8 +671,6 @@ def draw_unique_label(index)
     end
 
     def hide_labels?
-      return @hide_labels if is_a? Gruff::Bar
-
       @hide_line_markers
     end
  

[elrayle]

I created expected_java PNG32 bit versions of the new graphs. I don't have a way to verify that the images are correct for the required purpose. I used the following imagemagick command to create them.

$ cd test/expected_java
$ convert ../expected/bar_no_labels.png PNG32:bar_no_labels.png

[elrayle]

@Watson1978 This provides an implementation for bar charts and is ready for review.

[Watson1978]

If you want an accessor that is only effective for bar graphs, prepare it in bar.rb, not base.rb.
I don't want to provide anything in base.rb that isn't commonly used in all inherited classes.

[elrayle]

That makes sense. I'll move the changes to bar.

[elrayle]

@Watson1978 DONE

[Watson1978]

Thanks

[elrayle]

RE: side_bar - I like the idea of adding this change to side_bar, stacked_bar, and side_stacked_bar. I'll can look at adding these, but prefer to do it in a separate PR to keep this one simple.

[Watson1978]

separate PR to keep this one simple.

OK, I see 😉