phetsims / calculus-grapher

"Calculus Grapher" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 4 forks source link

Sinusoid Icon #333

Closed veillette closed 1 year ago

veillette commented 1 year ago

We have made some changes to the sinusoid mode in the most recent devs (see #302). As a result, the sine function has now a much wider width than it used to. We have not updated the icon to reflect this change.

image

I would propose to make the sinusoid icon more iconic by showing the classic sine function with one full wavelength.

veillette commented 1 year ago

I created a patch for my suggestion.

Patch for sine icon ````js Subject: [PATCH] maintain aspect ratio of CurveManipulationDisplayNode (see #332) --- Index: js/common/view/CurveManipulationIconNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CurveManipulationIconNode.ts b/js/common/view/CurveManipulationIconNode.ts --- a/js/common/view/CurveManipulationIconNode.ts (revision bfc57fb9a751928743dafd9d53939ff7ccc218d7) +++ b/js/common/view/CurveManipulationIconNode.ts (date 1680647276276) @@ -86,10 +86,10 @@ else if ( mode === CurveManipulationMode.SINUSOID ) { // Ad hoc variables to create sine function - const sinusoidWidth = 0.08 * xLength; + const sinusoidWidth = + CalculusGrapherConstants.CURVE_MANIPULATION_WIDTH_RANGE.defaultValue * xLength / ( 2 * Math.PI ); const y = 0.5 * yMax; - solidCurve.sinusoid( sinusoidWidth, xCenter, y ); - solidCurve.save(); + solidCurve.sinusoid( sinusoidWidth, xLength / 4, y ); solidCurve.shift( xMin, yCenter ); } else if ( mode === CurveManipulationMode.FREEFORM ) { ````

It would look like below. It is a little bit less busy and more in line with the other icons. The periodicity of the sine function is no longer apparent in the icon, but students should recognize a sine function with only one period.

pixelzoom commented 1 year ago

This looks great. I think it would be a nice improvement.

veillette commented 1 year ago

Assigning to @amanda-phet to decide if this is worth implementing.

veillette commented 1 year ago

Raising the priority. @amanda-phet it would be to know if we should cherry-pick this as for CG, or you have objections/suggestions to this proposal.

amanda-phet commented 1 year ago

This looks great to me, too! And is related to a question @KatieWoe asked me about, so I like it.

veillette commented 1 year ago

Thanks @amanda-phet. I'll commit the patch to master then.

veillette commented 1 year ago

The patch was committed to master.

veillette commented 1 year ago

Above, the commit on master was cherry-picked on the RC branch 1.0.

veillette commented 1 year ago

To verify in the next RC:

In PhET brand, verify that:

Nancy-Salpepi commented 1 year ago

Looks nice in rc.2! Closing