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

KeyboardDragListener: rename dragVelocity and shiftDragVelocity #1603

Closed
pixelzoom opened this issue Jan 27, 2024 · 4 comments
Closed

KeyboardDragListener: rename dragVelocity and shiftDragVelocity #1603

pixelzoom opened this issue Jan 27, 2024 · 4 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 27, 2024

This came up during discussion about KeyboardDragListener drag speeds in #1601. It's not the first time it's come when when designing how to configure KeyboardDragListener.

In 6910117, @jessegreenberg added the current KeyboardDragListener options for controlling drag speed:

  // While a direction key is held down, the target will move by this amount in view coordinates every second.
  // This is an alternative way to control motion with keyboard than dragDelta and produces smoother motion for
  // the object. dragVelocity and dragDelta options are mutually exclusive. See dragDelta for more information.
  dragVelocity?: number;

  // While a direction key is held down with the shift modifier key, the target will move by this amount in view
  // coordinates every second. Shift modifier should produce more fine-grained motion so this value needs to be less
  // than dragVelocity if provided. This is an alternative way to control motion with keyboard than dragDelta and
  // produces smoother motion for the object. dragVelocity and dragDelta options are mutually exclusive. See dragDelta
  // for more information.
  shiftDragVelocity?: number;

Velocity is a vector; speed is a scalar. So these options are incorrecly named - they should be dragSpeed and shiftDragSpeed. Let's change them so we don't continue to have this confusion, especially since PhET is a STEM project.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 27, 2024

There are 28 occurrences of dragVelocity:, 29 occurrences of shiftDragVelocity:. So this looks like a relatively straightforward change. If @jessegreenberg doesn't have time to do this, and approves of the proposed change, I'll be happy to handle it.

@jessegreenberg
Copy link
Contributor

Very good point! These are not vectors, "speed" sounds great.

@pixelzoom
Copy link
Contributor Author

1/28/2024 dev meeting:

Consensus was to change to dragSpeed and shiftDragSpeed. I'll do this.

pixelzoom added a commit to phetsims/balloons-and-static-electricity that referenced this issue Jan 29, 2024
pixelzoom added a commit to phetsims/faradays-electromagnetic-lab that referenced this issue Jan 29, 2024
@pixelzoom
Copy link
Contributor Author

PSA to Slack#developer:

PSA: KeyboardDragListener options are now named dragSpeed and shiftDragSpeed. Changes to repos that I am not responsible for include: balloons-and-static-electricity, friction, keplers-laws, projectile-data-lab, scenery-phet (MeasuringTapeNode), and solar-system-common.

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

2 participants