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

Add KeyboardDragListeners to MeasuringTapeNode #798

Closed
zepumph opened this issue Mar 3, 2023 · 5 comments
Closed

Add KeyboardDragListeners to MeasuringTapeNode #798

zepumph opened this issue Mar 3, 2023 · 5 comments

Comments

@zepumph
Copy link
Member

zepumph commented Mar 3, 2023

Over in phetsims/my-solar-system#86 it seemed worth a bit of exploration. Seems like the hardest part is just reusing the same code efficiently between dragListener and keyboardDragListener.

@zepumph zepumph self-assigned this Mar 3, 2023
@zepumph
Copy link
Member Author

zepumph commented Mar 3, 2023

I think it would be nice for @jessegreenberg to review this. The conversion was pretty straight forward. The worst part was that I know that the DragListener is using a bit of an old pattern for supporting dragging, and it wasn't very plug and play to convert it to use positionProperty + offset + transform + dragBoundsProperty settings. Thus, the options passed to DragListener and KeyboardDragListener aren't very aligned. Is there anything else you see?

@jessegreenberg
Copy link
Contributor

Changes to MeasuringTapeNode and KeyboardDragListener look right to me, thanks!

If the intent for keyboard input is "smooth" dragging, can we use the velocity options of KeyboardDragListener? I regret the default value of moveOnHoldInterval option, it should be a large value for the "drag delta" behavior. I made the change above, keeping the shift drag for the tip slower than the base. Does that seem OK?

@zepumph
Copy link
Member Author

zepumph commented Mar 6, 2023

I like it! Thanks so much.

@zepumph zepumph closed this as completed Mar 6, 2023
@zepumph
Copy link
Member Author

zepumph commented Mar 8, 2023

  • Add focus highlight around the whole base

@zepumph zepumph reopened this Mar 8, 2023
@zepumph
Copy link
Member Author

zepumph commented Mar 8, 2023

On second thought, the design team finds this box acceptable, because the measuring tape image is already kinda boxy:

image

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