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

Should dropper have a fixed position? #323

Closed
pixelzoom opened this issue Feb 1, 2023 · 7 comments
Closed

Should dropper have a fixed position? #323

pixelzoom opened this issue Feb 1, 2023 · 7 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Feb 1, 2023

Related to phetsims/qa#889.

This sim has all of the same problem with the movable dropper that we encountered in phetsims/ph-scale#270. Should we make the dropper also have a fixed position in this sim? Or is this sim somehow different because the shaker is (necessarily) movable?

My preference would be to also make the dropper have a fixed position here. Otherwise we'll be circling back to this when the sim gets alt input support.

@arouinfar
Copy link
Contributor

My preference would be to also make the dropper have a fixed position here. Otherwise we'll be circling back to this when the sim gets alt input support.

Yes, let's fix the position of the dropper @pixelzoom.

In case someone in the future questions this decision, the many reasons why the movable dropper was problematic in ph-scale are documented here: phetsims/ph-scale#270 (comment)

@arouinfar arouinfar assigned pixelzoom and unassigned arouinfar Feb 1, 2023
@pixelzoom
Copy link
Contributor Author

Next questions is where should the fixed position of the dropper be? The default when you start the sim is the first screenshot below. I have a slight aesthetic preference for centering it between the faucet and right edge of the beaker, second screenshot below -- @arouinfar is that OK with you?

screenshot_2249

screenshot_2250

@arouinfar
Copy link
Contributor

Centered looks good to me @pixelzoom.

@pixelzoom
Copy link
Contributor Author

Done in the above commit. @arouinfar please have a look in master, close if OK.

I'm glad we're doing this for the first version of the sims to have API files, so we don't need a migration rule.

@arouinfar
Copy link
Contributor

Looks good, thanks!

@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 pixelzoom assigned KatieWoe and unassigned arouinfar Feb 6, 2023
@pixelzoom
Copy link
Contributor Author

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

3 participants