Skip to content

Commit

Permalink
Bug fix where "sim voicing off" is not spoken, #846
Browse files Browse the repository at this point in the history
  • Loading branch information
@zepumph
zepumph committed Oct 7, 2022
1 parent f791974 commit 5c5770e
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 18 deletions.
27 changes: 14 additions & 13 deletions js/preferences/PreferencesToggleSwitch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export type PreferencesToggleSwitchOptions = SelfOptions & NodeOptions;

class PreferencesToggleSwitch<T> extends Node {
private readonly disposePreferencesToggleSwitch: () => void;
public readonly toggleSwitch: ToggleSwitch<T>;

public constructor( property: Property<T>, leftValue: T, rightValue: T, providedOptions?: PreferencesToggleSwitchOptions ) {
const options = optionize<PreferencesToggleSwitchOptions, SelfOptions, NodeOptions>()( {
Expand All @@ -88,7 +89,7 @@ class PreferencesToggleSwitch<T> 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 ),
Expand All @@ -105,14 +106,14 @@ class PreferencesToggleSwitch<T> 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
Expand All @@ -122,19 +123,19 @@ class PreferencesToggleSwitch<T> extends Node {

if ( alert ) {
this.alertDescriptionUtterance( alert );
toggleSwitch.voicingSpeakResponse( {
this.toggleSwitch.voicingSpeakResponse( {
contextResponse: Utterance.alertableToText( alert )
} );
}
};
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.
Expand All @@ -145,12 +146,12 @@ class PreferencesToggleSwitch<T> 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' );
Expand All @@ -176,7 +177,7 @@ class PreferencesToggleSwitch<T> extends Node {

this.disposePreferencesToggleSwitch = () => {
property.unlink( valueListener );
toggleSwitch.dispose();
this.toggleSwitch.dispose();
};
}

Expand Down
34 changes: 29 additions & 5 deletions js/toolbar/VoicingToolbarItem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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 ];

Expand Down Expand Up @@ -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<string>, a11yLabel: TReadOnlyProperty<string>, labelAlignGroup: AlignGroup, inputAlignGroup: AlignGroup, lookAndFeel: LookAndFeel, createAlert: () => string ) {
public constructor(
labelString: TReadOnlyProperty<string>,
a11yLabel: TReadOnlyProperty<string>,
labelAlignGroup: AlignGroup,
inputAlignGroup: AlignGroup,
lookAndFeel: LookAndFeel,
createAlert: () => string,
muteSwitchUtterance: Utterance
) {

this.lookAndFeel = lookAndFeel;
this.objectResponseUtterance = new Utterance();
Expand Down Expand Up @@ -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 );
Expand Down

0 comments on commit 5c5770e

Please sign in to comment.