-
Notifications
You must be signed in to change notification settings - Fork 4
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
Uninstrument Text and RichText #341
Comments
@jbphet please uninstrument these elements:
I identified NumberDisplay and TimeControlNode as the common code elements that need to be addressed. NumberDisplay will be handled in phetsims/scenery-phet#812. I will open a separate issue for TimeControlNode. @jbphet these issues do no block dev testing of Greenhouse Effect, but should be addressed before RC.
|
Actually, @jbphet I think we can uninstrument the NumberDisplays used on the Layer Model screen instead. |
I've done some of these, but have questions on some. I'll request some time with @arouinfar, and I think we'll be able to finalize this pretty quickly. Worthy of note: I'm not doing anything in the GE code to instrument |
Before these requests can be fully addressed, we need to have resolutions to the following common code issue, and they need to be included the RC branch: |
Thanks @jbphet the changes look good in master. I also added phetsims/joist#934 to the list in #341 (comment). |
@arouinfar - Can you check whether we've addressed all of these? I've looked it over some and I think we have gotten rid of most if not all of the instrumented text, but you know more about it than I. |
@jbphet I reviewed on master, and things are looking great. I couldn't find any extraneous instrumentation of Text/RichText, closing. |
In https://github.com/phetsims/phet-io/issues/1952#issuecomment-1664495275 we decided that we generally don't want to instrument Text nodes, and instead just want to focus on the stringProperties, especially if they are derived or pattern.
The text was updated successfully, but these errors were encountered: