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

FaucetNode pointer areas aren't behaving correctly #614

Closed
pixelzoom opened this issue Jul 22, 2020 · 10 comments
Closed

FaucetNode pointer areas aren't behaving correctly #614

pixelzoom opened this issue Jul 22, 2020 · 10 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jul 22, 2020

Reported in phetsims/ph-scale#180, but occurs with FaucetNode in all sims (ph-scale, BLL, EFAC, FPAF, Wave Interference). This is blocking RC for ph-scale and ph-scale-basics.

The mouseArea for FaucetNode's knob (aka "shooter") is not working correctly. It looks like this with ?showPointerAreas in ph-scale:

screenshot_436

But the lower 25% of the mouseArea is not responsive, and the cursor doesn't change. See the video capture in phetsims/ph-scale#180.

If I comment out this line in FaucetNode.js, then the default mouseArea works correctly:

355   knobNode.mouseArea = knobNode.localBounds.dilatedXY( config.mouseAreaXDilation, config.mouseAreaYDilation );

This line of code was modified by @chrisklus in 6bdfe83. Here's the diff:

-    knobNode.touchArea = Shape.rectangle( -dxTouch, -dyTouch, knobNode.width + 2 * dxTouch, knobNode.height + 2 * dyTouch );
-    knobNode.mouseArea = Shape.rectangle( -dxMouse, -dyMouse, knobNode.width + 2 * dxMouse, knobNode.height + 2 * dyMouse );
+    knobNode.mouseArea = knobNode.localBounds.dilatedXY( config.mouseAreaXDilation, config.mouseAreaYDilation );
@pixelzoom pixelzoom self-assigned this Jul 22, 2020
@pixelzoom pixelzoom changed the title FaucetNode mouseArea isn't behaving correctly FaucetNode pointer areas aren't behaving correctly Jul 22, 2020
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 22, 2020

This affects both touchArea and mouseArea, title of this issue changed accordingly. As noted in phetsims/ph-scale#180:

it looks to me like touchArea is also broken. Testing ph-scale 1.4.0-rc.1 on iPad6, I can't snag in the bottom part of the touchArea.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 22, 2020

This problem is not present in 1.2.20 (8/5/19). It is present in 1.3.0 (8/7/19). There were no changes to FaucetNode.js during that window. @chrisklus's change in 6bdfe83 was made 8/19/19.

@jonathanolson would you have some time to investigate this?

@jonathanolson
Copy link
Contributor

The cause seems to be this Scenery Image node, which is layered on top of the shooter:

image

This is triggered by an inputListener on the FaucetNode specifically. I'd recommend just changing the visual stacking order, @pixelzoom is that possible?

@pixelzoom
Copy link
Contributor Author

@jonathanolson thanks for finding the problem! Changing the order is definitely doable, and I've done that in the above commit.

The Node that you've highlighted above is bodyNode, and another way to solve this would be to set bodyNode to pickable: false. So I've done that too, in case the order is ever changed for some other reason.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 23, 2020

@KatieWoe please verify this in master, with a few sims that use FaucetNode (ph-scale, BLL, EFAC, FPAF, Wave Interference). If it looks good, assign back to me so that I can cherry-pick for ph-scale and ph-scale-basics RCs.

@jonathanolson
Copy link
Contributor

and another way to solve this would be to set bodyNode to pickable: false. So I've done that too, in case the order is ever changed for some other reason.

That might be a problem if (a) you want to prevent clicks on things behind a faucet (likely), or (b) you want a draggable faucet (less likely). Having it normal pickability still sounds correct, UNLESS its hit area isn't affected by that.

@pixelzoom
Copy link
Contributor Author

Good point. I've removed pickable:false for bodyNode.

@pixelzoom
Copy link
Contributor Author

Note to self: I need to cherry-pick ae6db07 and a78d82a.

@KatieWoe
Copy link

Does look fixed on master. WI has a button, rather than a knob, so I'm not sure if the issue ever showed up there at all. Others look ok.

pixelzoom added a commit to phetsims/ph-scale that referenced this issue Jul 23, 2020
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Jul 23, 2020
@pixelzoom
Copy link
Contributor Author

Cherry-picks done for ph-scale and ph-scale-basics.

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