phetsims / scenery-phet

Reusable components based on Scenery that are specific to PhET simulations.
MIT License
8 stars 6 forks source link

Create a GridIcon #777

Closed veillette closed 1 year ago

veillette commented 1 year ago

Grid icons are commonly used in our simulations to illustrate gridlines for a graph. At this point, PhET has settled on the look of the grid icon. However, the common code does not have Grid icon. The closest we have to a grid icon in the common code is the GridCheckbox in Scenery-PhET, which wraps a GridIcon into a checkbox.

However, I would like to make the case that it would be preferable to extirpate the icon from the checkBox.

A class such as VerticalCheckboxGroup requires items as parameters rather than checkboxes, therefore, we cannot leverage the verticalCheckboxGroup when one of the checkboxes in our group has a grid icon. In other cases, such as the quadrilateral sim, the grid icon could appear in the checkbox after a string.
image

To support these use cases, I proposed to create a GridIcon (based on the path created in GridCheckbox). As part of this issue the GridCheckbox should be rewritten to invoke the newly created GridIcon.

veillette commented 1 year ago

Assigning to @pixelzoom since he created GridCheckbox.

veillette commented 1 year ago

Looping in @jessegreenberg, since it may be useful to the quadrilateral sim

pixelzoom commented 1 year ago

Ready for review by @veillette.

I factored out GridIcon, which is used by GridCheckbox. Uses of GridCheckbox were modified to use the revised GridCheckboxOptions.

GridIcon is currently constrainted to be an NxN grid, which is sufficient for all of the current use cases and simplifies the caller's API. It would be easy to generalize to an MxN grid, but I felt like we should wait until that's necessary.

veillette commented 1 year ago

Thanks @pixelzoom,it is a nice generalization. I agree that we don't need MxN grid at this point.

I did a cursory check to see if other simulations could make use of the gridIcon.

Only the first case is applicable to the gridIcon, and the three others are beyond the realm of gridIcon. There are no commonalities to them that can be easily generalized.

I will create an issue for quadrilateral sim.

veillette commented 1 year ago

The work is done here. Closing