Add Voicing to ComboBox

[zepumph]

While working on phetsims/friction#268, @jessegreenberg and I realized that the best solution would be to add Voicing as a feature to ComboBox, instead of working around that with usages-specific voicing.

[jessegreenberg]

@zepumph and I worked on this together and added support in the above commits. Closing.

[zepumph]

I am working on some improvements to ComboBox Voicing because of ratio and proporiton over in phetsims/ratio-and-proportion#428, and after some review I have the following things that are needed (from the RAP design doc):

• Don't repeat the name response when selecting the current selection in ComboBoxListItemNode
• Ability to provide object/context/hint responses as appropriate for a couple cases.
  There may be more as we go.

[zepumph]

This is getting closer to a complete solution that takes into account context/hint responses. I opted out of the default focus behavior in voicing for both the button and list nodes. The only feature we have lost from it (that I know of) is the name response on keyboard refocus.

I still have to implement the context response, which is going to be quite a bit harder since we need it to be strings right now, and not creator functions that we can pass to the button/list item node. We may want to bring back creator functions for this case.

Index: js/ComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/ComboBox.js b/js/ComboBox.js
--- a/js/ComboBox.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/js/ComboBox.js	(date 1644453401130)
@@ -111,6 +111,11 @@
       accessibleNameBehavior: ACCESSIBLE_NAME_BEHAVIOR,
       helpTextBehavior: HELP_TEXT_BEHAVIOR,
 
+      // ComboBox does not mix Voicing, these will be passed to the ComboBoxButton.
+      voicingObjectResponse: null,
+      voicingContextResponse: null,
+      voicingHintResponse: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       phetioType: ComboBox.ComboBoxIO,
@@ -126,6 +131,7 @@
       `invalid listPosition: ${options.listPosition}` );
     assert && assert( _.includes( ALIGN_VALUES, options.align ),
       `invalid align: ${options.align}` );
+    assert && assert( !options.voicingNameResponse, 'ComboBox sets its own nameResponse' );
 
     super();
 
@@ -152,6 +158,9 @@
       mouseAreaXDilation: options.buttonMouseAreaXDilation,
       mouseAreaYDilation: options.buttonMouseAreaYDilation,
 
+      // voicing
+      voicingHintResponse: options.voicingHintResponse,
+
       // pdom - accessibleName and helpText are set via behavior functions on the ComboBox
 
       // phet-io
@@ -217,7 +226,15 @@
     // Clicking on the button toggles visibility of the list box
     this.button.addListener( () => {
       this.listBox.visibleProperty.value = !this.listBox.visibleProperty.value;
-      this.listBox.visibleProperty.value && this.listBox.focus();
+      if ( this.listBox.visibleProperty.value ) {
+
+        this.listBox.focus();
+
+        this.button.voicingSpeakFullResponse( {
+          objectResponse: null,
+          contextResponse: null
+        } );
+      }
     } );
 
     // @private the display that clickToDismissListener is added to, because the scene may change, see sun#14
