Support for alternative input

pixelzoom commented 1 year ago

@kathy-phet asked me to add alternative input to ph-scale and ph-scale-basics.

This includes:

[x] add "supportsInteractiveDescription": true to package.json
[x] keyboard traversal order
[x] KeyboardDragListener for pH probe in the Macro screen
[x] KeyboardDragListener for water faucets in Macro and Micro screens - common code, see https://github.com/phetsims/scenery-phet/issues/773
[x] KeyboardDragListener for dropper in Macro and Micro screens
[ ] PressListener for momentary button on dropper - common code, see https://github.com/phetsims/sun/issues/796
[x] KeyboardDragListener for interactive graph indicators in the MySolution screen
[x] hot keys?
[x] keyboard help dialogs

pixelzoom commented 1 year ago

@kathy-phet @arouinfar FYI...

After discussion with @arouinfar, here's the list of things that are not going to be straightforward:

Momentary button on dropper
Dragging the dropper while the momentary button is pressed
Faucet interaction and keyboard help, because it's common code (will require design)
Which draggable things should have their own keyboard help, vs something like "Move Draggable Items" (MoveDraggableItemsKeyboardHelpSection) that appears in Geometric Optics?

pixelzoom commented 1 year ago

After consulting with @arouinfar, no hot keys are necessary.

pixelzoom commented 1 year ago

Alt input for ph-scale is currently blocked by 2 issues:

That said, it's at a point where it's ready for @arouinfar to review.

I took my best guess at traveral order and keyboard help.

KeyboardDragListeners (for the draggable things) are configured like this:

eye dropper: dragVelocity: 300, shiftDragVelocity: 20
pH probe: dragVelocity: 300, shiftDragVelocity: 20
graph indicators: dragVelocity: 300, shiftDragVelocity: 40 (the indicator will not move with smaller values, due to precision of the model value)

Let me know what you'd like to change. And assign this issue back to me. Thanks!

arouinfar commented 1 year ago

Thanks @pixelzoom! Overall, looks good. The Keyboard Help content seems fine for now, but we'll likely need to adjust it once the FaucetNode interaction is finalized.

The focus order on the Macro screen is perfect, but Micro and My Solution feels slightly off. The order seems like a natural way to traverse through the interactive objects based on their visual layout. However, the graph area controls are secondary to the beaker area controls, and I think it will take too many tabs to get to controls like the dropper, faucets, and pH probe.

@pixelzoom can we try a different order for the Micro and My Solutions screens? Let's try: controls in box A > controls in box B > ResetAllButton > navbar. The relative focus order of the controls within each box can remain as-is.

pixelzoom commented 1 year ago

The Keyboard Help content seems fine for now, but we'll likely need to adjust it once the FaucetNode interaction is finalized.

Yep, I'm planning on it.

... can we try a different order for the Micro and My Solutions screens? Let's try: controls in box A > controls in box B > ResetAllButton > navbar. The relative focus order of the controls within each box can remain as-is.

Done, ready for review.

arouinfar commented 1 year ago

@pixelzoom focus order all looks good, but I discovered the OH- graph indicator isn't working as it should.

Steps to reproduce:

Open My Solution
Tab to the OH- graph indicator
Use either up or down arrow, and you'll see this:

https://user-images.githubusercontent.com/8419308/194587028-a1823d65-e297-4236-b874-e76981acb57b.mov

This happens in macOS 12.4 in Chrome 106 and Safari 15.5.

pixelzoom commented 1 year ago

Reproduced. And the H3O+ indicator does not have this problem.

pixelzoom commented 1 year ago

The KeyboardDragListener for the OH- indicator was incorrrectly wired up to the same arguments as the H3O+ indicator --- a copy-paste error. Fixed in the above commit. Good catch @arouinfar, please verify in master.

arouinfar commented 1 year ago

Looks great, thanks @pixelzoom!

arouinfar commented 1 year ago

Not sure if this should be left open since there is outstanding common code work related to this issue, so back to @pixelzoom.

pixelzoom commented 1 year ago

Yes, let's leave this open and "on-hold".

@kathy-phet @arouinfar reminder that further work is blocked by:

I don't know what the timeframe is for getting pH Scale into QA, but it's unlikely to happen any sooner than 1 week after those issues are addressed.

pixelzoom commented 1 year ago

@kathy-phet @arouinfar What is the the status of the work that is blocking this issue? Has it been started? Is it scheduled? Who is doing it?

arouinfar commented 1 year ago

I've provided feedback for both of the blocking issues, and it's my understanding that it's up to the a11y team to get things working. I hoped that these issues would come up during this week's sprint, but I suppose there was more fundamental work to be done.

Momentary buttons: we already have an alt input pattern for standard toggle buttons, so I think we could try treating momentary buttons like standard toggle buttons for the purposes of alt input and just see how it goes.
FaucentNode: current implementation in pH Scale is passable, but needs improvement and doesn't scale to all instances of FaucetNode. I've made suggestions in the scenery-phet issue.

We likely need to schedule a design meeting with the relevant team members to push this work along. There's also a need for some sound design too, see https://github.com/phetsims/ph-scale/issues/248.

That said, pH Scale isn't high priority for PhET-iO releases, so realistically we're looking at Q1 or Q2 2023. Really up to @kathy-phet how she wants to prioritize this work.

pixelzoom commented 1 year ago

@kathy-phet wrote me on Slack:

I chatted with Amy and we will move forward with pH Scale / pH Scale Basics phet-io. Disable the sounds and make a call on whether to leave the existing keyboard navigation in, but not label it as supported, or just disable it entirely. Amy said it is helpful, even if it isn’t perfect. ...

Slack discussion with @arouinfar:

@pixelzoom

I would be fine if the sim was fully functional. But the momentary button on the dropper does not work at all. So my preference would be to disable alt input for the next release.

@arouinfar

There is precedent for leaving alt input on, but not advertising it. For example, we didn’t turn off alt input for Waves Intro, even though there are a few components that don’t get focus at all (everything in the toolbox).

@pixelzoom

Is there a precedent for having things that don’t work get focus? Should I at least remove the dropper from the traversal order? Is there a precedent for having things that don’t work get focus? Should I at least remove the dropper from the traversal order, since it doesn’t work at all? I also feel like the faucets should be removed from the traversal order, because their usability with the keyboard is currently awful.

@pixelzoom

Another reason to turn off alt input might be the fact that keyboard help is currently leaking memory in PhET-iO versions — and that’s a blocking issue. See https://github.com/phetsims/ph-scale/issues/253 and https://github.com/phetsims/scenery-phet/issues/769. If we turn off alt input, we can also turn off keyboard help.

@arouinfar

Let’s just turn it off completely, then.

@pixelzoom

OK, I really think that’s best.

pixelzoom commented 1 year ago

In https://github.com/phetsims/ph-scale/issues/249#issuecomment-1319265006, @arouinfar said:

Let’s just turn it off completely, then.

Alternative input was disabled in the above commits, by removing "supportsInteractiveDescription": true from package.json for ph-scale and ph-scale-basics.

Unassigning and labeling this issue as deferred, to be addressed sometime after publication of 1.6 PhET-iO.

pixelzoom commented 1 year ago

In the above commits, I also commented out createKeyboardHelpNode in all Screens. Otherwise the keyboard help is created and disposed by the State wrapper, and we'll have the memory leak. These lines of code (noted with a TODO referencing this issue) should be restored when work on this is resumed.

pixelzoom commented 4 weeks ago

In https://github.com/phetsims/ph-scale/commit/2d1e8b0b71c5ce4b7f01db6a588300e86919c0d0, @jonathanolson added "supportsInteractiveDescription": true to package.json for the Description prototype. This turns on alt input, which is still not completely supported. So labeling this issue as blocks-sim-publication.

As for the status of alt input...

FaucetNode support for alt input was added in https://github.com/phetsims/scenery-phet/issues/773 and https://github.com/phetsims/scenery-phet/issues/839.
Momentary buttons still do not support alt input, and this sim uses RoundMomentaryButton specifically.

pixelzoom commented 3 weeks ago

https://github.com/phetsims/ph-scale/commit/420ac6131529517b28c10eb5554f6f00e767b3f3 by @jonathanolson will need to be reviewed when work on alternative input resumes. That change prevents setting the pdomOrder when running with ?supportsDescriptionPlugin.

jonathanolson commented 2 weeks ago

Noted in discussions that the description logic plugin will control the pdomOrder when the query parameter is true.

Additionally, committed above to put the DescriptionRegistry change above under the query parameter, so that our sim actions are 100% query parameter feature-flag controlled.

pixelzoom commented 1 week ago

In Slack#description-tooling, there was a request to add alt input support for FaucetNode. That support has been available for a long time. The issue is that alt input is "off" for ph-scale and ph-scale-basics. So...

As a result of the above commits, alt input is now "on" by default for ph-scale and ph-scale-basics. I checked FaucetNode, and it’s working as expected. Both sims also have the “Faucet Controls” section in their keyboard help.

phetsims / ph-scale

Support for alternative input #249