-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Should this sim use ZoomButtonGroup? #313
Comments
Thanks @jessegreenberg. I commented in phetsims/scenery-phet#645 for clarification about future support for buttons with the magnifying glass icon. We're definitely not going to make this change for the upcoming release, and any decision we make about the appearance of the zoom buttons can be deferred until we work on the phet-io design of this sim. |
To clarify, after phetsims/circuit-construction-kit-common#620 ZoomButtonGroup supports magnifying glasses or +/- icons. This issue is more about eliminating the duplicated code in Energy Skate Park that creates separate buttons, applies touch areas, puts them in a VBox, wires up listeners, etc. I agree this will be important for PhET-iO or a11y but we don't necessarily need to hold off until then. (Also, I understand if an imminent deadline prevents making this refactor in the near future). |
Further clarification... This does not necessarily mean changing the look of the zoom buttons. There are 2 subclasses of ZoomButtonGroup. A primary motivation for changing to one of these subclasses it that you will get consistency of PhET-iO implementation, and not have to instrument sim-specifiic code. MagnifyingGlassZoomButtonGroup uses the same magnifying glasses currently used in this sim. If you want this sim to look the same, use this class. PlusMinusZoomButtonGroup is the more moderning looking +/- buttons that @jessegreenberg showed above. Use this class if you want to change to buttons that look like this: |
This sim is now using MagnifyingGlassZoomButtonGroup. Behavior is the same and a side-by-side comparison of the sim before and after shows no change in look or layout. EnergyGraphZoomButton could be deleted. Since MagnifyingGlassZoomButtonGroup exists there is no need to discuss changing the look in this sim so this issue can be closed. |
In phetsims/scenery-phet#645 it was mentioned that ZoomButtonGroup may be replacing ZoomButton. The ZoomButtonGroup looks like
But it is being used in all new charts and in things that used to use ZoomButton, like SeismographNode.
@arouinfar should the ZoomButtons in this sim be replaced with ZoomButtonGroup? Not for imminent release, but near future and PhET-iO work?
The text was updated successfully, but these errors were encountered: