Remove getSpacingFromLineType

[samreid]

During https://github.com/phetsims/griddle/issues/59 I noticed getSpacingFromLineType. During the review, I found that function may be unnecessary. Here is a proposed patch which seems to work properly in the griddle harness:

```diff Index: js/ScrollingChartNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/ScrollingChartNode.js (revision 5dafdd0764d8c9ec1a32418304e930780d78da0c) +++ js/ScrollingChartNode.js (date 1597186585793) @@ -272,7 +272,6 @@ this.dynamicSeriesMap.delete( dynamicSeries ); } - /** * Set line spacings for the grid and labels. * @public @@ -299,7 +298,7 @@ redrawLabels() { if ( this.showVerticalGridLabels ) { const verticalLabelChildren = []; - const yPositions = this.gridNode.getLinePositionsInGrid( GridNode.LineType.MAJOR_HORIZONTAL ); + const yPositions = this.gridNode.getLinePositionsInGrid( this.majorHorizontalLineSpacing, GridNode.LineType.MAJOR_HORIZONTAL ); yPositions.forEach( yPosition => { const viewY = this.modelViewTransformProperty.get().modelToViewY( yPosition ); const labelPoint = this.graphPanel.localToParentPoint( new Vector2( this.gridNode.bounds.left, viewY ) ); @@ -317,7 +316,7 @@ // draw labels along the horizontal lines const horizontalLabelChildren = []; - const xPositions = this.gridNode.getLinePositionsInGrid( GridNode.LineType.MAJOR_VERTICAL ); + const xPositions = this.gridNode.getLinePositionsInGrid( this.majorVerticalLineSpacing, GridNode.LineType.MAJOR_VERTICAL ); xPositions.forEach( xPosition => { const viewX = this.modelViewTransformProperty.get().modelToViewX( xPosition ); const labelPoint = this.graphPanel.localToParentPoint( new Vector2( viewX, this.gridNode.bounds.bottom ) ); Index: js/GridNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/GridNode.js (revision 5dafdd0764d8c9ec1a32418304e930780d78da0c) +++ js/GridNode.js (date 1597186877166) @@ -208,7 +208,7 @@ const shape = new Shape(); const modelViewTransform = this.modelViewTransformProperty.get(); - const xPositions = this.getLinePositionsInGrid( lineType ); + const xPositions = this.getLinePositionsInGrid( lineSpacing, lineType ); xPositions.forEach( xPosition => { const viewPosition = modelViewTransform.modelToViewX( xPosition ); shape.moveTo( viewPosition, 0 ); @@ -223,6 +223,7 @@ /** * Draws horizontal lines with the provided spacing. A shape is drawn and set to the provided Path. * @private + * * @param {number} lineSpacing * @param {LineType} lineType * @param {Path} linesPath @@ -231,7 +232,7 @@ const shape = new Shape(); const modelViewTransform = this.modelViewTransformProperty.get(); - const yPosition = this.getLinePositionsInGrid( lineType ); + const yPosition = this.getLinePositionsInGrid( lineSpacing, lineType ); yPosition.forEach( yPosition => { const viewPosition = modelViewTransform.modelToViewY( yPosition ); shape.moveTo( 0, viewPosition ); @@ -252,43 +253,17 @@ this.drawMinorLines(); } - /** - * Get the line spacing for the provided line type. If spacing is not defined for the provided lineType, returns null. - * @public - * - * @param {LineType} lineType - * @returns {null|number} - */ - getSpacingFromLineType( lineType ) { - let lineSpacing = null; - if ( lineType === LineType.MAJOR_VERTICAL ) { - lineSpacing = this.majorVerticalLineSpacing; - } - else if ( lineType === LineType.MAJOR_HORIZONTAL ) { - lineSpacing = this.majorHorizontalLineSpacing; - } - else if ( lineType === LineType.MINOR_VERTICAL ) { - lineSpacing = this.minorVerticalLineSpacing; - } - else if ( lineType === LineType.MINOR_HORIZONTAL ) { - lineSpacing = this.minorHorizontalLineSpacing; - } - - return lineSpacing; - } - /** * Returns an array of positions of grid lines in model coordinates, within the view bounds of the grid. Useful * for decorating the grid with labels or other things. * @public * + * @param {number|null} spacing * @param {LineType} lineType * @returns {number[]} */ - getLinePositionsInGrid( lineType ) { + getLinePositionsInGrid( spacing, lineType ) { assert && assert( LineType.includes( lineType ), 'provided lineType should be one of LineType' ); - - const spacing = this.getSpacingFromLineType( lineType ); assert && assert( spacing === null || typeof spacing === 'number', `spacing not defined for ${lineType}` ); const positions = []; ```

It uses existing values for spacing and direct lookup to get the values. @jessegreenberg does this look good to you? If so, feel free to commit it, or let me know if I should.

[jessegreenberg]

This looks great @samreid thanks! I committed this.