-
Notifications
You must be signed in to change notification settings - Fork 6
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
NumberDisplay: uninstrument Text, fix stringProperty dependencies #812
Comments
This has become an issue for instrumentation of Acid-Base Solutions, see phetsims/acid-base-solutions#197. |
From design meeting today, we decided to:
We recognized that there could be a desire to see what the display is showing, but due to the complexity, we don't feel like it is worth keeping around in the current code base. For migration rules, the current stringProperty is a DerivedProperty, so it will just need to be a This is NOT blocking Natural Selection because they uninstrumented the few NumberDisplays in that sim. |
This is likely blocking Acid Base Solutions. Scheduling a few weeks out (MK out next week) |
It's also possible (and my recommendation, but up to @arouinfar) to uninstrument the one NumberDisplay that appears in Acid-Base Solutions. See phetsims/acid-base-solutions#197 (comment). |
This work will block the publication of Greenhouse Effect, see phetsims/greenhouse-effect#341. Greenhouse Effect will likely be be ready for dev testing this week. I don't think this should block dev testing, but it should be addressed before taking SHAs for RC testing. Edit: we may be able to uninstrument the NumberDisplays in Greenhouse Effect instead. I'll update my comment once we decide. Edit 2: we decided to uninstrument the NumberDisplays, so this no longer blocks Greenhouse Effect. |
I wonder if #828 is going to make this issue obsolete. Either way. I never got to this because it never became blocking for an instrumentation of a sim. It will be best to work on this when in the context of a sim that I can test and commit changes against. |
Related to https://github.com/phetsims/phet-io/issues/1952 ...
Over in phetsims/natural-selection#339, I attempted to uninstrument the
valueText
andvalueText.stringProperty
PhET-iO elements in NumberDisplay, and addvalueStringProperty
. This quickly turned into a big effort, and I punted. In addition to having to update API and migration rules for many (most?) PhET-iO sims,valueText.stringProperty
is missing an important dependency - the string pattern that gets filled in.I'll also note that NumberDisplay has become quite the mess. I was the original author, and I almost didn't recognize it. So much chaos has been introduced that it might be better to rewrite, rather than revise.
Assigning to @kathy-phet for prioritization, assign, and decide whether this blocks anything.
@samreid @zepumph @brent-phet @arouinfar FYI.
The text was updated successfully, but these errors were encountered: