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

Use KeyboardListener instead of globalKeyStateTracker and custom listeners #1126

Closed
19 tasks done
jessegreenberg opened this issue Jul 20, 2023 · 6 comments
Closed
19 tasks done
Assignees

Comments

@jessegreenberg
Copy link

jessegreenberg commented Jul 20, 2023

Searching for globalKeyStateTracker. keydown:, and keyup: I found this list of files that might be improved by using KeyboardListener:

Important notes:

  • Remember to dispose the listener, if you need to.

Examples:

globalKeyStateTracker was often used to add support for global hotkeys. It is better to use KeyboardListener now for this case. Here is an example of the old style an an example switching to KeyboardListener.

Old style
globalKeyStateTracker.keydownEmitter.addListener( event => {

  // Make sure this Node is visible, pdomVisible, and enabled for input.
  if ( this.pdomDisplayed && this.enabledProperty.value ) {
    if ( KeyboardUtils.isKeyEvent( event, KeyboardUtils.KEY_ESCAPE ) ) {
      // Escape
      ...
    }
    else if ( globalKeyStateTracker.altKeyDown && KeyboardUtils.isKeyEvent( event, KeyboardUtils.KEY_C ) ){
      // Alt+C
      ...
    }
  }
} );
Using KeyboardListener
const keyboardListener = new KeyboardListener( {
  keys: [ 'escape', 'alt+c' ],
  callback: ( event, listener ) => {
    if ( listener.keysPressed === 'escape' ) {
      // escape key was pressed
    }
    else if ( listener.keysPressed === 'alt+c' ) {
      // alt+c were pressed
    }
  },
  
  // By making this listener "global" it will fire no matter where focus is in the document as long as
  // myNode is visible and has input enabled. If this is false, callback will only fire when myNode has keyboard focus.
  global: true
} );

myNode.addInputListener( keyboardListener );

Often, a custom input listener was used for keydown and keyup events added to a particular Node. It might be better to use KeyboardListener for these for now.

Old style
    preferencesTabs.addInputListener( {
      keydown: event => {
        if ( KeyboardUtils.isKeyEvent( event.domEvent, KeyboardUtils.KEY_DOWN_ARROW ) ) {
          this.focusSelectedPanel();
        }
      }
    } );
Using KeyboardListener
    preferencesTabs.addInputListener( new KeyboardListener( {
      keys: [ 'arrowDown' ],
      callback: () => {
        this.focusSelectedPanel();
      }
    } ) );
@jessegreenberg
Copy link
Author

jessegreenberg commented Jul 20, 2023

  • localeProperty.ts

This one is for ?keyboardLocaleSwitcher. Since there is no view component to add a listener to, using the global globalKeyStateTracker is probably best but curious if @samreid has an opinion so Ill put his name next to that one. An alternative could be to move this code to a view component, but that is not a big improvement in my opinion.

EDIT: Similar story for phetioDevSaveLoad.ts

@pixelzoom
Copy link
Contributor

I've attempted to follow the example provided, and it's not working in fourier-making-waves WaveGameLevelNode. So I'm moving that conversion to phetsims/fourier-making-waves#237.

@pixelzoom pixelzoom removed their assignment Jul 21, 2023
jessegreenberg added a commit to phetsims/sun that referenced this issue Aug 3, 2023
jessegreenberg added a commit to phetsims/sun that referenced this issue Aug 3, 2023
@jbphet
Copy link

jbphet commented Aug 3, 2023

Can a static list of range keys be added that would parallel the functionality we previously got from KeyboardUtils.isRangeKey()?

jessegreenberg added a commit to phetsims/scenery-phet that referenced this issue Aug 3, 2023
jbphet added a commit to phetsims/energy-forms-and-changes that referenced this issue Aug 3, 2023
@jessegreenberg
Copy link
Author

Can a static list of range keys be added that would parallel the functionality we previously got from KeyboardUtils.isRangeKey()?

Thanks @jbphet, I added functions for this in the above commit. They are exported from EnglishStringToCodeMap.

@jessegreenberg
Copy link
Author

Regarding #1126 (comment), after seeing the usage in phetsims/energy-forms-and-changes@3a708eb, I changed it a little. Functions and constants were moved to EnglishStringKeyUtils.

jessegreenberg added a commit to phetsims/energy-forms-and-changes that referenced this issue Aug 3, 2023
zepumph added a commit to phetsims/ratio-and-proportion that referenced this issue Aug 3, 2023
jbphet added a commit to phetsims/faradays-law that referenced this issue Aug 4, 2023
@jbphet
Copy link

jbphet commented Aug 4, 2023

I changed the ones assigned to me with the exception of FaradaysLawKeyboardDragListener. This one is a drag listener rather than a general keyboard listener, and it extends KeyboardDragListener, which doesn't appear to be deprecated or anything. In other words, it doesn't seem like an improvement to migrate this one, so I'm not going to.

@jbphet jbphet removed their assignment Aug 4, 2023
@zepumph zepumph closed this as completed Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants