Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review Voicing (Interactive Description) design for radio buttons #122

Open
jessegreenberg opened this issue Jan 10, 2025 · 6 comments
Open

Comments

@jessegreenberg
Copy link
Contributor

From #108, @terracoda would like to review the Voicing design for radio buttons. The accessibleName and helpText for the group are not delivered in the Voicing design.

@terracoda terracoda changed the title Review Voicing design for radio buttons Review Voicing (Interactive Description) design for radio buttons Jan 14, 2025
@terracoda
Copy link

terracoda commented Jan 14, 2025

For Interactive Description, I think we should use the radio button design pattern we used for Ohm's Law for Tri Tour:
Screenshot 2025-01-14 at 12 05 57

This means for Interactive Description we need to add help text to the PDOM:

  • "Choose unit for angle."

Note: I changed the content order for the help text from "Choose angle units" to "Choose units for angle," to make it very clear one is choosing the units and not choosing an angle.

Note: We could remove "Angle" from the H3-group name, but I think it is best to leave the repetition here for now, until we get a sense of what learners think. The help text is accessible on-demand in both the case of Interactive Description and Voicing, so a learner does not have to hear it if they do not need it. We can always adjust when we add Tier 2 description.

@jessegreenberg, the Angle Units radio button group should handle the placement of the help text automatically, but in case it does not. The help text should come after the H3 and before the radio buttons. The Trig Functions radio group should work the same, and it already has the help text implemented in the PDOM, though I might update the actual content of the help text (separate issue).

For Voicing, when the Helpful Hints option in Preferences is checked, voice the hint response on focus of the radio button groups. When that option is not checked, learners are choosing less information, so it is fine to not voice any additional information besides the radio button names.

I've updated the design doc to match this issue.

@terracoda
Copy link

Tagging @kathy-phet and @amanda-phet just to let them know I reviewed the design for both the Interactive Description and the Voicing for the radio button groups.

@amanda-phet
Copy link
Contributor

I think this change makes sense.

@terracoda
Copy link

terracoda commented Jan 17, 2025

Designer Note:
For the Voicing feature "unit" and "angle" need to be in the help text because the Voicing Responses do not easily capture group names that come from heading content in the PDOM. We can make up for this by providing that detail, i.e."angle units" in the help text.

@jessegreenberg
Copy link
Contributor Author

The tricky part about this is the Voicing request:

For Voicing, when the Helpful Hints option in Preferences is checked, voice the hint response on focus of the radio button groups.

Neither scenery focus events nor Voicing have a good way to accomplish only voicing the hint response when focus first lands on a group of items. Ill think about this part and report back if it takes up too much time.

@jessegreenberg
Copy link
Contributor Author

OK, I think this will work well. About ready to commit but need to join another meeting - to be finished up soon:

Subject: [PATCH] Use PhetioIDUtilsModule, see https://github.com/phetsims/tandem/issues/316
---
Index: scenery/js/accessibility/GroupFocusListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/accessibility/GroupFocusListener.ts b/scenery/js/accessibility/GroupFocusListener.ts
new file mode 100644
--- /dev/null	(date 1737563957515)
+++ b/scenery/js/accessibility/GroupFocusListener.ts	(date 1737563957515)
@@ -0,0 +1,63 @@
+// Copyright 2021-2024, University of Colorado Boulder
+
+/**
+ * A listener that helps track focus for groups of Nodes. Lets you
+ *
+ * @author Jesse Greenberg (PhET Interactive Simulations)
+ */
+
+import BooleanProperty from '../../../axon/js/BooleanProperty.js';
+import Property from '../../../axon/js/Property.js';
+import TProperty from '../../../axon/js/TProperty.js';
+import { FocusManager, Node, scenery, SceneryEvent, TInputListener } from '../imports.js';
+
+export default class GroupFocusListener implements TInputListener {
+
+  // True when the focus is somewhere in this group.
+  public readonly focusInGroupProperty: TProperty<boolean>;
+
+  // The target of the focus, or null if the focus is not in this group.
+  public readonly focusTargetProperty: TProperty<Node | null>;
+
+  private _focusWasInGroup = false;
+
+  private readonly groupParent: Node;
+
+  public constructor( groupParent: Node ) {
+
+    this.focusInGroupProperty = new BooleanProperty( false );
+    this.focusTargetProperty = new Property( null );
+    this.groupParent = groupParent;
+  }
+
+  public focusin( event: SceneryEvent ): void {
+    this.focusInGroupProperty.value = true;
+    this.focusTargetProperty.value = event.target;
+    this._focusWasInGroup = true;
+  }
+
+  public focusout( event: SceneryEvent ): void {
+    const nextTargetTrail = FocusManager.pdomFocusProperty.value?.trail;
+    if ( nextTargetTrail && nextTargetTrail.containsNode( this.groupParent ) ) {
+
+      // The focusTargetProperty will be updated in the focusin event so there is
+      // nothing to do
+    }
+    else {
+      this.focusInGroupProperty.value = false;
+      this.focusTargetProperty.value = null;
+      this._focusWasInGroup = false;
+    }
+  }
+
+  public get focusWasInGroup(): boolean {
+    return this._focusWasInGroup;
+  }
+
+  public dispose(): void {
+    this.focusInGroupProperty.dispose();
+    this.focusTargetProperty.dispose();
+  }
+}
+
+scenery.register( 'GroupFocusListener', GroupFocusListener );
\ No newline at end of file
Index: sun/js/AquaRadioButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/AquaRadioButtonGroup.ts b/sun/js/AquaRadioButtonGroup.ts
--- a/sun/js/AquaRadioButtonGroup.ts	(revision 508c4611552a1edba4609920593598c4329dce71)
+++ b/sun/js/AquaRadioButtonGroup.ts	(date 1737563152380)
@@ -13,7 +13,7 @@
 import optionize, { combineOptions } from '../../phet-core/js/optionize.js';
 import PickOptional from '../../phet-core/js/types/PickOptional.js';
 import StrictOmit from '../../phet-core/js/types/StrictOmit.js';
-import { assertNoAdditionalChildren, FlowBox, FlowBoxOptions, KeyboardUtils, ParallelDOM, ParallelDOMOptions, PDOMPeer, SceneryConstants, SceneryEvent, TrimParallelDOMOptions } from '../../scenery/js/imports.js';
+import { assertNoAdditionalChildren, FlowBox, FlowBoxOptions, GroupFocusListener, KeyboardUtils, ParallelDOM, ParallelDOMOptions, PDOMPeer, SceneryConstants, SceneryEvent, TrimParallelDOMOptions } from '../../scenery/js/imports.js';
 import multiSelectionSoundPlayerFactory from '../../tambo/js/multiSelectionSoundPlayerFactory.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import AquaRadioButton, { AquaRadioButtonOptions } from './AquaRadioButton.js';
@@ -131,6 +131,24 @@
 
     super( options );
 
+    const groupFocusListener = new GroupFocusListener( this );
+    this.addInputListener( groupFocusListener );
+
+    groupFocusListener.focusTargetProperty.link( focusTarget => {
+      if ( focusTarget ) {
+        const targetButton = focusTarget as AquaRadioButton<T>;
+        if ( groupFocusListener.focusWasInGroup ) {
+          targetButton.voicingSpeakNameResponse();
+        }
+        else {
+          targetButton.voicingSpeakFullResponse( {
+            contextResponse: null,
+            hintResponse: this.helpText
+          } );
+        }
+      }
+    } );
+
     // pdom - this node's primary sibling is aria-labelledby its own label so the label content is read whenever
     // a member of the group receives focus
     this.addAriaLabelledbyAssociation( {
@@ -167,6 +185,7 @@
       this.removeInputListener( intentListener );
       radioButtons.forEach( radioButton => radioButton.dispose() );
       this.onInputEmitter.dispose();
+      groupFocusListener.dispose();
       nodes.forEach( node => node.dispose() );
     };
 
Index: scenery/js/imports.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/imports.ts b/scenery/js/imports.ts
--- a/scenery/js/imports.ts	(revision 6c7e027cf85ee31cc62db294e9ec7f87ce659261)
+++ b/scenery/js/imports.ts	(date 1737562828136)
@@ -186,6 +186,7 @@
 export type { ReadingBlockUtteranceOptions } from './accessibility/voicing/ReadingBlockUtterance.js';
 export { default as FocusDisplayedController } from './accessibility/FocusDisplayedController.js';
 export { default as FocusManager } from './accessibility/FocusManager.js';
+export { default as GroupFocusListener } from './accessibility/GroupFocusListener.js';
 export { default as HighlightPath } from './accessibility/HighlightPath.js';
 export type { HighlightPathOptions } from './accessibility/HighlightPath.js';
 export { default as GroupHighlightPath } from './accessibility/GroupHighlightPath.js';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants