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

Problems with implementation and help-content for hotkeys. #716

Closed
pixelzoom opened this issue Jan 4, 2022 · 4 comments
Closed

Problems with implementation and help-content for hotkeys. #716

pixelzoom opened this issue Jan 4, 2022 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 4, 2022

The way that hotkeys are specified seems odd (and brittle) to me. For example, in BASE BalloonNode.js, here’s the implementation of J+W, which moves the balloon to the wall:

    this.keyboardDragHandler.addHotkeys( [
      {
        keys: [ KeyboardUtils.KEY_J, KeyboardUtils.KEY_W ],
        callback: () => {
          this.jumpBalloon( new Vector2( X_POSITIONS.AT_WALL, model.getCenterY() ) );
        }
      },

Here’s the corresponding help implementation in BASEKeyboardHelpContent.js:

    const jumpToWallRow = KeyboardHelpSection.createJumpKeyRow( 'W', jumpCloseToWallLabelString, jumpsCloseToWwallDescriptionString );

Two major problems:

  • There are two entirely different ways of specifying J+W. There are even two different ways of specifying the character 'W'!
  • Nothing is keeping these in in sync. Nothing is shared. If I change one, I've broken the other.

Are you sure this is the pattern that you want propagated to all sims?

@pixelzoom
Copy link
Contributor Author

This might be a duplicate issue. I know that I encountered this same problem when working on Fourier back in June 2021, but don’t recall if I reported it, or which repo it might have been in.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 4, 2022

Reviewing scenery-phet issues for Q4 planning...

No response in 10 months. Assigning to @kathy-phet to prioritize.

@zepumph
Copy link
Member

zepumph commented Oct 6, 2022

Subset of phetsims/scenery#1298

@jessegreenberg
Copy link
Contributor

This is a duplicate of phetsims/scenery#1266 and we will continue with this issue there. (There are more notes over there so Ill close this one).

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

5 participants