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

Uninstrument showMouseCueProperty #95

Open
marlitas opened this issue Feb 13, 2025 · 2 comments
Open

Uninstrument showMouseCueProperty #95

marlitas opened this issue Feb 13, 2025 · 2 comments
Assignees

Comments

@marlitas
Copy link
Contributor

numberPairs.tenScreen.model.groupSelectBeadsModel.showMouseCueProperty

numberPairs.tenScreen.model.groupSelectLocationObjectsModel.showMouseCueProperty

@marlitas marlitas self-assigned this Feb 13, 2025
@pixelzoom
Copy link
Collaborator

pixelzoom commented Feb 25, 2025

I took a look at this, but could not address it because the requirements are unclear. showMouseCueProperty lives in GroupSelectModel.ts, so changing it's instrumentation will affect other sims. It's unclear whether it should be uninstrumented for all sims, or GroupSelectModel needs an additional option to specify whether it's instrumented. And I'm guessing that this requires discussion outside the Number Pairs team.

@pixelzoom
Copy link
Collaborator

pixelzoom commented Feb 25, 2025

I will also note that PhET typically avoids using "show" and "hide" in tandem names, and it's concerning to find it used here in common code. "Visible" is the preferred naming convention, in this case mouseCueVisibleProperty: Property<boolean>.

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

2 participants