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 addHotkey does not work with 1 key. #1331

Closed
pixelzoom opened this issue Dec 16, 2021 · 4 comments
Closed

KeyboardDragListener addHotkey does not work with 1 key. #1331

pixelzoom opened this issue Dec 16, 2021 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

KeyboardDragListener addHotkey is broken when 1 key is specified.

This works:

    keyboardDragListener.addHotkeys( [ {
      keys: [ KeyboardUtils.KEY_J, KeyboardUtils.KEY_W ],
      callback: () => {
        console.log( 'escape' );
        ruler.visibleProperty.value = false;
        this.iconNode.focus();
      }
    } ] );

This doesn't work:

    keyboardDragListener.addHotkeys( [ {
      keys: [ KeyboardUtils.KEY_J ],
      callback: () => {
        console.log( 'escape' );
        ruler.visibleProperty.value = false;
        this.iconNode.focus();
      }
    } ] );

We also tried holding the key down for > 800ms, and setting hotkeyHoldInterval: 0.

@zepumph
Copy link
Member

zepumph commented Dec 17, 2021

Bug fixed. Thanks for the help @jessegreenberg. @pixelzoom I updated the usage in GO. Anything else here?

@pixelzoom
Copy link
Contributor Author

Looks great,and working great in geometric optics. Thanks for the quick turnaround. Closing.

@jessegreenberg
Copy link
Contributor

This fix introduced a bug identified in #1334. Just checking the length of hotkeysDownList wasn't sufficient because that means the callback would fire if any one of the hotkeys are down. It is fixed with the above commit, @zepumph is this change OK?

@jessegreenberg jessegreenberg assigned zepumph and unassigned pixelzoom Jan 3, 2022
@zepumph
Copy link
Member

zepumph commented Jan 3, 2022

Aha! Yes that makes sense. Sorry about that. I tested before and after changes in GO and BASE, all looks good to me.

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

3 participants