Options for BarChartNode labels

[jessegreenberg]

One of the TODOs mentioned in #29 is

      if ( bar.labelString ) {
        var labelText = new RichText( bar.labelString, {
          rotation: -Math.PI / 2,
          font: new PhetFont( { size: 12, weight: 'bold' } ),
          fill: bar.entries.length === 1 ? bar.entries[ 0 ].color : 'black',
          maxWidth: 40 // TODO: Standardize
        } );

I need to set the maxWidth of the label for phetsims/energy-skate-park#31 so Ill add the options.

[jessegreenberg]

Done in the above commit. @Denz1994 would you please review?

[Denz1994]

Thanks for handling this @jessegreenberg.

My only suggestion would be to allow the client to pass in options.barLabelOptions.rotation and not explicitly set it in BarChartNode.js. So options would look like this:

 // passed along to the RichText
    options.barLabelOptions = _.extend( {
      font: new PhetFont( { size: 12, weight: 'bold' } ),

      // chosen by inspection, good for short labels
      maxWidth: 40,
      rotation: -Math.PI / 2
    }, options.barLabelOptions );

This would remove the below assertion and declaration in BarChartNode.js

assert && assert( options.barLabelOptions.rotation === undefined, 'label rotation set for BarChartNode orientation' );
options.barLabelOptions.rotation = -Math.PI / 2;

The initial reason for defaulting to vertical labels was so they would fit on the x-axis in the current sims using BarChartNode.js, but there may be a future case where labels shouldn't be vertical by default (i.e. bar labels text using only numbers). Also, if we ever needed horizontal support for BarChartNode.js we may not want to keep the bars labels vertical, as noted by the TODO in the header.

/**
 * Bar chart for sims that need to display separate values
 *
 * TODO: Add horizontal support
 *

[jessegreenberg]

That sounds great @Denz1994, thanks for reviewing. Done in the above commit, can you take a look? Anything else for this issue?

[Denz1994]

Looks great. I'll close this issue. Thanks for handling this.