Add keyboard input to Keypad

[zepumph]

From phetsims/projectile-motion#307, @matthew-blackman and I would like to explore adding keyboard input to Keypad.

[zepumph]

@matthew-blackman and I felt good enough getting to a commit point here. We wanted to note that there is still open keypad work

#271
#283
#758

But we still wanted to brainstorm some alt-input support for the keypad. We made some decisions along the way that we don't assume are set, but wanted to have something to work with. Open questions:

• We must focus the entire keypad (as a PDOM element) in order to enter input. How else could we do this? Could it be a global listener that is only active if the keypad is visible? What if 2 keypads are visible?
• We removed focus from each individual button, that seemed like a no-brainer, but perhaps is worthy of more discussion.
• We are using the minus key for the +/- key, currently plus doesn't work, and it isn't clear what we would do if we made a calculator that had that key AND the subtraction sign, though we likely won't get there for a while.
• In typescript we ran into one spot where we needed to fudge things a bit, since KeyboardListener works best when it receieves a tuple type of the keys it is listening for. We support any dynamic set of Keys, so it was worth typecasting to a OneKeyStroke[] instead. The way to make things fully type safe is to parametrize Key and Keypad on the keystroke and keys set (respectively) that it is provided. This is a bit overkill in my book.

I'd love to speak with @jessegreenberg at some point to discuss some ideas about global listening, as he has more experience in that department. Next @matthew-blackman and I will look at using this in Projectile-Motion over in phetsims/projectile-motion#307

[terracoda]

Is there a working example? I looked in projectile motion, but didn't see a keypad.

[arouinfar]

@terracoda I don't think Projectile Motion has alt input enabled, though I could be wrong. Even so, the keypad is a bit hard to find. It's on the Lab screen, but you have to select "Custom" in the ComboBox and then click on the edit button next to the text field to bring up the keypad.

[zepumph]

It works, but interactive description isn't on by default. Try testing it out in with https://phet-dev.colorado.edu/html/projectile-motion/1.1.0-dev.39/phet/projectile-motion_all_phet.html?supportsInteractiveDescription=true&screens=4

[zepumph]

@AgustinVallejo and I would like to have this in the alt-input of My Solar System, since it is already quite close. Assigning ourselves.

[zepumph]

With phetsims/my-solar-system#86 making good progress on alt input in My Solar System, I took a stab at a few improvements here. We now ensure that you don't have duplicate keys kverwriting each other in the keyboard listener, and we support the NumberPad keys.

I think it would be best to do a co-review with @jessegreenberg about the current progress of alt input in Keypad,

I also would like to a @arouinfar and/or @terracoda take it for a test drive. You can try it in My Solar System by pressing clicking the number display next to the mass sliders. that opens a KeypadDialog:

Have a look here: https://phet-dev.colorado.edu/html/my-solar-system/1.1.0-dev.4/phet/my-solar-system_en_phet.html

[jessegreenberg]

Great, lets talk soon! I think phetsims/scenery@ffad825 is causing issues on CT like

Uncaught Error: Assertion failed: How can we have a null key from a keydown in KeyboardDragListener?

Adding blocks-publication until we can take a look.

[zepumph]

I wish I could use space or enter to submit the value after typing it in, rather than tabbing forward to the enter button.

I think that the reason it hadn't been done yet is because the keypad had no understanding of "submit". That exists outside of the Keypad class. Luckily!!! We have a KeypadDialog, which brings the two together in common code. What a perfect spot to add what you are talking about, and gain this support for My Solar System. Please review, as it submits from enter and space.

Some varieties of keypad include keys for variables. For example, Area Model Algebra has x and x2 keys. Will this work generalize to any keypad? I'm not sure how to type x2 for this sort of keypad entry, so maybe the variable keys would be separately focused? I understand if this is beyond the scope of the current work, but just want to make folks aware of the different sorts of keypads.

My effort focuses around what was already in Keypad. There are many other spots where new Key() is called for special keys with special accumulators, but in those sim-specific cases, they will need to support the keyboard keys x^2 etc maps to. That seems like a great item to defer without adding technical debt, since it isn't in common code. Instead I was working on the common keys:

scenery-phet/js/keypad/Keypad.ts

Lines 33 to 48 in d5dcf86

	const _0 = new Key( '0', KeyID.ZERO, { keyboardIdentifiers: [ '0', 'Numpad0' ] } );
	const _1 = new Key( '1', KeyID.ONE, { keyboardIdentifiers: [ '1', 'Numpad1' ] } );
	const _2 = new Key( '2', KeyID.TWO, { keyboardIdentifiers: [ '2', 'Numpad2' ] } );
	const _3 = new Key( '3', KeyID.THREE, { keyboardIdentifiers: [ '3', 'Numpad3' ] } );
	const _4 = new Key( '4', KeyID.FOUR, { keyboardIdentifiers: [ '4', 'Numpad4' ] } );
	const _5 = new Key( '5', KeyID.FIVE, { keyboardIdentifiers: [ '5', 'Numpad5' ] } );
	const _6 = new Key( '6', KeyID.SIX, { keyboardIdentifiers: [ '6', 'Numpad6' ] } );
	const _7 = new Key( '7', KeyID.SEVEN, { keyboardIdentifiers: [ '7', 'Numpad7' ] } );
	const _8 = new Key( '8', KeyID.EIGHT, { keyboardIdentifiers: [ '8', 'Numpad8' ] } );
	const _9 = new Key( '9', KeyID.NINE, { keyboardIdentifiers: [ '9', 'Numpad9' ] } );
	const WIDE_ZERO = new Key( '0', KeyID.ZERO, { horizontalSpan: 2, keyboardIdentifiers: [ '0', 'Numpad0' ] } );
	const DECIMAL_KEY = new Key( '.', KeyID.DECIMAL, { keyboardIdentifiers: [ 'period', 'NumpadDecimal' ] } );
	const BACKSPACE_KEY = new Key( ( new BackspaceIcon( { scale: 1.5 } ) ),
	KeyID.BACKSPACE, { keyboardIdentifiers: [ 'backspace' ] } );
	const PLUS_MINUS_KEY = new Key( `${PLUS_CHAR}/${MINUS_CHAR}`, KeyID.PLUS_MINUS, {
	keyboardIdentifiers: [ 'minus', 'plus', 'NumpadSubtract', 'NumpadAdd' ]

Any other thoughts?

[jessegreenberg]

Regarding #790 (comment).

Fast typing is not working well because of this line: https://github.com/phetsims/scenery/blob/0e14250441d1c4a400ff92e6e635660bc106e54a/js/listeners/KeyboardListener.ts#L252

To prevent key groups that overlap keys (like "alt+r" and "alt+shift+r") from firing at the same time, the listener fires when exclusively the keys of a particular group are down.

With fast typing, multiple number keys are down at once and so none of them fire.

[zepumph]

If we don't want to have something like allowKeyOverlap for a general solution. Can we hard code a list of "modifier keys" that the exclusivety applies to. I don't think that should bleed into something like pressing keys: "4434" quickly. Also I could see that getting buggy when we try to use this in KeyboardDragListener for "wasd" keys.

[jessegreenberg]

You are so right. I think we should have something like allowKeyOverlap for this and maybe it should even be true by default. Previously that option was exploring validation to try to prevent multiple KeyboardListeners from overlapping keys for phetsims/scenery#1445.

[jessegreenberg]

I added an option allowOtherKeys for this that will allow keys other than those provided in the group to be down, and the option is now used by Keypad. @zepumph are you OK with this?

I understand if this is beyond the scope of the current work, but just want to make folks aware of the different sorts of keypads.

Just brainstorming on this consideration, not saying we should do it: All buttons themselves could remain focusable while number input is still supported by a listener on the Keypad. A groupFocusHighlight surrounds the Keypad to hopefully indicate that you can type numbers.

[arouinfar]

Please review, as it submits from enter and space.

Love it! 🎉 🎉 🎉

My effort focuses around what was already in Keypad. There are many other spots where new Key() is called for special keys with special accumulators, but in those sim-specific cases, they will need to support the keyboard keys x^2 etc maps to. That seems like a great item to defer without adding technical debt, since it isn't in common code. Instead I was working on the common keys:

Makes sense to me, thanks for the clarification @zepumph.

Just brainstorming on this consideration, not saying we should do it: All buttons themselves could remain focusable while number input is still supported by a listener on the Keypad.

Thanks @jessegreenberg that sounds like a good path forward when the time comes.

[zepumph]

Thanks @jessegreenberg that sounds like a good path forward when the time comes.

Or, we could also add not that any Key that doesn't have a keyboardIdentifier provided, can automatically be focusable, that will make it very straight forward as to what keys are in fact supported by the common code implementation. I'll see if that is easy. It is kinda like future proofing ourselves.

[zepumph]

Ok. two pieces we are tracking in the same issue here:

1. I made it so individual buttons are focusable if they don't support keyboardIdentifiers. I tested this out in area model and found an easy fix where those Keys could use common code to get the keyboard support. Again, I didn't do anything to the x2 case, and that's great! We will get to that for that specific sim later. I feel good about this.
2. @jessegreenberg, I freaking love your option! allowOtherKeys is greatly helpful in our case, AND I think it will be used time and time again in the future. I wonder if we need to apply some heuristic about what an "other key" is. Modifier keys feel like they actually aren't "other" keys. For example, I think that "exclusively down" should be used to ensure that "c" isn't fired when "ctrl+c" is fired. That would be weird behavior. I know that PhET has a bit of a strange definition of modifier key (not exclusively alt/ctrl/shift), but perhaps we can treat those specially? Would you like a quick conversation about this?

[zepumph]

@jessegreenberg and I worked on (2) and got a bit stuck. We will pick it up over in phetsims/scenery#1445. We are still fine with the current option, as it is helping us, though it is overly complicated.

Ready to close!

[zepumph]

Over in phetsims/my-solar-system#86, @samreid found a bug in keypadLayer where pressing enter should also close the dialog. This occurs when you press the enter button, but when you press enter/space while the keypad is highlighted, it doesn't close the keypad. Let's fix it!

[zepumph]

This is a bit workaroundy, because the bug was based on the weirdness with keydown not forwarding correctly, but the keyup listener is quite idiomatic to "submitting" on our project, so I feel good about this solution. It also mimics how the actual enter button works just below the keypad.