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

Possible to have an arrow with no height #430

Closed
Tracked by #938
KatieWoe opened this issue Apr 19, 2022 · 5 comments
Closed
Tracked by #938

Possible to have an arrow with no height #430

KatieWoe opened this issue Apr 19, 2022 · 5 comments
Labels

Comments

@KatieWoe
Copy link
Contributor

Device
iPad
OS
iPadOS 15.4.1
Browser
Safari
Problem Description
For phetsims/qa#798
If you drag the arrow to the middle line exactly, it doesn't jump to a taller value and seems to disappear. You can still grab it with touch by swiping over it. May be possible on other devices, but I've mostly seen it on iPad.
Visuals
0AB86F52-9A53-441B-BBFD-62DEAF5FDF1B

@KatieWoe KatieWoe added the type:bug Something isn't working label Apr 19, 2022
@pixelzoom
Copy link
Contributor

Good find!

Here's the relevant code in ArrowObjectNode.ts that snaps the arrow to its minimum magnitude:

    const end = ( sign: 1 | -1 ) => {
      const arrowLength = arrowObject.positionProperty.value.y - optic.positionProperty.value.y;
      if ( Math.abs( arrowLength ) < SNAP_TO_MIN_MAGNITUDE ) {
        const x = arrowObject.positionProperty.value.x;
        const y = sign * Math.sign( arrowLength ) * SNAP_TO_MIN_MAGNITUDE;
        arrowObject.positionProperty.value = new Vector2( x, y );
      }
    };

The problem line is:

        const y = sign * Math.sign( arrowLength ) * SNAP_TO_MIN_MAGNITUDE;

When arrowLength is exactly zero, Math.sign( 0 ) returns 0, so y = 0.

Fixed in the above commit. @KatieWoe please verify in master. Be sure to test with both mouse, touch, and keyboard dragging. If it looks OK, change label to "fixed-awaiting-deploy".

@KatieWoe
Copy link
Contributor Author

It looks good on master with all three inputs. However, I will note that in the original dev test I was never able to do this with a mouse, and it seemed almost impossible with keyboard nav, so confirming those is a bit harder.

@pixelzoom
Copy link
Contributor

... I will note that in the original dev test I was never able to do this with a mouse, and it seemed almost impossible with keyboard nav, so confirming those is a bit harder.

Understood. I wanted you to test with mouse and keyboard to verify that the fix hadn't broken anything more general with drag handling of the arrows.

@KatieWoe
Copy link
Contributor Author

Things looked ok from what I saw.

@KatieWoe
Copy link
Contributor Author

KatieWoe commented May 5, 2023

Looks ok in dev.1

@KatieWoe KatieWoe closed this as completed May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants