search

phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

Generalize "point tool body". #167

Closed pixelzoom closed 1 year ago

pixelzoom commented 3 years ago

As discussed in https://github.com/phetsims/graphing-quadratics/issues/158...

graphing-lines and graphing-quadratics both have "point tools". But they have different implementations of PointTool (model) and PointToolNode (view). The reason for that is documented in #18. (graphing-slope-intercept consists of a screen from graphing-lines, so uses the point tool from graphing-lines.)

Both implementations of PointToolNode use .png files that were exported from graphing-lines/asserts/point_tool.ai. Then we needed a wider display area (to show more decimal places) in https://github.com/phetsims/graphing-quadratics/issues/158, so we copied point_tool.ai to graphing-quadratics/asserts/point_tool.ai, and made adjusts specific to https://github.com/phetsims/graphing-quadratics/issues/158, to be released in Graphing Quadratics 1.2.

We realized that a better long-term solution is to ditch the .ai and .png files for PointToolNode, and create a "point tool body" (PointToolBodyNode?) programmatically. It should be parameterized to handle different sizes of display area, different sizes of "grippy" area, different colors, etc. The basic body shape will use ShadedRectangleNode. The grippy parts will likely be a set of Paths that are filled with a LinearGradient to give the illusion of being lit from above.

PointToolBodyNode will not include the probe part of the tool. Adding a sim-specific probe will be the responsibility of the client application.

This mockup by @amanda-phet shows the revised PointToolNode on the right:

image

pixelzoom commented 3 years ago

This issue is deferred until Graphing Quadratics 1.2 is published https://github.com/phetsims/graphing-quadratics/issues/137.

pixelzoom commented 3 years ago

Self assigning. I'll probably have some time to do this after Graphing Quadratics 1.2 is published.

pixelzoom commented 3 years ago

@amanda-phet ready for review in master. I used your mockup (above) as a guide for colors, spacing, etc.

Here are before and after screenshots.

Graphing Quadratics:

screenshot_1230 screenshot_1229

Graphing Lines:

screenshot_1232 screenshot_1231

Also note that Level 3 of the Line Game screen in Graphing Lines has point tools as the reward. Run the sin with ?showReward, play Level 3 (no need to get any correct answers), and you'll see a reward that uses the new point tool:

screenshot_1234
pixelzoom commented 3 years ago

I should also note that I did NOT migrate PointToolBodyNode to common code. It's not clear which repo it should live in, and it's currently used only within the "graphing lines" family of sims. So as with the other components that are shared by that family, it currently lives in the graphing-lines repo. If/when a more general use is needed, PointToolBodyNode can be easily migrated to some other repo.

amanda-phet commented 3 years ago

Looks like a nice change overall. Thanks for making this happen @pixelzoom !

pixelzoom commented 3 years ago

Screenshots will need to be revised for the sims that use the point tool. I've created issues for each sim, linked above.

This issue is done, and is ready to be verified in a future QA process.

pixelzoom commented 1 year ago

There's no reason to have QA verify this, it's gotten a workout while getting the sim ready for https://github.com/phetsims/graphing-quadratics/issues/176. Closing.