search

phetsims / least-squares-regression

"Least-Squares Regression" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/least-squares-regression
GNU General Public License v3.0
0 stars 4 forks source link

SceneryPhet/GridCheckbox should be leveraged #83

Open veillette opened 2 years ago

veillette commented 2 years ago

The GridIcon and its invocation in a checkbox should be replaced by the common code GridCheckbox. Discovered while working on https://github.com/phetsims/scenery-phet/issues/777

veillette commented 2 years ago

In order to use the Scenery-Phet gridcheckBox, I has to adjust the gridSize and gridStroke to match the least-squares-regression grid Icon. 

    // Create grid checkbox with grid icon
    const gridCheckbox = new GridCheckbox( model.showGridProperty, {
      gridSize: 48,
      gridStroke: LeastSquaresRegressionConstants.MAJOR_GRID_STROKE_COLOR
    } );

Making a visual comparison:

Using SceneryPhet/GridCheckBox Least-Squares Regression screenshot (2)

Using Least-Squares-Regression/GridIcon Least-Squares Regression screenshot (3)

One can notice a slight offset between the spacing of the checkbox and the grid icon.

Assigning to @amanda-phet to approve of the visual change.

amanda-phet commented 2 years ago

@veillette this looks totally fine to me.

Could we see if the default GridCheckbox is adequate? I looked at two sims that use GridCheckbox (I think) and it looks pretty similar to this one. Was the main difference the stroke color? If so, maybe we should stick with the default color, for consistency between sims?

veillette commented 2 years ago

The main difference is the stroke color and the size (it is quite a bit larger than the default, 48 instead of 30). The default color is rgb( 100, 100, 100 ) for the gridCheckBox but least squares regression uses rgb( 128, 128, 128 ). However, that very same color is also used for the major grid stroke on the graph. I don't know if that affects your decision.

I included screenshots (with a portion of the graph with major stroke) for ease of comparison

With LSR size and color image

with LSR size but gridcheckbox default color image

with gridcheckbox default color and size image

Let me know what you think. @amanda-phet