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

Add Voicing #363

Closed
zepumph opened this issue Mar 2, 2021 · 13 comments
Closed

Add Voicing #363

zepumph opened this issue Mar 2, 2021 · 13 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Mar 2, 2021

No description provided.

@terracoda
Copy link
Contributor

@zepumph, for clarity, in other repos, I have been using (Self-Voicing) to preface the Self-Voicing design implementation ;-)

@zepumph zepumph changed the title Add self voicing Add Self-Voicing Mar 9, 2021
@zepumph
Copy link
Member Author

zepumph commented Apr 2, 2021

@zepumph
Copy link
Member Author

zepumph commented Apr 2, 2021

I am having trouble figuring out where to start here. This is all not a big deal, but I feel like it is important to fully explain what I have tried to do, and how it hasn't been fruitful to try to progress voicing as a feature.

At this point I have no idea if this is my fault, and I just don't understand how to enabled/use this feature, or if there are bugs blocking me. It doesn't seem very cost-effective to stay at this on my own, so I'm going to bring in @jessegreenberg for some assistance.

Currently my priorities for this issue are as follows:

  • Get some self voicing playing from master/branches (unbuilt mode, not in dev versions)
  • Outfit Ratio and Proportion with the necessary boilerplate to begin working on voicing here
  • Begin with this table about the "My Challenge" accordion box in the design doc: Table: All Inputs for My Challenge accordion box (ready)

@zepumph
Copy link
Member Author

zepumph commented Apr 14, 2021

Index: js/ratio-and-proportion-main.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/ratio-and-proportion-main.js b/js/ratio-and-proportion-main.js
--- a/js/ratio-and-proportion-main.js	(revision c2d885ae1c247d0eb4424c9a8b7d21114245a22a)
+++ b/js/ratio-and-proportion-main.js	(date 1618357210767)
@@ -6,7 +6,7 @@
  * @author Michael Kauzmann (PhET Interactive Simulations)
  */
 
-// import PreferencesConfiguration from '../../joist/js/preferences/PreferencesConfiguration.js';
+import PreferencesConfiguration from '../../joist/js/preferences/PreferencesConfiguration.js';
 import Sim from '../../joist/js/Sim.js';
 import simLauncher from '../../joist/js/simLauncher.js';
 import Tandem from '../../tandem/js/Tandem.js';
@@ -24,12 +24,15 @@
     qualityAssurance: 'Logan Bray, Steele Dalton, Megan Lai, Brooklyn Lash, Liam Mulhall, Devon Quispe, Kathryn Woessner',
     soundDesign: 'Ashton Morris'
   },
-  hasKeyboardHelpContent: true
-  // preferencesConfiguration: new PreferencesConfiguration( {
-  //   audioOptions: {
-  //     supportsVoicing: true
-  //   }
-  // } )
+  hasKeyboardHelpContent: true,
+  preferencesConfiguration: new PreferencesConfiguration( {
+    audioOptions: {
+      supportsVoicing: true
+    },
+    visualOptions: {
+      supportsInteractiveHighlights: true
+    }
+  } )
 };
 
 // launch the sim - beware that scenery Image nodes created outside of simLauncher.launch() will have zero bounds
Index: ratio-and-proportion-strings_en.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion-strings_en.json b/ratio-and-proportion-strings_en.json
--- a/ratio-and-proportion-strings_en.json	(revision c2d885ae1c247d0eb4424c9a8b7d21114245a22a)
+++ b/ratio-and-proportion-strings_en.json	(date 1618357904841)
@@ -525,6 +525,12 @@
       "currentChallengeHidden": {
         "value": "Current challenge hidden."
       },
