From 5c5770ecb76d6e37fcff89b6d70de9cac3d543cb Mon Sep 17 00:00:00 2001 From: Michael Kauzmann Date: Fri, 7 Oct 2022 11:36:34 -0600 Subject: [PATCH] Bug fix where "sim voicing off" is not spoken, https://github.com/phetsims/joist/issues/846 --- js/preferences/PreferencesToggleSwitch.ts | 27 +++++++++--------- js/toolbar/VoicingToolbarItem.ts | 34 +++++++++++++++++++---- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/js/preferences/PreferencesToggleSwitch.ts b/js/preferences/PreferencesToggleSwitch.ts index a24e4dd4..5cfbe6c3 100644 --- a/js/preferences/PreferencesToggleSwitch.ts +++ b/js/preferences/PreferencesToggleSwitch.ts @@ -64,6 +64,7 @@ export type PreferencesToggleSwitchOptions = SelfOptions & NodeOptions; class PreferencesToggleSwitch extends Node { private readonly disposePreferencesToggleSwitch: () => void; + public readonly toggleSwitch: ToggleSwitch; public constructor( property: Property, leftValue: T, rightValue: T, providedOptions?: PreferencesToggleSwitchOptions ) { const options = optionize()( { @@ -88,7 +89,7 @@ class PreferencesToggleSwitch extends Node { super( options ); - const toggleSwitch = new ToggleSwitch( property, leftValue, rightValue, merge( options.toggleSwitchOptions, { + this.toggleSwitch = new ToggleSwitch( property, leftValue, rightValue, merge( options.toggleSwitchOptions, { // enabled:true by default, but disable if fuzzing when supporting voicing enabled: !( phet.chipper.isFuzzEnabled() && phet.chipper.queryParameters.supportsVoicing ), @@ -105,14 +106,14 @@ class PreferencesToggleSwitch extends Node { } ) ); if ( options.leftValueLabel ) { - options.leftValueLabel.right = toggleSwitch.left - options.valueLabelXSpacing; - options.leftValueLabel.centerY = toggleSwitch.centerY; - toggleSwitch.addChild( options.leftValueLabel ); + options.leftValueLabel.right = this.toggleSwitch.left - options.valueLabelXSpacing; + options.leftValueLabel.centerY = this.toggleSwitch.centerY; + this.toggleSwitch.addChild( options.leftValueLabel ); } if ( options.rightValueLabel ) { - options.rightValueLabel.left = toggleSwitch.right + options.valueLabelXSpacing; - options.rightValueLabel.centerY = toggleSwitch.centerY; - toggleSwitch.addChild( options.rightValueLabel ); + options.rightValueLabel.left = this.toggleSwitch.right + options.valueLabelXSpacing; + options.rightValueLabel.centerY = this.toggleSwitch.centerY; + this.toggleSwitch.addChild( options.rightValueLabel ); } // a11y - Describe the change in value if context responses were provided in options. Listener needs to be @@ -122,7 +123,7 @@ class PreferencesToggleSwitch extends Node { if ( alert ) { this.alertDescriptionUtterance( alert ); - toggleSwitch.voicingSpeakResponse( { + this.toggleSwitch.voicingSpeakResponse( { contextResponse: Utterance.alertableToText( alert ) } ); } @@ -130,11 +131,11 @@ class PreferencesToggleSwitch extends Node { property.lazyLink( valueListener ); // This component manages disabledOpacity, we don't want it to compound over subcomponents. - toggleSwitch.disabledOpacity = 1; + this.toggleSwitch.disabledOpacity = 1; this.disabledOpacity = SceneryConstants.DISABLED_OPACITY; this.enabledProperty.link( enabled => { - toggleSwitch.enabled = enabled; + this.toggleSwitch.enabled = enabled; } ); // Layout using GridBox and layoutOptions will accomplish the following when all components are available. @@ -145,12 +146,12 @@ class PreferencesToggleSwitch extends Node { } ); this.addChild( gridBox ); - toggleSwitch.layoutOptions = { + this.toggleSwitch.layoutOptions = { row: 0, column: 1, xAlign: 'right' }; - gridBox.addChild( toggleSwitch ); + gridBox.addChild( this.toggleSwitch ); if ( options.labelNode ) { assert && assert( options.labelNode.layoutOptions === null, 'PreferencesToggleSwitch will control layout' ); @@ -176,7 +177,7 @@ class PreferencesToggleSwitch extends Node { this.disposePreferencesToggleSwitch = () => { property.unlink( valueListener ); - toggleSwitch.dispose(); + this.toggleSwitch.dispose(); }; } diff --git a/js/toolbar/VoicingToolbarItem.ts b/js/toolbar/VoicingToolbarItem.ts index d56c613f..56084ade 100644 --- a/js/toolbar/VoicingToolbarItem.ts +++ b/js/toolbar/VoicingToolbarItem.ts @@ -10,7 +10,7 @@ import BooleanProperty from '../../../axon/js/BooleanProperty.js'; import PlayStopButton from '../../../scenery-phet/js/buttons/PlayStopButton.js'; import PhetFont from '../../../scenery-phet/js/PhetFont.js'; -import { AlignGroup, Display, HBox, Node, NodeOptions, ReadingBlockHighlight, SceneryEvent, Text, voicingManager, VoicingText, voicingUtteranceQueue } from '../../../scenery/js/imports.js'; +import { AlignGroup, Display, HBox, Node, NodeOptions, ReadingBlockHighlight, SceneryEvent, Text, Voicing, voicingManager, VoicingText, voicingUtteranceQueue } from '../../../scenery/js/imports.js'; import Tandem from '../../../tandem/js/Tandem.js'; import Utterance from '../../../utterance-queue/js/Utterance.js'; import joist from '../joist.js'; @@ -21,6 +21,7 @@ import LookAndFeel from '../LookAndFeel.js'; import optionize, { EmptySelfOptions } from '../../../phet-core/js/optionize.js'; import PickRequired from '../../../phet-core/js/types/PickRequired.js'; import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; +import animationFrameTimer from '../../../axon/js/animationFrameTimer.js'; // constants const CONTENT_VERTICAL_SPACING = 10; @@ -86,13 +87,19 @@ class VoicingToolbarItem extends Node { tandem: options.tandem.createTandem( 'muteSpeechSwitch' ) } ); + // We don't care about description for this Utterance because it fixes a voicing specific bug. + const toggleSwitchUtterance = muteSpeechSwitch.toggleSwitch.voicingUtterance; + // layout const labelAlignGroup = new AlignGroup(); const inputAlignGroup = new AlignGroup(); - const overviewRow = new LabelButtonRow( overviewStringProperty, playOverviewStringProperty, labelAlignGroup, inputAlignGroup, lookAndFeel, alertManager.createOverviewContent.bind( alertManager ) ); - const detailsRow = new LabelButtonRow( detailsStringProperty, playDetailsStringProperty, labelAlignGroup, inputAlignGroup, lookAndFeel, alertManager.createDetailsContent.bind( alertManager ) ); - const hintRow = new LabelButtonRow( hintStringProperty, playHintStringProperty, labelAlignGroup, inputAlignGroup, lookAndFeel, alertManager.createHintContent.bind( alertManager ) ); + const overviewRow = new LabelButtonRow( overviewStringProperty, playOverviewStringProperty, labelAlignGroup, inputAlignGroup, + lookAndFeel, alertManager.createOverviewContent.bind( alertManager ), toggleSwitchUtterance ); + const detailsRow = new LabelButtonRow( detailsStringProperty, playDetailsStringProperty, labelAlignGroup, inputAlignGroup, + lookAndFeel, alertManager.createDetailsContent.bind( alertManager ), toggleSwitchUtterance ); + const hintRow = new LabelButtonRow( hintStringProperty, playHintStringProperty, labelAlignGroup, inputAlignGroup, + lookAndFeel, alertManager.createHintContent.bind( alertManager ), toggleSwitchUtterance ); this.children = [ muteSpeechSwitch, quickInfoText, overviewRow.content, detailsRow.content, hintRow.content ]; @@ -147,8 +154,17 @@ class LabelButtonRow { * @param inputAlignGroup - To align all inputs in the VoicingToolbarItem * @param lookAndFeel * @param createAlert - function that creates the alert when the button is pressed + * @param muteSwitchUtterance */ - public constructor( labelString: TReadOnlyProperty, a11yLabel: TReadOnlyProperty, labelAlignGroup: AlignGroup, inputAlignGroup: AlignGroup, lookAndFeel: LookAndFeel, createAlert: () => string ) { + public constructor( + labelString: TReadOnlyProperty, + a11yLabel: TReadOnlyProperty, + labelAlignGroup: AlignGroup, + inputAlignGroup: AlignGroup, + lookAndFeel: LookAndFeel, + createAlert: () => string, + muteSwitchUtterance: Utterance + ) { this.lookAndFeel = lookAndFeel; this.objectResponseUtterance = new Utterance(); @@ -189,9 +205,17 @@ class LabelButtonRow { if ( endedUtterance === this.objectResponseUtterance ) { this.playingProperty.set( false ); + const hasMuteSwitchUtterance = voicingUtteranceQueue.hasUtterance( muteSwitchUtterance ); + // clear the voicingUtteranceQueue because stale alerts may have collected while the quick info button announced voicingUtteranceQueue.clear(); + // We don't want to trigger an utteranceQueue change in response to an utterance change---it results in + // an infinite loop. + hasMuteSwitchUtterance && animationFrameTimer.runOnNextTick( () => { + Voicing.alertUtterance( muteSwitchUtterance ); + } ); + // Remove if listener wasn't interrupted by Display input. if ( Display.inputListeners.includes( displayListener ) ) { Display.removeInputListener( displayListener );