Index: js/ComboBoxListBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/ComboBoxListBox.js b/js/ComboBoxListBox.js
--- a/js/ComboBoxListBox.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/js/ComboBoxListBox.js	(date 1644453307274)
@@ -65,6 +65,8 @@
     // Pops down the list box and sets the property.value to match the chosen item.
     const fireAction = new Action( event => {
 
+      const oldValue = property.value;
+
       const listItemNode = event.currentTarget;
       assert && assert( listItemNode instanceof ComboBoxListItemNode, 'expected a ComboBoxListItemNode' );
 
@@ -77,6 +79,13 @@
       // hide the list
       hideListBoxCallback();
 
+      if ( oldValue !== property.value ) {
+        listItemNode.voicingSpeakFullResponse( {
+          objectResponse: null,
+          hintResponse: null
+        } );
+      }
+
       // prevent nodes (eg, controls) behind the list from receiving the event
       event.abort();
     }, {
Index: js/ComboBoxListItemNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/ComboBoxListItemNode.js b/js/ComboBoxListItemNode.js
--- a/js/ComboBoxListItemNode.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/js/ComboBoxListItemNode.js	(date 1644452946710)
@@ -98,6 +98,9 @@
 
     super( options );
 
+    // Handle Voicing on focus in a more custom way
+    this.setVoicingFocusListener( () => {} );
+
     // @public (read-only)
     this.item = item;
 
Index: js/ComboBoxButton.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/ComboBoxButton.js b/js/ComboBoxButton.js
--- a/js/ComboBoxButton.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/js/ComboBoxButton.js	(date 1644453376277)
@@ -143,6 +143,9 @@
 
     super( options );
 
+    // Handle Voicing on focus in a more custom way
+    this.setVoicingFocusListener( () => {} );
+
     const updateItemLayout = () => {
       if ( options.align === 'left' ) {
         itemNodeWrapper.left = itemAreaStrut.left + itemXMargin;

[zepumph]

Here is a good checkpoint. I added context responses via a new Voicing feature called voicingCreateContextResponse.

There are a fair number of TODOs to go over with @jessegreenberg, we aren't ready to commit yet.

Index: sun/js/ComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBox.js b/sun/js/ComboBox.js
--- a/sun/js/ComboBox.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBox.js	(date 1644511517112)
@@ -111,6 +111,11 @@
       accessibleNameBehavior: ACCESSIBLE_NAME_BEHAVIOR,
       helpTextBehavior: HELP_TEXT_BEHAVIOR,
 
+      // ComboBox does not mix Voicing, these will be passed to the ComboBoxButton.
+      // TODO: support creator for hintResponse? https://github.com/phetsims/sun/issues/726
+      voicingCreateContextResponse: null,
+      voicingHintResponse: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       phetioType: ComboBox.ComboBoxIO,
@@ -126,6 +131,7 @@
       `invalid listPosition: ${options.listPosition}` );
     assert && assert( _.includes( ALIGN_VALUES, options.align ),
       `invalid align: ${options.align}` );
+    assert && assert( !options.voicingNameResponse, 'ComboBox sets its own nameResponse' );
 
     super();
 
@@ -152,6 +158,9 @@
       mouseAreaXDilation: options.buttonMouseAreaXDilation,
       mouseAreaYDilation: options.buttonMouseAreaYDilation,
 
+      // voicing
+      voicingHintResponse: options.voicingHintResponse,
+
       // pdom - accessibleName and helpText are set via behavior functions on the ComboBox
 
       // phet-io
@@ -187,6 +196,10 @@
         lineWidth: options.listLineWidth,
         visible: false,
 
+        comboBoxListItemNodeOptions: {
+          voicingCreateContextResponse: options.voicingCreateContextResponse
+        },
+
         // sound generation
         openedSoundPlayer: options.openedSoundPlayer,
         closedNoChangeSoundPlayer: options.closedNoChangeSoundPlayer,
@@ -217,7 +230,16 @@
     // Clicking on the button toggles visibility of the list box
     this.button.addListener( () => {
       this.listBox.visibleProperty.value = !this.listBox.visibleProperty.value;
-      this.listBox.visibleProperty.value && this.listBox.focus();
+      if ( this.listBox.visibleProperty.value ) {
+
+        this.listBox.focus();
+
+        // When the button opens the listbox, voice a response
+        this.button.voicingSpeakFullResponse( {
+          objectResponse: null,
+          contextResponse: null
+        } );
+      }
     } );
 
     // @private the display that clickToDismissListener is added to, because the scene may change, see sun#14
Index: sun/js/ComboBoxListBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListBox.js b/sun/js/ComboBoxListBox.js
--- a/sun/js/ComboBoxListBox.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBoxListBox.js	(date 1644511517118)
@@ -42,6 +42,9 @@
       yMargin: 8,
       backgroundPickable: true,
 
+      // Options that apply to every item Node created in the list
+      comboBoxListItemNodeOptions: {},
+
       // pdom
       tagName: 'ul',
       ariaRole: 'listbox',
@@ -65,6 +68,8 @@
     // Pops down the list box and sets the property.value to match the chosen item.
     const fireAction = new Action( event => {
 
+      const oldValue = property.value;
+
       const listItemNode = event.currentTarget;
       assert && assert( listItemNode instanceof ComboBoxListItemNode, 'expected a ComboBoxListItemNode' );
 
@@ -77,6 +82,17 @@
       // hide the list
       hideListBoxCallback();
 
+      const responseOptions = {
+        objectResponse: null,
+        hintResponse: null
+      };
+
+      // If there is no change in value, then there is no context response
+      if ( oldValue === property.value ) {
+        responseOptions.contextResponse = null;
+      }
+      listItemNode.voicingSpeakFullResponse( responseOptions );
+
       // prevent nodes (eg, controls) behind the list from receiving the event
       event.abort();
     }, {
@@ -122,7 +138,7 @@
     items.forEach( ( item, index ) => {
 
       // Create the list item node
-      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, {
+      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, merge( {
         align: options.align,
         highlightFill: options.highlightFill,
         highlightCornerRadius: options.cornerRadius,
@@ -132,7 +148,7 @@
         left: 0.5 * options.xMargin,
         top: ( 0.5 * options.yMargin ) + ( index * highlightHeight ),
         tandem: item.tandemName ? tandem.createTandem( item.tandemName ) : Tandem.OPTIONAL
-      } );
+      }, options.comboBoxListItemNodeOptions ) );
       listItemNodes.push( listItemNode );
 
       listItemNode.addInputListener( selectionListener );
Index: sun/js/ComboBoxListItemNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListItemNode.js b/sun/js/ComboBoxListItemNode.js
--- a/sun/js/ComboBoxListItemNode.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBoxListItemNode.js	(date 1644511517106)
@@ -98,6 +98,10 @@
 
     super( options );
 
+    // Handle Voicing on focus in a more custom way
+    // TODO: is there a way to handle this via a mutator key options? https://github.com/phetsims/sun/issues/726
+    this.setVoicingFocusListener( () => {} );
+
     // @public (read-only)
     this.item = item;
 
Index: scenery/js/accessibility/voicing/Voicing.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/accessibility/voicing/Voicing.ts b/scenery/js/accessibility/voicing/Voicing.ts
--- a/scenery/js/accessibility/voicing/Voicing.ts	(revision 5b05bc3095501bac4982a7b907c3333c2ec867ca)
+++ b/scenery/js/accessibility/voicing/Voicing.ts	(date 1644510709688)
@@ -40,6 +40,7 @@
   'voicingNameResponse',
   'voicingObjectResponse',
   'voicingContextResponse',
+  'voicingCreateContextResponse',
   'voicingHintResponse',
   'voicingUtteranceQueue',
   'voicingResponsePatternCollection',
@@ -47,6 +48,8 @@
   'voicingFocusListener'
 ];
 
+type ResponseCreator = () => string | null;
+
 type VoicingSelfOptions = {
   voicingNameResponse?: string | null,
   voicingObjectResponse?: string | null,
@@ -79,6 +82,10 @@
     // ResponsePacket that holds all the supported responses to be Voiced
     _voicingResponsePacket!: ResponsePacket;
 
+    // Instead of setting responses as strings, you can alternately pass in a function that will be called to populate
+    // the context response for the response being spoken.
+    _voicingCreateContextResponse!: ResponseCreator | null;
+
     // The utteranceQueue that responses for this Node will be spoken through.
     // By default (null), it will go through the singleton voicingUtteranceQueue, but you may need separate
     // UtteranceQueues for different areas of content in your application. For example, Voicing and
@@ -118,6 +125,7 @@
       super.initialize && super.initialize();
 
       this._voicingResponsePacket = new ResponsePacket();
+      this._voicingCreateContextResponse = null;
       this._voicingUtteranceQueue = null;
       this._voicingFocusListener = this.defaultFocusListener;
 
@@ -142,7 +150,7 @@
       const options = optionize<ResponseOptions, {}, ResponseOptions>( {
         nameResponse: this._voicingResponsePacket.nameResponse,
         objectResponse: this._voicingResponsePacket.objectResponse,
-        contextResponse: this._voicingResponsePacket.contextResponse,
+        contextResponse: this._getContextResponseText(),
         hintResponse: this._voicingResponsePacket.hintResponse
       }, providedOptions );
 
@@ -208,7 +216,7 @@
 
       // options are passed along to collectAndSpeakResponse, see that function for additional options
       const options = optionize<ResponseOptions, {}, ResponseOptions>( {
-        contextResponse: this._voicingResponsePacket.contextResponse
+        contextResponse: this._getContextResponseText()
       }, providedOptions );
 
       this.collectAndSpeakResponse( options );
@@ -264,6 +272,10 @@
       }
     }
 
+    _getContextResponseText(): string | null {
+      return this._voicingCreateContextResponse ? this._voicingCreateContextResponse() : this._voicingResponsePacket.contextResponse;
+    }
+
     /**
      * Sets the voicingNameResponse for this Node. This is usually the label of the element and is spoken
      * when the object receives input. When requesting speech, this will only be spoken if
@@ -324,6 +336,25 @@
 
     get voicingContextResponse(): string | null { return this.getVoicingContextResponse(); }
 
+    /**
+     * Set a function used to create the context response for this Node. Note that if using this setter, it will override
+     * anything set to voicingContextResponse.
+     */
+    setVoicingCreateContextResponse( responseCreator: ResponseCreator | null ) {
+      this._voicingCreateContextResponse = responseCreator;
+    }
+
+    set voicingCreateContextResponse( responseCreator: ResponseCreator | null ) { this.setVoicingCreateContextResponse( responseCreator ); }
+
+    /**
+     * Gets the context-response creator function for this Node.
+     */
+    getVoicingCreateContextResponse(): ResponseCreator | null {
+      return this._voicingCreateContextResponse;
+    }
+
+    get voicingCreateContextResponse(): ResponseCreator | null { return this.getVoicingCreateContextResponse(); }
+
     /**
      * Sets the hint response for this Node. This is usually a response that describes how to interact with this Node.
      * When requesting speech, this will only be spoken when responseCollector.hintResponsesEnabledProperty is set to
Index: ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
--- a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(revision 5b9c8544bf3b2337980a029f2be049fb66479e23)
+++ b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(date 1644511114396)
@@ -82,6 +82,7 @@
     ], targetRatioProperty, comboBoxListParent, {
       helpText: ratioAndProportionStrings.a11y.discover.challengesHelpText,
       voicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      voicingCreateContextResponse: () => ratioDescriber.getProximityToNewChallengeRatioSentence(),
       maxWidth: 300, // empirically determined
 
       // phet-io
Index: sun/js/ComboBoxButton.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxButton.js b/sun/js/ComboBoxButton.js
--- a/sun/js/ComboBoxButton.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBoxButton.js	(date 1644508500908)
@@ -143,6 +143,9 @@
 
     super( options );
 
+    // Handle Voicing on focus in a more custom way
+    this.setVoicingFocusListener( () => {} );
+
     const updateItemLayout = () => {
       if ( options.align === 'left' ) {
         itemNodeWrapper.left = itemAreaStrut.left + itemXMargin;

Next, I'll add the name response back when changing the focus with keyboard in the listbox.

[zepumph]

I was able to get the listbox selection focus voicing to work by adding a custom focus listener to the list item nodes, with a switch to disable it upon first focus (opening the listbox), because that has a different voicing response.

At this point, I would consider ComboBox feature-complete as needed for Ratio and Proporiton.

Next steps are to go over this with @jessegreenberg, likely together, and try to improve it, simplify it, and discuss the new API need in the Voicing trait.

Index: sun/js/ComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBox.js b/sun/js/ComboBox.js
--- a/sun/js/ComboBox.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBox.js	(date 1644511517112)
@@ -111,6 +111,11 @@
       accessibleNameBehavior: ACCESSIBLE_NAME_BEHAVIOR,
       helpTextBehavior: HELP_TEXT_BEHAVIOR,
 
+      // ComboBox does not mix Voicing, these will be passed to the ComboBoxButton.
+      // TODO: support creator for hintResponse? https://github.com/phetsims/sun/issues/726
+      voicingCreateContextResponse: null,
+      voicingHintResponse: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       phetioType: ComboBox.ComboBoxIO,
@@ -126,6 +131,7 @@
       `invalid listPosition: ${options.listPosition}` );
     assert && assert( _.includes( ALIGN_VALUES, options.align ),
       `invalid align: ${options.align}` );
+    assert && assert( !options.voicingNameResponse, 'ComboBox sets its own nameResponse' );
 
     super();
 
@@ -152,6 +158,9 @@
       mouseAreaXDilation: options.buttonMouseAreaXDilation,
       mouseAreaYDilation: options.buttonMouseAreaYDilation,
 
+      // voicing
+      voicingHintResponse: options.voicingHintResponse,
+
       // pdom - accessibleName and helpText are set via behavior functions on the ComboBox
 
       // phet-io
@@ -187,6 +196,10 @@
         lineWidth: options.listLineWidth,
         visible: false,
 
+        comboBoxListItemNodeOptions: {
+          voicingCreateContextResponse: options.voicingCreateContextResponse
+        },
+
         // sound generation
         openedSoundPlayer: options.openedSoundPlayer,
         closedNoChangeSoundPlayer: options.closedNoChangeSoundPlayer,
@@ -217,7 +230,16 @@
     // Clicking on the button toggles visibility of the list box
     this.button.addListener( () => {
       this.listBox.visibleProperty.value = !this.listBox.visibleProperty.value;
-      this.listBox.visibleProperty.value && this.listBox.focus();
+      if ( this.listBox.visibleProperty.value ) {
+
+        this.listBox.focus();
+
+        // When the button opens the listbox, voice a response
+        this.button.voicingSpeakFullResponse( {
+          objectResponse: null,
+          contextResponse: null
+        } );
+      }
     } );
 
     // @private the display that clickToDismissListener is added to, because the scene may change, see sun#14
Index: sun/js/ComboBoxListBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListBox.js b/sun/js/ComboBoxListBox.js
--- a/sun/js/ComboBoxListBox.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBoxListBox.js	(date 1644511851646)
@@ -42,6 +42,9 @@
       yMargin: 8,
       backgroundPickable: true,
 
+      // Options that apply to every item Node created in the list
+      comboBoxListItemNodeOptions: {},
+
       // pdom
       tagName: 'ul',
       ariaRole: 'listbox',
@@ -65,6 +68,8 @@
     // Pops down the list box and sets the property.value to match the chosen item.
     const fireAction = new Action( event => {
 
+      const oldValue = property.value;
+
       const listItemNode = event.currentTarget;
       assert && assert( listItemNode instanceof ComboBoxListItemNode, 'expected a ComboBoxListItemNode' );
 
@@ -77,6 +82,17 @@
       // hide the list
       hideListBoxCallback();
 
+      const responseOptions = {
+        objectResponse: null,
+        hintResponse: null
+      };
+
+      // If there is no change in value, then there is no context response
+      if ( oldValue === property.value ) {
+        responseOptions.contextResponse = null;
+      }
+      listItemNode.voicingSpeakFullResponse( responseOptions );
+
       // prevent nodes (eg, controls) behind the list from receiving the event
       event.abort();
     }, {
@@ -122,7 +138,7 @@
     items.forEach( ( item, index ) => {
 
       // Create the list item node
-      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, {
+      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, merge( {
         align: options.align,
         highlightFill: options.highlightFill,
         highlightCornerRadius: options.cornerRadius,
@@ -132,7 +148,7 @@
         left: 0.5 * options.xMargin,
         top: ( 0.5 * options.yMargin ) + ( index * highlightHeight ),
         tandem: item.tandemName ? tandem.createTandem( item.tandemName ) : Tandem.OPTIONAL
-      } );
+      }, options.comboBoxListItemNodeOptions ) );
       listItemNodes.push( listItemNode );
 
       listItemNode.addInputListener( selectionListener );
@@ -189,9 +205,11 @@
     this.addInputListener( {
 
       // When the list box gets focus, transfer focus to the ComboBoxListItemNode that matches property.value.
-      focus: event => {
+      focus: () => {
         if ( this.visible ) {
-          this.getListItemNode( property.value ).focus();
+          const listItemNode = this.getListItemNode( property.value );
+          listItemNode.preventNextVoicingOnFocus();
+          listItemNode.focus();
         }
       },
 
Index: sun/js/ComboBoxListItemNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListItemNode.js b/sun/js/ComboBoxListItemNode.js
--- a/sun/js/ComboBoxListItemNode.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBoxListItemNode.js	(date 1644511989768)
@@ -98,6 +98,17 @@
 
     super( options );
 
+    this.blockNextFocusResponse = false;
+
+    // Handle Voicing on focus in a more custom way
+    // TODO: is there a way to handle this via a mutator key options? https://github.com/phetsims/sun/issues/726
+    this.setVoicingFocusListener( () => {
+      if ( !this.blockNextFocusResponse ) {
+        this.voicingSpeakNameResponse();
+      }
+      this.blockNextFocusResponse = false;
+    } );
+
     // @public (read-only)
     this.item = item;
 
@@ -110,6 +121,14 @@
       enter() { highlightRectangle.fill = options.highlightFill; },
       exit() { highlightRectangle.fill = null; }
     } );
+
+  }
+
+  /**
+   * @public
+   */
+  preventNextVoicingOnFocus() {
+    this.blockNextFocusResponse = true;
   }
 }
 
Index: scenery/js/accessibility/voicing/Voicing.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/accessibility/voicing/Voicing.ts b/scenery/js/accessibility/voicing/Voicing.ts
--- a/scenery/js/accessibility/voicing/Voicing.ts	(revision 5b05bc3095501bac4982a7b907c3333c2ec867ca)
+++ b/scenery/js/accessibility/voicing/Voicing.ts	(date 1644510709688)
@@ -40,6 +40,7 @@
   'voicingNameResponse',
   'voicingObjectResponse',
   'voicingContextResponse',
+  'voicingCreateContextResponse',
   'voicingHintResponse',
   'voicingUtteranceQueue',
   'voicingResponsePatternCollection',
@@ -47,6 +48,8 @@
   'voicingFocusListener'
 ];
 
+type ResponseCreator = () => string | null;
+
 type VoicingSelfOptions = {
   voicingNameResponse?: string | null,
   voicingObjectResponse?: string | null,
@@ -79,6 +82,10 @@
     // ResponsePacket that holds all the supported responses to be Voiced
     _voicingResponsePacket!: ResponsePacket;
 
+    // Instead of setting responses as strings, you can alternately pass in a function that will be called to populate
+    // the context response for the response being spoken.
+    _voicingCreateContextResponse!: ResponseCreator | null;
+
     // The utteranceQueue that responses for this Node will be spoken through.
     // By default (null), it will go through the singleton voicingUtteranceQueue, but you may need separate
     // UtteranceQueues for different areas of content in your application. For example, Voicing and
@@ -118,6 +125,7 @@
       super.initialize && super.initialize();
 
       this._voicingResponsePacket = new ResponsePacket();
+      this._voicingCreateContextResponse = null;
       this._voicingUtteranceQueue = null;
       this._voicingFocusListener = this.defaultFocusListener;
 
@@ -142,7 +150,7 @@
       const options = optionize<ResponseOptions, {}, ResponseOptions>( {
         nameResponse: this._voicingResponsePacket.nameResponse,
         objectResponse: this._voicingResponsePacket.objectResponse,
-        contextResponse: this._voicingResponsePacket.contextResponse,
+        contextResponse: this._getContextResponseText(),
         hintResponse: this._voicingResponsePacket.hintResponse
       }, providedOptions );
 
@@ -208,7 +216,7 @@
 
       // options are passed along to collectAndSpeakResponse, see that function for additional options
       const options = optionize<ResponseOptions, {}, ResponseOptions>( {
-        contextResponse: this._voicingResponsePacket.contextResponse
+        contextResponse: this._getContextResponseText()
       }, providedOptions );
 
       this.collectAndSpeakResponse( options );
@@ -264,6 +272,10 @@
       }
     }
 
+    _getContextResponseText(): string | null {
+      return this._voicingCreateContextResponse ? this._voicingCreateContextResponse() : this._voicingResponsePacket.contextResponse;
+    }
+
     /**
      * Sets the voicingNameResponse for this Node. This is usually the label of the element and is spoken
      * when the object receives input. When requesting speech, this will only be spoken if
@@ -324,6 +336,25 @@
 
     get voicingContextResponse(): string | null { return this.getVoicingContextResponse(); }
 
+    /**
+     * Set a function used to create the context response for this Node. Note that if using this setter, it will override
+     * anything set to voicingContextResponse.
+     */
+    setVoicingCreateContextResponse( responseCreator: ResponseCreator | null ) {
+      this._voicingCreateContextResponse = responseCreator;
+    }
+
+    set voicingCreateContextResponse( responseCreator: ResponseCreator | null ) { this.setVoicingCreateContextResponse( responseCreator ); }
+
+    /**
+     * Gets the context-response creator function for this Node.
+     */
+    getVoicingCreateContextResponse(): ResponseCreator | null {
+      return this._voicingCreateContextResponse;
+    }
+
+    get voicingCreateContextResponse(): ResponseCreator | null { return this.getVoicingCreateContextResponse(); }
+
     /**
      * Sets the hint response for this Node. This is usually a response that describes how to interact with this Node.
      * When requesting speech, this will only be spoken when responseCollector.hintResponsesEnabledProperty is set to
Index: ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
--- a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(revision 5b9c8544bf3b2337980a029f2be049fb66479e23)
+++ b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(date 1644511114396)
@@ -82,6 +82,7 @@
     ], targetRatioProperty, comboBoxListParent, {
       helpText: ratioAndProportionStrings.a11y.discover.challengesHelpText,
       voicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      voicingCreateContextResponse: () => ratioDescriber.getProximityToNewChallengeRatioSentence(),
       maxWidth: 300, // empirically determined
 
       // phet-io
Index: sun/js/ComboBoxButton.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxButton.js b/sun/js/ComboBoxButton.js
--- a/sun/js/ComboBoxButton.js	(revision 19eb62fb1debfa6bcbe119855b7a1038000fb2a6)
+++ b/sun/js/ComboBoxButton.js	(date 1644508500908)
@@ -143,6 +143,9 @@
 
     super( options );
 
+    // Handle Voicing on focus in a more custom way
+    this.setVoicingFocusListener( () => {} );
+
     const updateItemLayout = () => {
       if ( options.align === 'left' ) {
         itemNodeWrapper.left = itemAreaStrut.left + itemXMargin;

[zepumph]

• Nothing on tab navigation focus of the button. We have to be careful here since we re-focus the button on new selection from the list box.

[jessegreenberg]

We just discussed the patch and its items and produced the following patch, which has some improvements and a few renames. Otherwise, we like the solution so far.

Index: ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
--- a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(revision 5b9c8544bf3b2337980a029f2be049fb66479e23)
+++ b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(date 1644528392384)
@@ -82,6 +82,7 @@
     ], targetRatioProperty, comboBoxListParent, {
       helpText: ratioAndProportionStrings.a11y.discover.challengesHelpText,
       voicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      voicingCreateContextResponse: () => ratioDescriber.getProximityToNewChallengeRatioSentence(),
       maxWidth: 300, // empirically determined
 
       // phet-io
Index: sun/js/ComboBoxButton.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxButton.js b/sun/js/ComboBoxButton.js
--- a/sun/js/ComboBoxButton.js	(revision 46d890e399813e1cccd81b3295d425d4fac97837)
+++ b/sun/js/ComboBoxButton.js	(date 1644528392437)
@@ -143,6 +143,9 @@
 
     super( options );
 
+    // Handle Voicing on focus in a more custom way
+    this.setVoicingFocusListener( () => {} );
+
     const updateItemLayout = () => {
       if ( options.align === 'left' ) {
         itemNodeWrapper.left = itemAreaStrut.left + itemXMargin;
Index: sun/js/ComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBox.js b/sun/js/ComboBox.js
--- a/sun/js/ComboBox.js	(revision 46d890e399813e1cccd81b3295d425d4fac97837)
+++ b/sun/js/ComboBox.js	(date 1644531249569)
@@ -111,6 +111,11 @@
       accessibleNameBehavior: ACCESSIBLE_NAME_BEHAVIOR,
       helpTextBehavior: HELP_TEXT_BEHAVIOR,
 
+      // TODO: Rename these names everywhere
+      // ComboBox does not mix Voicing, these will be passed to the ComboBoxButton.
+      comboBoxVoicingCreateContextResponse: null,
+      comboBoxVoicingHintResponse: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       phetioType: ComboBox.ComboBoxIO,
@@ -126,6 +131,7 @@
       `invalid listPosition: ${options.listPosition}` );
     assert && assert( _.includes( ALIGN_VALUES, options.align ),
       `invalid align: ${options.align}` );
+    assert && assert( !options.voicingNameResponse, 'ComboBox sets its own nameResponse' );
 
     super();
 
@@ -152,6 +158,9 @@
       mouseAreaXDilation: options.buttonMouseAreaXDilation,
       mouseAreaYDilation: options.buttonMouseAreaYDilation,
 
+      // voicing
+      voicingHintResponse: options.voicingHintResponse,
+
       // pdom - accessibleName and helpText are set via behavior functions on the ComboBox
 
       // phet-io
@@ -187,6 +196,10 @@
         lineWidth: options.listLineWidth,
         visible: false,
 
+        comboBoxListItemNodeOptions: {
+          voicingCreateContextResponse: options.voicingCreateContextResponse
+        },
+
         // sound generation
         openedSoundPlayer: options.openedSoundPlayer,
         closedNoChangeSoundPlayer: options.closedNoChangeSoundPlayer,
Index: sun/js/ComboBoxListBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListBox.js b/sun/js/ComboBoxListBox.js
--- a/sun/js/ComboBoxListBox.js	(revision 46d890e399813e1cccd81b3295d425d4fac97837)
+++ b/sun/js/ComboBoxListBox.js	(date 1644529238950)
@@ -42,6 +42,9 @@
       yMargin: 8,
       backgroundPickable: true,
 
+      // Options that apply to every item Node created in the list
+      comboBoxListItemNodeOptions: {},
+
       // pdom
       tagName: 'ul',
       ariaRole: 'listbox',
@@ -65,6 +68,8 @@
     // Pops down the list box and sets the property.value to match the chosen item.
     const fireAction = new Action( event => {
 
+      const oldValue = property.value;
+
       const listItemNode = event.currentTarget;
       assert && assert( listItemNode instanceof ComboBoxListItemNode, 'expected a ComboBoxListItemNode' );
 
@@ -77,6 +82,17 @@
       // hide the list
       hideListBoxCallback();
 
+      const responseOptions = {
+        objectResponse: null,
+        hintResponse: null
+      };
+
+      // If there is no change in value, then there is no context response
+      if ( oldValue === property.value ) {
+        responseOptions.contextResponse = null;
+      }
+      listItemNode.voicingSpeakFullResponse( responseOptions );
+
       // prevent nodes (eg, controls) behind the list from receiving the event
       event.abort();
     }, {
@@ -122,7 +138,7 @@
     items.forEach( ( item, index ) => {
 
       // Create the list item node
-      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, {
+      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, merge( {
         align: options.align,
         highlightFill: options.highlightFill,
         highlightCornerRadius: options.cornerRadius,
@@ -132,7 +148,7 @@
         left: 0.5 * options.xMargin,
         top: ( 0.5 * options.yMargin ) + ( index * highlightHeight ),
         tandem: item.tandemName ? tandem.createTandem( item.tandemName ) : Tandem.OPTIONAL
-      } );
+      }, options.comboBoxListItemNodeOptions ) );
       listItemNodes.push( listItemNode );
 
       listItemNode.addInputListener( selectionListener );
@@ -189,9 +205,11 @@
     this.addInputListener( {
 
       // When the list box gets focus, transfer focus to the ComboBoxListItemNode that matches property.value.
-      focus: event => {
+      focus: () => {
         if ( this.visible ) {
-          this.getListItemNode( property.value ).focus();
+          const listItemNode = this.getListItemNode( property.value );
+          listItemNode.supplyHintResponseOnNextFocus();
+          listItemNode.focus();
         }
       },
 
Index: sun/js/ComboBoxListItemNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListItemNode.js b/sun/js/ComboBoxListItemNode.js
--- a/sun/js/ComboBoxListItemNode.js	(revision 46d890e399813e1cccd81b3295d425d4fac97837)
+++ b/sun/js/ComboBoxListItemNode.js	(date 1644529393100)
@@ -10,10 +10,7 @@
 
 import Shape from '../../kite/js/Shape.js';
 import merge from '../../phet-core/js/merge.js';
-import { Voicing } from '../../scenery/js/imports.js';
-import { IndexedNodeIO } from '../../scenery/js/imports.js';
-import { Node } from '../../scenery/js/imports.js';
-import { Rectangle } from '../../scenery/js/imports.js';
+import { IndexedNodeIO, Node, Rectangle, Voicing } from '../../scenery/js/imports.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import ComboBoxItem from './ComboBoxItem.js';
 import sun from './sun.js';
@@ -98,6 +95,22 @@
 
     super( options );
 
+    // @private
+    this._supplyHintResponseOnNextFocus = false;
+
+    // Handle Voicing on focus in a more custom way
+    this.addInputListener( {
+      focus: () => {
+        this.voicingSpeakNameResponse( {
+          hintResponse: this._supplyHintResponseOnNextFocus ? this.voicingHintResponse : null
+        } );
+        this._supplyHintResponseOnNextFocus = false;
+      }
+    } );
+
+    // TODO: We should be able to set this to null, see https://github.com/phetsims/scenery/issues/1357
+    this.voicingFocusListener = () => {};
+
     // @public (read-only)
     this.item = item;
 
@@ -110,6 +123,14 @@
       enter() { highlightRectangle.fill = options.highlightFill; },
       exit() { highlightRectangle.fill = null; }
     } );
+
+  }
+
+  /**
+   * @public
+   */
+  supplyHintResponseOnNextFocus() {
+    this._supplyHintResponseOnNextFocus = true;
   }
 }
 
Index: scenery/js/accessibility/voicing/Voicing.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/accessibility/voicing/Voicing.ts b/scenery/js/accessibility/voicing/Voicing.ts
--- a/scenery/js/accessibility/voicing/Voicing.ts	(revision 89c6e2be7c9bb05cefbb1f46048910a7bf40962a)
+++ b/scenery/js/accessibility/voicing/Voicing.ts	(date 1644530314308)
@@ -41,6 +41,7 @@
   'voicingNameResponse',
   'voicingObjectResponse',
   'voicingContextResponse',
+  'voicingCreateContextResponse',
   'voicingHintResponse',
   'voicingUtteranceQueue',
   'voicingResponsePatternCollection',
@@ -48,6 +49,8 @@
   'voicingFocusListener'
 ];
 
+type ResponseCreator = () => string | null;
+
 type VoicingSelfOptions = {
   voicingNameResponse?: string | null,
   voicingObjectResponse?: string | null,
@@ -80,6 +83,10 @@
     // ResponsePacket that holds all the supported responses to be Voiced
     _voicingResponsePacket!: ResponsePacket;
 
+    // Instead of setting responses as strings, you can alternately pass in a function that will be called to populate
+    // the context response for the response being spoken.
+    _voicingCreateContextResponse!: ResponseCreator | null;
+
     // The utteranceQueue that responses for this Node will be spoken through.
     // By default (null), it will go through the singleton voicingUtteranceQueue, but you may need separate
     // UtteranceQueues for different areas of content in your application. For example, Voicing and
@@ -119,6 +126,7 @@
       super.initialize && super.initialize();
 
       this._voicingResponsePacket = new ResponsePacket();
+      this._voicingCreateContextResponse = null;
       this._voicingUtteranceQueue = null;
       this._voicingFocusListener = this.defaultFocusListener;
 
@@ -143,7 +151,7 @@
       const options = optionize<ResponseOptions, {}, ResponseOptions>( {
         nameResponse: this._voicingResponsePacket.nameResponse,
         objectResponse: this._voicingResponsePacket.objectResponse,
-        contextResponse: this._voicingResponsePacket.contextResponse,
+        contextResponse: this._getContextResponseToSpeak(),
         hintResponse: this._voicingResponsePacket.hintResponse
       }, providedOptions );
 
@@ -209,7 +217,7 @@
 
       // options are passed along to collectAndSpeakResponse, see that function for additional options
       const options = optionize<ResponseOptions, {}, ResponseOptions>( {
-        contextResponse: this._voicingResponsePacket.contextResponse
+        contextResponse: this._getContextResponseToSpeak()
       }, providedOptions );
 
       this.collectAndSpeakResponse( options );
@@ -265,6 +273,12 @@
       }
     }
 
+    _getContextResponseToSpeak(): string | null {
+
+      // TODO: Assert mutually exclusive here (should work because this would be after setting), https://github.com/phetsims/scenery/issues/1325
+      return this._voicingCreateContextResponse ? this._voicingCreateContextResponse() : this._voicingResponsePacket.contextResponse;
+    }
+
     /**
      * Sets the voicingNameResponse for this Node. This is usually the label of the element and is spoken
      * when the object receives input. When requesting speech, this will only be spoken if
@@ -325,6 +339,25 @@
 
     get voicingContextResponse(): string | null { return this.getVoicingContextResponse(); }
 
+    /**
+     * Set a function used to create the context response for this Node. Note that if using this setter, it will override
+     * anything set to voicingContextResponse.
+     */
+    setVoicingCreateContextResponse( responseCreator: ResponseCreator | null ) {
+      this._voicingCreateContextResponse = responseCreator;
+    }
+
+    set voicingCreateContextResponse( responseCreator: ResponseCreator | null ) { this.setVoicingCreateContextResponse( responseCreator ); }
+
+    /**
+     * Gets the context-response creator function for this Node.
+     */
+    getVoicingCreateContextResponse(): ResponseCreator | null {
+      return this._voicingCreateContextResponse;
+    }
+
+    get voicingCreateContextResponse(): ResponseCreator | null { return this.getVoicingCreateContextResponse(); }
+
     /**
      * Sets the hint response for this Node. This is usually a response that describes how to interact with this Node.
      * When requesting speech, this will only be spoken when responseCollector.hintResponsesEnabledProperty is set to

[zepumph]

And what a great patch it is!

Over in phetsims/scenery#1325 (comment) I proceeded far enough to commit the voicingCreateContextResponse addition to voicing, so if I can figure out the first focus of the button, I think I will be close to a commit here.

[zepumph]

As far as I can tell, this is working perfectly, but I'm confused because I really expected an extra name response when selecting from the list box, because it focuses back on the button, and the button is now using its default focus listener. I could not a single time get the duplication to occur though.

I want to sleep on it, and perhaps touch base with JG tomorrow about it.

Index: sun/js/ComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBox.js b/sun/js/ComboBox.js
--- a/sun/js/ComboBox.js	(revision ee0ef701860f0e316b9f9e62a2d6e7a1001df9bd)
+++ b/sun/js/ComboBox.js	(date 1644547267706)
@@ -111,6 +111,10 @@
       accessibleNameBehavior: ACCESSIBLE_NAME_BEHAVIOR,
       helpTextBehavior: HELP_TEXT_BEHAVIOR,
 
+      // ComboBox does not mix Voicing, these will be passed to the ComboBoxButton.
+      comboBoxVoicingCreateContextResponse: null,
+      comboBoxVoicingHintResponse: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       phetioType: ComboBox.ComboBoxIO,
@@ -187,6 +191,11 @@
         lineWidth: options.listLineWidth,
         visible: false,
 
+        comboBoxListItemNodeOptions: {
+          voicingCreateContextResponse: options.comboBoxVoicingCreateContextResponse,
+          voicingHintResponse: options.comboBoxVoicingHintResponse
+        },
+
         // sound generation
         openedSoundPlayer: options.openedSoundPlayer,
         closedNoChangeSoundPlayer: options.closedNoChangeSoundPlayer,
Index: sun/js/ComboBoxListBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListBox.js b/sun/js/ComboBoxListBox.js
--- a/sun/js/ComboBoxListBox.js	(revision ee0ef701860f0e316b9f9e62a2d6e7a1001df9bd)
+++ b/sun/js/ComboBoxListBox.js	(date 1644543823847)
@@ -42,6 +42,9 @@
       yMargin: 8,
       backgroundPickable: true,
 
+      // Options that apply to every item Node created in the list
+      comboBoxListItemNodeOptions: {},
+
       // pdom
       tagName: 'ul',
       ariaRole: 'listbox',
@@ -65,6 +68,8 @@
     // Pops down the list box and sets the property.value to match the chosen item.
     const fireAction = new Action( event => {
 
+      const oldValue = property.value;
+
       const listItemNode = event.currentTarget;
       assert && assert( listItemNode instanceof ComboBoxListItemNode, 'expected a ComboBoxListItemNode' );
 
@@ -77,6 +82,17 @@
       // hide the list
       hideListBoxCallback();
 
+      const responseOptions = {
+        objectResponse: null,
+        hintResponse: null
+      };
+
+      // If there is no change in value, then there is no context response
+      if ( oldValue === property.value ) {
+        responseOptions.contextResponse = null;
+      }
+      listItemNode.voicingSpeakFullResponse( responseOptions );
+
       // prevent nodes (eg, controls) behind the list from receiving the event
       event.abort();
     }, {
@@ -122,7 +138,7 @@
     items.forEach( ( item, index ) => {
 
       // Create the list item node
-      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, {
+      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, merge( {
         align: options.align,
         highlightFill: options.highlightFill,
         highlightCornerRadius: options.cornerRadius,
@@ -132,7 +148,7 @@
         left: 0.5 * options.xMargin,
         top: ( 0.5 * options.yMargin ) + ( index * highlightHeight ),
         tandem: item.tandemName ? tandem.createTandem( item.tandemName ) : Tandem.OPTIONAL
-      } );
+      }, options.comboBoxListItemNodeOptions ) );
       listItemNodes.push( listItemNode );
 
       listItemNode.addInputListener( selectionListener );
@@ -189,9 +205,11 @@
     this.addInputListener( {
 
       // When the list box gets focus, transfer focus to the ComboBoxListItemNode that matches property.value.
-      focus: event => {
+      focus: () => {
         if ( this.visible ) {
-          this.getListItemNode( property.value ).focus();
+          const listItemNode = this.getListItemNode( property.value );
+          listItemNode.supplyHintResponseOnNextFocus();
+          listItemNode.focus();
         }
       },
 
Index: sun/js/ComboBoxListItemNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListItemNode.js b/sun/js/ComboBoxListItemNode.js
--- a/sun/js/ComboBoxListItemNode.js	(revision ee0ef701860f0e316b9f9e62a2d6e7a1001df9bd)
+++ b/sun/js/ComboBoxListItemNode.js	(date 1644543823851)
@@ -10,10 +10,7 @@
 
 import Shape from '../../kite/js/Shape.js';
 import merge from '../../phet-core/js/merge.js';
-import { Voicing } from '../../scenery/js/imports.js';
-import { IndexedNodeIO } from '../../scenery/js/imports.js';
-import { Node } from '../../scenery/js/imports.js';
-import { Rectangle } from '../../scenery/js/imports.js';
+import { IndexedNodeIO, Node, Rectangle, Voicing } from '../../scenery/js/imports.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import ComboBoxItem from './ComboBoxItem.js';
 import sun from './sun.js';
@@ -98,6 +95,22 @@
 
     super( options );
 
+    // @private
+    this._supplyHintResponseOnNextFocus = false;
+
+    // Handle Voicing on focus in a more custom way
+    this.addInputListener( {
+      focus: () => {
+        this.voicingSpeakNameResponse( {
+          hintResponse: this._supplyHintResponseOnNextFocus ? this.voicingHintResponse : null
+        } );
+        this._supplyHintResponseOnNextFocus = false;
+      }
+    } );
+
+    // TODO: We should be able to set this to null, see https://github.com/phetsims/scenery/issues/1357
+    this.voicingFocusListener = () => {};
+
     // @public (read-only)
     this.item = item;
 
@@ -110,6 +123,14 @@
       enter() { highlightRectangle.fill = options.highlightFill; },
       exit() { highlightRectangle.fill = null; }
     } );
+
+  }
+
+  /**
+   * @public
+   */
+  supplyHintResponseOnNextFocus() {
+    this._supplyHintResponseOnNextFocus = true;
   }
 }
 
Index: ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
--- a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(revision 157ce138001d4c3b898884e2b8110c8ad0ced191)
+++ b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(date 1644544993582)
@@ -81,7 +81,8 @@
       } )
     ], targetRatioProperty, comboBoxListParent, {
       helpText: ratioAndProportionStrings.a11y.discover.challengesHelpText,
-      voicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      comboBoxVoicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      comboBoxVoicingCreateContextResponse: () => ratioDescriber.getProximityToNewChallengeRatioSentence(),
       maxWidth: 300, // empirically determined
 
       // phet-io

[zepumph]

And one more with updates from phetsims/scenery#1357

Index: sun/js/ComboBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBox.js b/sun/js/ComboBox.js
--- a/sun/js/ComboBox.js	(revision ee0ef701860f0e316b9f9e62a2d6e7a1001df9bd)
+++ b/sun/js/ComboBox.js	(date 1644547267706)
@@ -111,6 +111,10 @@
       accessibleNameBehavior: ACCESSIBLE_NAME_BEHAVIOR,
       helpTextBehavior: HELP_TEXT_BEHAVIOR,
 
+      // ComboBox does not mix Voicing, these will be passed to the ComboBoxButton.
+      comboBoxVoicingCreateContextResponse: null,
+      comboBoxVoicingHintResponse: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       phetioType: ComboBox.ComboBoxIO,
@@ -187,6 +191,11 @@
         lineWidth: options.listLineWidth,
         visible: false,
 
+        comboBoxListItemNodeOptions: {
+          voicingCreateContextResponse: options.comboBoxVoicingCreateContextResponse,
+          voicingHintResponse: options.comboBoxVoicingHintResponse
+        },
+
         // sound generation
         openedSoundPlayer: options.openedSoundPlayer,
         closedNoChangeSoundPlayer: options.closedNoChangeSoundPlayer,
Index: sun/js/ComboBoxListBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListBox.js b/sun/js/ComboBoxListBox.js
--- a/sun/js/ComboBoxListBox.js	(revision ee0ef701860f0e316b9f9e62a2d6e7a1001df9bd)
+++ b/sun/js/ComboBoxListBox.js	(date 1644543823847)
@@ -42,6 +42,9 @@
       yMargin: 8,
       backgroundPickable: true,
 
+      // Options that apply to every item Node created in the list
+      comboBoxListItemNodeOptions: {},
+
       // pdom
       tagName: 'ul',
       ariaRole: 'listbox',
@@ -65,6 +68,8 @@
     // Pops down the list box and sets the property.value to match the chosen item.
     const fireAction = new Action( event => {
 
+      const oldValue = property.value;
+
       const listItemNode = event.currentTarget;
       assert && assert( listItemNode instanceof ComboBoxListItemNode, 'expected a ComboBoxListItemNode' );
 
@@ -77,6 +82,17 @@
       // hide the list
       hideListBoxCallback();
 
+      const responseOptions = {
+        objectResponse: null,
+        hintResponse: null
+      };
+
+      // If there is no change in value, then there is no context response
+      if ( oldValue === property.value ) {
+        responseOptions.contextResponse = null;
+      }
+      listItemNode.voicingSpeakFullResponse( responseOptions );
+
       // prevent nodes (eg, controls) behind the list from receiving the event
       event.abort();
     }, {
@@ -122,7 +138,7 @@
     items.forEach( ( item, index ) => {
 
       // Create the list item node
-      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, {
+      const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, merge( {
         align: options.align,
         highlightFill: options.highlightFill,
         highlightCornerRadius: options.cornerRadius,
@@ -132,7 +148,7 @@
         left: 0.5 * options.xMargin,
         top: ( 0.5 * options.yMargin ) + ( index * highlightHeight ),
         tandem: item.tandemName ? tandem.createTandem( item.tandemName ) : Tandem.OPTIONAL
-      } );
+      }, options.comboBoxListItemNodeOptions ) );
       listItemNodes.push( listItemNode );
 
       listItemNode.addInputListener( selectionListener );
@@ -189,9 +205,11 @@
     this.addInputListener( {
 
       // When the list box gets focus, transfer focus to the ComboBoxListItemNode that matches property.value.
-      focus: event => {
+      focus: () => {
         if ( this.visible ) {
-          this.getListItemNode( property.value ).focus();
+          const listItemNode = this.getListItemNode( property.value );
+          listItemNode.supplyHintResponseOnNextFocus();
+          listItemNode.focus();
         }
       },
 
Index: sun/js/ComboBoxListItemNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/ComboBoxListItemNode.js b/sun/js/ComboBoxListItemNode.js
--- a/sun/js/ComboBoxListItemNode.js	(revision ee0ef701860f0e316b9f9e62a2d6e7a1001df9bd)
+++ b/sun/js/ComboBoxListItemNode.js	(date 1644547675656)
@@ -10,10 +10,7 @@
 
 import Shape from '../../kite/js/Shape.js';
 import merge from '../../phet-core/js/merge.js';
-import { Voicing } from '../../scenery/js/imports.js';
-import { IndexedNodeIO } from '../../scenery/js/imports.js';
-import { Node } from '../../scenery/js/imports.js';
-import { Rectangle } from '../../scenery/js/imports.js';
+import { IndexedNodeIO, Node, Rectangle, Voicing } from '../../scenery/js/imports.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import ComboBoxItem from './ComboBoxItem.js';
 import sun from './sun.js';
@@ -49,6 +46,9 @@
       // elements so they receive pointer events
       positionInPDOM: true,
 
+      // voicing
+      voicingFocusListener: null,
+
       // phet-io
       tandem: Tandem.REQUIRED,
 
@@ -98,6 +98,19 @@
 
     super( options );
 
+    // @private
+    this._supplyHintResponseOnNextFocus = false;
+
+    // Handle Voicing on focus in a more custom way
+    this.addInputListener( {
+      focus: () => {
+        this.voicingSpeakNameResponse( {
+          hintResponse: this._supplyHintResponseOnNextFocus ? this.voicingHintResponse : null
+        } );
+        this._supplyHintResponseOnNextFocus = false;
+      }
+    } );
+
     // @public (read-only)
     this.item = item;
 
@@ -110,6 +123,14 @@
       enter() { highlightRectangle.fill = options.highlightFill; },
       exit() { highlightRectangle.fill = null; }
     } );
+
+  }
+
+  /**
+   * @public
+   */
+  supplyHintResponseOnNextFocus() {
+    this._supplyHintResponseOnNextFocus = true;
   }
 }
 
Index: ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts
--- a/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(revision 157ce138001d4c3b898884e2b8110c8ad0ced191)
+++ b/ratio-and-proportion/js/discover/view/ChallengeRatioComboBoxNode.ts	(date 1644544993582)
@@ -81,7 +81,8 @@
       } )
     ], targetRatioProperty, comboBoxListParent, {
       helpText: ratioAndProportionStrings.a11y.discover.challengesHelpText,
-      voicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      comboBoxVoicingHintResponse: ratioAndProportionStrings.a11y.discover.challengesHelpText,
+      comboBoxVoicingCreateContextResponse: () => ratioDescriber.getProximityToNewChallengeRatioSentence(),
       maxWidth: 300, // empirically determined
 
       // phet-io

[zepumph]

• When this is done, let's also add this to molarity in Add Voicing to ComboBox molarity#243

[zepumph]

I finally got to a point where I felt good to commit. The RAP design doc (which has two examples of combo boxes, are both working well.

@terracoda, I a design question for you about the general combo box voicing implementaiton.

Right now we are not voicing anything when you close the listbox without selecting. i.e. escape/tab keys, or click away. Is this expected, or do you want to it repeat the object response (like when selecting the current selection from the listbox). I like it the way it is now since it is info that the user seems to not be asking for. Let me know if you agree.

After that I will pass it over for code review.

[pixelzoom]

I took a look at the shas, and I don't see anything amiss.

While investigating some of the new options that were added for voicing, I ran across a more general problem in related to voicing in common code. Options are not being documented, and do not not meet PhET coding standards for TypeScript. For example:

No documentation in Voicing.ts:

type VoicingSelfOptions = {
  voicingNameResponse?: VoicingResponse,
  voicingObjectResponse?: VoicingResponse,
  voicingContextResponse?: VoicingResponse,
  voicingHintResponse?: VoicingResponse,
  voicingUtteranceQueue?: UtteranceQueue,
  voicingResponsePatternCollection?: ResponsePatternCollection,
  voicingIgnoreVoicingManagerProperties?: boolean,
  voicingFocusListener?: SceneryListenerFunction<FocusEvent> | null

  // The utterance to use if you want this response to be more controlled in the UtteranceQueue. This Utterance will be
  // used by all responses spoken by this class.
  voicingUtterance?: Utterance;
};

No documentation in AccessibleValueHandler.ts:

type VoicingOnEndResponseOptions = {
  onlyOnValueChange?: boolean;
  withObjectResponse?: boolean;
};

... and there may be others.

Is voicing work getting a proper code review? (Asking because I was not asked to review general voicing here, only ComboBox.)

[pixelzoom]

In ResponsePacket.ts, options do not follow PhET new pattern. You have:

type ResponsePacketOptions = {
 ... // a bunch of options specific to ResponsePacket
};

Should be:

type SelfOptions = {
 ... // a bunch of options specific to ResponsePacket
};

type ResponsePacketOptions = SelfOptions;

See Voicing.ts for an example of the correct pattern usage.

[pixelzoom]

Back to @zepumph.

[zepumph]

It seems clear that we don't yet have a good overview document explaining voicing as a feature. JG wrote https://github.com/phetsims/qa/blob/master/documentation/qa-book.md#voicing, which is quite good, though likely not where we want scenery users to go in the future. But for now it may be helpful.

Options are not being documented, and do not not meet PhET coding standards for TypeScript. For example:

I updated documentation for VoicingOptions. It isn't clear that this is universal a universal standpoint. Indeed where options are defined is a great place to document options, but my understanding was not that this is the sole place where documentation must occur. For example, see NodeOptions, which are all mutated such that set* functions are the primary spot for documentation.

RE: #726 (comment)

This is not what I remember from out conversation. @samreid mentioned that he was doing this improve his understanding of the pattern, but this step feels superfluous and adds unneeded logic since phetsims/phet-core#105, where the default to SelfOptions because the ProvidedOptions. When there is no supertype, I strongly feel that ResponseCollector is written idiomatically and conventionally.

In general many of these comments are just making me more worried about how we will ensure that latest and greatest typescript convention decisions are propagated to all code. We will need to stay vigilant for quite some time to make sure that common code Typescript is as good as it can be. (. . . michael steps down from soapbox).

[pixelzoom]

In #726 (comment), @zepumph said:

... I updated documentation for VoicingOptions. It isn't clear that this is universal a universal standpoint. Indeed where options are defined is a great place to document options, but my understanding was not that this is the sole place where documentation must occur.

From Monday's TypeScript meeting, @zepumph wrote in phetsims/chipper#1201 (comment):

We will also add that we prefer having the API doc for options where the option is defined in the type, instead of where the default value is defined.

So it sounds like the convention was established on Monday, but you're arguing that it doesn't need to be applied universally. Please clarify, I'm confused.

Re use of SelfOptions, @zepumph said:

This is not what I remember from out conversation. @samreid mentioned that he was doing this improve his understanding of the pattern, but this step feels superfluous and adds unneeded logic since phetsims/phet-core#105, where the default to SelfOptions because the ProvidedOptions. When there is no supertype, I strongly feel that ResponseCollector is written idiomatically and conventionally.

Having exceptions to the convention:

• makes the convention more difficult to explain
• muddies the waters when reading usages of optionize<ProvidedOptions, SelfOptions, ParentOptions>, as in ResponsePacketOptions
• make it more likely that SelfOptions won't be properly factored out when a future change makes it appropriate to do so

So my preference would be to stick to the convention, since it has a lot of benefits. That's what I'll be doing in code that I write.

[zepumph]

So my preference would be to stick to the convention, since it has a lot of benefits. That's what I'll be doing in code that I write.

If that is the case, please explain a case to me where it is conventional to use the default type parameter for SelfOptions (which is ProvidedOptions).

[pixelzoom]

The fact that SelfOptions defaults to ProvidedOptions is an implementation detail that's internal to optionize. Why should I care, and shouldn't I avoid writing code that depends on that?

If you're asking when it's appropriate to use optionize with 1 type variable, like:

optionize<MyClassOptions >

... I really don't know. I haven't had a use for it. And I still don't understand the optionize<ItemOptions> example in WilderOptionsPattern.ts that we're discussing in phetsims/phet-core#105.

[zepumph]

Sounds good let's continue that discussion over there.

I took a look at the shas, and I don't see anything amiss.

Everything non combo-box related was dealt will or will be over in phetsims/phet-core#105. I think we are ready to close. Thanks all.

[zepumph]

Over in phetsims/ratio-and-proportion#445 we are needing to make one small tweak to ComboBox voicing. When selecting the button, which opens the list box, we want to hear the Name response + hint, not just the object + hint.

[pixelzoom]

After @zepumph reopened, I'm still assigned. So self unassigning.

[zepumph]

Looks like this was done in ab3bc7c, but never followed up here. All is good and phetsims/ratio-and-proportion#445 has been closed.