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

visibleProperty should be uninstrumented instead of read-only #319

Closed
pixelzoom opened this issue Jan 31, 2023 · 3 comments
Closed

visibleProperty should be uninstrumented instead of read-only #319

pixelzoom opened this issue Jan 31, 2023 · 3 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 31, 2023

For phetsims/qa#889, noticed while working on #317.

There are 7 inappropriate occurrences of this option:

visiblePropertyOptions: { phetioReadOnly: true }

This instruments visibleProperty but makes it read-only. It's only appropriate to use this when the visibility of the element can somehow be controlled via the sim, in case the client wants to inspect the value of visibleProperty. In all cases for this sim, the visibility never changes, so it would be more appropriate to uninstrument visibleProperty, like this:

phetioVisiblePropertyInstrumented: false

This will require grunt generate-phet-io-api to update the API files. @arouinfar FYI.

@pixelzoom pixelzoom self-assigned this Jan 31, 2023
@pixelzoom
Copy link
Contributor Author

Done in the above commit, and API files regenerated for both sims. There's nothing to review here, because these visibleProperty elements are now gone. So I'll go ahead and close.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 6, 2023

Reopening to confirm in phetsims/qa#894. It looks like changes I made during dev testing did not get into the 1.7 release branch.

@samreid @matthew-blackman FYI.

@pixelzoom pixelzoom reopened this Feb 6, 2023
@pixelzoom
Copy link
Contributor Author

Looks OK in 1.7. 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