Alt input and UI sound for FineCoarseSpinner (and NumberSpinner?)

[pixelzoom]

Needed for Gas Properties suite PhET-iO release, phetsims/gas-properties#191.

In phetsims/gas-properties#213:

• FineCoarseSpinner - in Particles accordion boxes. How should alt input behave? Should it be like NumberSpinner? Should each button get focus and operate like a push button? Or should the entire FineCoarseSpinner get focus and use left/right arrows for increment/decrement and shift for fine/coarse?

In phetsims/gas-properties#214:

• FineCoarseSpinner - all 4 buttons currently have the same default push button sound. Should there be different sounds for increment/decrement and fine/coarse?

[pixelzoom]

From phetsims/gas-properties#213 (comment), here are @terracoda's thoughts on alt input:

@pixelzoom, I agree, I think the FineCoarseSpinner is a number spinner type interaction. I think the number spinners In GFLB and RaP are implemented like discrete sliders.

The FineCoarseSpinner adds a second mode to the number spinner - a single-release and bunch-release. This could be handled in different ways, so it would be good to discuss options.

One option is to make 2 tab stops (like two separate number spinners on top of each other) and have one release singles and the other release bursts, but both share the same model value total. The first Tab Stop encases the single buttons, the second Tab Stop encases the burst buttons.

Option two could use custom modifier keys on a single number spinner to release singles or bursts. E.g. arrow keys -> singles; Page Up/Page Down -->> bursts. Just an idea. Depending on keys, the appropriate button is highlighted.

There could be other options. These are just ideas.

[pixelzoom]

Regardless of what we do with hotkeys or the individual push buttons, we'll need a group focus highlight. I added that in 886af75 and it looks like this:

[pixelzoom]

Looking at NumberSpinner, there seems to be more work to do there too:

• Should there be different sounds for increment/decrement and min/max? (ala Slider)
• Should there be keyboard help for NumberSpinner? And for FineCoarseSpinner? And should they share a common keyboard help because there's a lot of overlap?

[jessegreenberg]

For alt input, I think we will want this to behave like NumberSpinner, here is a patch and a build to help with discussion. I am not sure about the sound question.

Subject: [PATCH] Remove opt out of Interactive Highlights, see https://github.com/phetsims/friction/issues/201
---
Index: js/FineCoarseSpinner.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/FineCoarseSpinner.ts b/js/FineCoarseSpinner.ts
--- a/js/FineCoarseSpinner.ts	(revision 69f356768c26e0d2af3ca23e272f118cc16f3ef2)
+++ b/js/FineCoarseSpinner.ts	(date 1713373102027)
@@ -17,38 +17,50 @@
 import Tandem from '../../tandem/js/Tandem.js';
 import NumberDisplay, { NumberDisplayOptions } from './NumberDisplay.js';
 import sceneryPhet from './sceneryPhet.js';
+import AccessibleNumberSpinner, { AccessibleNumberSpinnerOptions } from '../../sun/js/accessibility/AccessibleNumberSpinner.js';
 
 type SelfOptions = {
   deltaFine?: number; // amount to increment/decrement when the 'fine' tweakers are pressed
   deltaCoarse?: number; // amount to increment/decrement when the 'coarse' tweakers are pressed
   spacing?: number; // horizontal space between subcomponents
   numberDisplayOptions?: StrictOmit<NumberDisplayOptions, 'tandem'>;
-  arrowButtonOptions?: StrictOmit<ArrowButtonOptions, 'numberOfArrows' | 'tandem'>;
+  arrowButtonOptions?: StrictOmit<ArrowButtonOptions, 'numberOfArrows' | 'tandem' | 'focusable'>;
 };
 
-export type FineCoarseSpinnerOptions = SelfOptions & StrictOmit<NodeOptions, 'children'>;
+type ParentOptions = AccessibleNumberSpinnerOptions & NodeOptions;
 
-export default class FineCoarseSpinner extends Node {
+export type FineCoarseSpinnerOptions = SelfOptions & StrictOmit<ParentOptions, 'children' | 'valueProperty' | 'enabledRangeProperty' | 'keyboardStep' | 'shiftKeyboardStep' | 'pageKeyboardStep'>;
+
+export default class FineCoarseSpinner extends AccessibleNumberSpinner( Node, 0 ) {
 
   private readonly disposeFineCoarseSpinner: () => void;
 
   public constructor( numberProperty: NumberProperty, providedOptions?: FineCoarseSpinnerOptions ) {
 
     const options = optionize<FineCoarseSpinnerOptions,
-      StrictOmit<SelfOptions, 'numberDisplayOptions' | 'arrowButtonOptions'>, NodeOptions>()( {
+      StrictOmit<SelfOptions, 'numberDisplayOptions' | 'arrowButtonOptions'>, ParentOptions>()( {
 
       // SelfOptions
       deltaFine: 1,
       deltaCoarse: 10,
       spacing: 10,
 
+      // AccessibleNumberSpinnerOptions
+      valueProperty: numberProperty,
+      enabledRangeProperty: numberProperty.rangeProperty,
+
+      // Instead of changing the value with keyboard step options, the arrow buttons are synthetically
+      // pressed in response to keyboard input so that the buttons look pressed.
+      keyboardStep: 0,
+      shiftKeyboardStep: 0,
+      pageKeyboardStep: 0,
+
       // NodeOptions
       disabledOpacity: 0.5, // {number} opacity used to make the control look disabled
       tandem: Tandem.REQUIRED,
       tandemNameSuffix: 'Spinner',
       phetioFeatured: true,
-      phetioEnabledPropertyInstrumented: true,
-      groupFocusHighlight: true // see https://github.com/phetsims/scenery-phet/issues/794
+      phetioEnabledPropertyInstrumented: true
     }, providedOptions );
 
     const range = numberProperty.range;
@@ -58,6 +70,7 @@
 
     // options for the 'fine' arrow buttons, which show 1 arrow
     const fineButtonOptions: ArrowButtonOptions = combineOptions<ArrowButtonOptions>( {
+      focusable: false,
       numberOfArrows: 1,
       arrowWidth: 12, // width of base
       arrowHeight: 14, // from tip to base
@@ -80,6 +93,7 @@
 
     // options for the 'coarse' arrow buttons, which show 2 arrows
     const coarseButtonOptions = combineOptions<ArrowButtonOptions>( {}, fineButtonOptions, {
+      focusable: false,
       numberOfArrows: 2,
       arrowSpacing: -0.5 * fineButtonArrowHeight, // arrows overlap
 
@@ -145,6 +159,20 @@
     };
     numberProperty.link( numberPropertyListener ); // unlink required in dispose
 
+    // pdom - script clicks of arrow buttons from alt input events so that the buttons look pressed while the key is down
+    const increasedListener = ( isDown: boolean ) => {
+      if ( isDown ) {
+        this.shiftKeyDown ? incrementFineButton.pdomClick() : incrementCoarseButton.pdomClick();
+      }
+    };
+    const decreasedListener = ( isDown: boolean ) => {
+      if ( isDown ) {
+        this.shiftKeyDown ? decrementFineButton.pdomClick() : decrementCoarseButton.pdomClick();
+      }
+    };
+    this.pdomIncrementDownEmitter.addListener( increasedListener );
+    this.pdomDecrementDownEmitter.addListener( decreasedListener );
+
     this.disposeFineCoarseSpinner = () => {
 
       if ( numberProperty.hasListener( numberPropertyListener ) ) {

https://phet-dev.colorado.edu/html/jg-tests/scenery-phet_en_phet.html?screens=6

[jessegreenberg]

Some notes from discussion with @terracoda @pixelzoom @arouinfar:

1. We want it to behave like a NumberSpinner (like the patch in Alt input and UI sound for FineCoarseSpinner (and NumberSpinner?) #847 (comment))
2. The default button sounds are acceptable.
3. We need a generalized KeyboardHelpSection for the components that use AccessibleNumberSpinner.

• It should include options for what icons/controls are included, and what general language to use.
• We think "Spinner" is the most user friendly term (for dialog contents).
• Note that this could be a general class for SliderControlsKeyboardHelpContent.

[pixelzoom]

We need a generalized KeyboardHelpSection for the components that use AccessibleNumberSpinner.

FYI, in Gas Properties I added a placeholder for 'Spinner Controls', so that @arouinfar and I have something for designing the layout of our keyboard help dialogs. See phetsims/gas-properties#215 (comment) and SpinnerControlsKeyboardHelpSection. I'm not implying that this should inform the design or wording of what will be done for AccessibleNumberSpinner, just wanted you to be aware of it.

[terracoda]

Examples KBH Dialogues for Sliders. @pixelzoom provides starting examples for Number spinner and coarse number spinner (previous comment).

Sliders and number spinners share a lot of functionality and behavior. They differ in visual representation. And I think that the number spinners can only have 2 step sizes (default and small) whereas sliders can have 3 (default, small and large).

Default Slider Control from Fourier

Custom Heading for Sliders in Molarity

Even More Customized in GFL

[terracoda]

Hmm, the regular Number Spinner seems more like a slider and can have up to 3 step sizes, but the Fine Coarse Spinner seems to be limited to a default large step (or burst) and a small (or single release) with the Shift modifier.

{{Spinner || Slider || Spinner and Slider}} Controls (regular Number Spinner and Slider)

• Adjust {{spinner || slider || spinner or slider}} [Left][Right] or [Up][Down]
• Adjust in smaller steps [Shift] + [Left][Right] or [Shift] + [Up][Down]
• Adjust in larger steps [Pg Up] + [Pg Dn]
• Jump to minimum [HOME]
• Jump to maximum [END]

Spinner Controls (Fine Coarse Spinner)

• Adjust spinner [Left][Right] or [Up][Down]
• Adjust in smaller steps [Shift] + [Left][Right] || [Shift] + [Up][Down] || [Shift] + [Left][Right] or [Shift] + [Up][Down]
• Jump to minimum [HOME]
• Jump to maximum [END]

Question:
With Fine Coarse Spinner are you always adding and removing things?
Just wondering if this makes more sense?

Spinner Controls (Fine Coarse Spinner Idea)

• Add bursts [Left][Right] or [Up][Down]
• Add one [Shift] + [Left][Right] || [Shift] + [Up][Down] || [Shift] + [Left][Right] or [Shift] + [Up][Down]]
• Remove All [HOME]
• Add Max Amount [END]

NOTE: Removed "in" before "bursts".

[terracoda]

Assigning to @arouinfar for comment. Am I capturing everything? Tagging @jessegreenberg and @pixelzoom.

[terracoda]

We'll need accessible text for the KBH dialog as well. I just add "with" in front of the keys, and add "arrow keys" or "key" at the end in most or all cases.

{{Spinner || Slider || Spinner and Slider}} Controls (regular Number Spinner and Slider)

• Adjust {{spinner || slider || spinner or slider}} with Left and Right arrow keys or Up and Down arrow keys.
• Adjust in smaller steps with Shift plus Left and Right arrow keys or Shift plus Up and Down arrow keys.
• Adjust in larger steps with Page Up and Page Down keys.
• Jump to minimum with Home key.
• Jump to maximum with End key.

Spinner Controls (Fine Coarse Spinner)

• Adjust spinner with Left and Right arrow keys or Up and Down arrow keys.
• Adjust in smaller steps with Shift plus Left and Right arrow keys. || Shift plus Up and Down arrow keys. || Shift plus Left and Right arrow keys or Shift plus Up and Down arrow keys.
• Jump to minimum with Home key.
• Jump to maximum with End key.

Spinner Controls (Fine Coarse Spinner Idea)

• Add bursts with Left and Right arrow keys or Up and Down arrow keys.
• Add one with Shift plus Left and Right arrow keys || Shift plus Up and Down arrow keys || Shift plus Left and Right arrow keys or Shift plus Up and Down arrow keys.
• Remove all with Home key.
• Add max amount with End key.

[jessegreenberg]

The above commits add the help content for "spinner" controls with SpinnerControlsKeyboardHelpSection. @terracoda and I met to review the look and PDOM content in the scenery-phet demo.

@pixelzoom can you please review? f66256f can be ignored, it is an abandoned branch.

A while ago we talked about a new superclass for this. But I thought SliderControlsKeyboardHelpSection was still the appropriate class to extend for this. Let me know what you think, happy to make changes.

[pixelzoom]

I'm not a fan of SliderControlsKeyboardHelpSection extends SliderControlsKeyboardHelpSection because a spinner does not extend a slider. Probably OK for now, but I predict this will need to be changed in the future.

That said... SliderControlsKeyboardHelpSection looks good integrated into FEL, see screenshot below. But a different problem (and probably worth a different issue) is the amount of duplication that we have between keyboard help for various UI components, as demonstrated by "Slider" and "Spinner" in this screenshot. Rather than having keyboard help for each type of UI component, it would be more usable and scalable to expand the "Basic Actions".

[jessegreenberg]

Yes, that is a good idea. I made a new issue for that.

because a spinner does not extend a slider. Probably OK for now, but I predict this will need to be changed in the future.

OK, understood. I am going to close this for now but will keep that in mind. Thanks!