+      "myChallenge": {
+        "value": "My challenge {{targetAntecedent}} to {{targetConsequent}}."
+      },
+      "myChallengeHidden": {
+        "value": "Current challenge hidden."
+      },
       "capitalized": {
         "at": {
           "value": "At"
Index: js/create/view/MyChallengeAccordionBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/create/view/MyChallengeAccordionBox.js b/js/create/view/MyChallengeAccordionBox.js
--- a/js/create/view/MyChallengeAccordionBox.js	(revision c2d885ae1c247d0eb4424c9a8b7d21114245a22a)
+++ b/js/create/view/MyChallengeAccordionBox.js	(date 1618357994926)
@@ -151,11 +151,20 @@
     this.expandedProperty.value = DEFAULT_EXPANDED;
 
     const accordionBoxUtterance = new ActivationUtterance();
+    const accordionBoxVoicingUtterance = new ActivationUtterance();
     this.expandedProperty.lazyLink( expanded => {
-      accordionBoxUtterance.alert = expanded ?
-                                    ratioDescriber.getCurrentChallengeSentence( targetAntecedentProperty.value, targetConsequentProperty.value ) :
-                                    ratioAndProportionStrings.a11y.ratio.currentChallengeHidden;
+
+      if ( expanded ) {
+        accordionBoxUtterance.alert = ratioDescriber.getCurrentChallengeSentence( targetAntecedentProperty.value, targetConsequentProperty.value );
+        accordionBoxVoicingUtterance.alert = ratioDescriber.getMyChallengeSentence( targetAntecedentProperty.value, targetConsequentProperty.value );
+      }
+      else {
+        accordionBoxUtterance.alert = ratioAndProportionStrings.a11y.ratio.currentChallengeHidden;
+        accordionBoxVoicingUtterance.alert = ratioAndProportionStrings.a11y.ratio.myChallengeHidden;
+      }
+
       phet.joist.sim.utteranceQueue.addToBack( accordionBoxUtterance );
+      phet.joist.sim.voicingUtteranceQueue && phet.joist.sim.voicingUtteranceQueue.addToBack( accordionBoxVoicingUtterance );
     } );
 
     Property.multilink( [ targetAntecedentProperty, targetConsequentProperty ], ( targetAntecedent, targetConsequent ) => {
Index: js/common/view/describers/RatioDescriber.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/describers/RatioDescriber.js b/js/common/view/describers/RatioDescriber.js
--- a/js/common/view/describers/RatioDescriber.js	(revision c2d885ae1c247d0eb4424c9a8b7d21114245a22a)
+++ b/js/common/view/describers/RatioDescriber.js	(date 1618357904849)
@@ -136,6 +136,19 @@
     } );
   }
 
+  /**
+   * @public
+   * @param {number} antecedent
+   * @param {number} consequent
+   * @returns {string}
+   */
+  getMyChallengeSentence( antecedent, consequent ) {
+    return StringUtils.fillIn( ratioAndProportionStrings.a11y.ratio.myChallenge, {
+      targetAntecedent: antecedent,
+      targetConsequent: consequent
+    } );
+  }
+
   /**
    * @public
    * @param {number} antecedent

@zepumph
Copy link
Member Author

zepumph commented Nov 11, 2021

Since last working on this issue, we have merged voicing implementation into master, and I have converted RAP to typescript. Both of these items have caused quite a bit of divergence between the rap preferences branch and master. Here is the list of commits (from newest to oldest) that are in the preferences branch but not on master. I should go through each one to make sure I get master up to its best spot before proceeding with new voicing code.

df4fcab
3bbc8a2
43276b6
5286638
6493040
293897a
51e1c60
663aa9c -> BothHandsPDOMNode see #388
e55182a
0669c50
5a1df5b
861ce67
aa510d8 -> my challenge accordion box
e790141
c616115 -> my challenge accordion box
fc470ad
8a5a939
81515b4
268adf8
d5cf235 -> my challenge accordion box
f493080
ff895fd -> tick mark radio button group
3f21ef1 -> tick mark radio button group
40c5bee
eaebd78
ec7c6de
196c79b
4129501 - AccordionBox not applied, see #381 (comment)
fd8845a - AccordionBox not applied, see #381 (comment)
25c797c - AccordionBox not applied, see #381 (comment)
6a83969
f271b8a
7eace29
c2d885a
463469e
bf66e29

@zepumph
Copy link
Member Author

zepumph commented Nov 11, 2021

Also, note that voicing was added to master in #410 (oops).

@zepumph
Copy link
Member Author

zepumph commented Nov 11, 2021

Going through, most was just merging with master or bumping the version number for a deploy. Basically the three pieces that are on the branch that we should copy functionality out for when it is time is MyChallengeAccordionBox, TickMarkRadioButtonGroup, and one commit to BothHandsPDOMNode. I've annotated the above commits accordingly.

@terracoda
Copy link
Contributor

I agree @zepumph, good to get all boiler plate stuff ready, and then I thought it might be useful to implement my requested changes to PDOM strings. This will hopefully be beneficial down the road.

@terracoda
Copy link
Contributor

As I said on slack, I marked issues that have PDOM changes as hi priroty.

zepumph added a commit that referenced this issue Feb 7, 2022
zepumph added a commit that referenced this issue Feb 7, 2022
@zepumph
Copy link
Member Author

zepumph commented Feb 7, 2022

I added the details and overview buttons for the screens above.

@zepumph
Copy link
Member Author

zepumph commented Feb 24, 2022

@terracoda FYI, I just turned on interactive highlights.

@zepumph
Copy link
Member Author

zepumph commented Mar 2, 2022

I believe that every item in the design doc that is RAP-specific is done except the Both hands interaction.

@zepumph
Copy link
Member Author

zepumph commented Apr 27, 2022

This issue has run its course. We are feature complete, and will generally create new issues as needed to address issues that arise.

@zepumph zepumph closed this as completed Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants