Skip to content
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

use SCENERY_PHET/NumberDisplay #96

Closed
pixelzoom opened this issue Feb 3, 2020 · 3 comments
Closed

use SCENERY_PHET/NumberDisplay #96

pixelzoom opened this issue Feb 3, 2020 · 3 comments

Comments

@pixelzoom
Copy link
Contributor

Related to #92...

The pH meters in Macro and Micro screens should be using NumberDisplay. Ditto for the displays on the graph. They predate NumberDisplay, so have their own implementation to display the value. This is important for phet-io and a11y.

@pixelzoom pixelzoom self-assigned this Feb 3, 2020
@pixelzoom pixelzoom removed their assignment Feb 4, 2020
@pixelzoom pixelzoom changed the title use NumberDisplay use SCENERY_PHET/NumberDisplay Feb 5, 2020
pixelzoom added a commit that referenced this issue Feb 6, 2020
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 6, 2020

I changed PHMeterNode to use NumberDisplay, and that went smoothly.

Changing GraphIndicatorNode is not as easy. It displays values in scientific notation (e.g. "1 x 10-7"), and that's not supported by NumberDisplay. So I'm going to put that on hold until we see what phet-io instrumentation (if any) is needed for GraphIndicatorNode.

Still to do: use NumberDisplay in MacroPHMeterNode.

@pixelzoom pixelzoom self-assigned this Feb 6, 2020
pixelzoom added a commit that referenced this issue Feb 6, 2020
pixelzoom added a commit that referenced this issue Feb 6, 2020
@pixelzoom
Copy link
Contributor Author

GraphIndicatorNode's display is not necessary to instrument. Over in #105, the Properties that contain the values were instrumented. So I'm not going to attempt to use NumberDisplay there.

pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Feb 7, 2020
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Feb 7, 2020
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Feb 7, 2020
@pixelzoom
Copy link
Contributor Author

MacroPHMeterNode has been changed to use NumberDisplay, and refactored to have this structure:

screenshot_105

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant