-
Notifications
You must be signed in to change notification settings - Fork 5
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
Beaker falls into burner or floor more easily #227
Comments
I was able to duplicate this on my touch-enabled Win 10 machine. The screenshot below shows a situation that I was able to get the sim into by pushing the block down on the beaker. Normally, I think I'd say that this is a multi-touch issue, and a bit of an odd one at that, so it isn't worth the time and effort to fix, but this looks fairly bad. I think it would be worthwhile to spend a little time investigating whether way can recover from this situation in a way that doesn't end up with the beaker on top of the burner. |
@chrisklus - can you please take a look and see if there is a way to essentially move the beaker to a new spot if it ends up in the situation shown? |
Actually, I just took a look and I think I may have a fix, or at least an improvement. Assigning to myself. |
There was a problem with the portion of the code that constrains the drag motion of a model element based on the position of the other model elements. The gist of the problem is that when a block is dragged and tested against the beakers' bounds, the edges of the beakers are used, but when the beaker was being dragged and its bounds were being compared against those of the blocks, the outline shape of the beaker was being used instead of the edges. In a non-multi-touch environment, this is never a problem, since there is (pretty much) no situation where a beaker can be dragged up to a block such that the block goes inside of the beaker. In a multi-touch environment, that situation can be easily made to happen. I've changed to code to use the beakers' edges in these case, and this change appears to have made the problem of the beakers ending up on top of the burners go away. It also seems to prevent the problem where beakers can be pushed below the edge of the bench, and it improves multi-touch interaction in general when beakers and blocks and being dragged at the same time - it's less jumpy and jittery. I also simplified the code a bit to make it more clear the type of interaction being constrained in each case, i.e. beaker-block or block-block (beaker-beaker is handled in a different area). In the interest of time, I haven't tested these changes a ton, nor done a lot of regression testing to make sure that this didn't break other dragging interactions, but the changes were reasonably well contained, so I'm hopeful that it's fine. @KatieWoe - can you please test that this is fixed on master and that the other dragging interactions still work correctly? If all looks good, please assign to @chrisklus for review. |
Good catch. I think I know exactly what's up and will address shortly. |
Fixed. @KatieWoe, please take another look. I think this may also fix one of the interactions that you showed me (that we decided not to follow up on) where the block could be dragged through the bottom of the beaker during some multi-touch interactions. |
This problem does seem to be solved. I'm still seeing the one we decided not to address. But like we said, it should be perfectly acceptable. |
Over to @chrisklus for review. |
Nice fix! I tested on one of our iPads and the code changes look good. Closing. |
Device
iPad (2018)
OS
iOS 12.1.4
Browser
Safari
Problem Description
For phetsims/qa#293. May be an easier to achieve version of #163.
Attempting to place a block in a beaker while both are being held causes the beaker to move down through the floor/burner. Is in published version.
Steps to Reproduce
Screenshots
https://drive.google.com/file/d/1mrWjbU4W-MYjRasaIA1B1PF7EUhCkjvw/view?usp=sharing
The text was updated successfully, but these errors were encountered: