From 8a5b0702eb9ddd0455d98e6ca84fdcebd27e77e8 Mon Sep 17 00:00:00 2001 From: zepumph Date: Fri, 22 Apr 2022 14:21:24 -0600 Subject: [PATCH] go-beyond-edge should change based on most recent input modality, https://github.com/phetsims/ratio-and-proportion/issues/457 --- .../view/BothHandsInteractionListener.ts | 7 +- js/common/view/BothHandsPDOMNode.ts | 18 +++-- js/common/view/RAPScreenView.ts | 3 +- js/common/view/RatioHalf.ts | 11 ++- .../view/describers/BothHandsDescriber.ts | 9 ++- .../view/describers/HandPositionsDescriber.ts | 74 ++++++++++++++----- js/create/view/CreateScreenView.ts | 2 +- js/discover/view/DiscoverScreenView.ts | 2 +- 8 files changed, 90 insertions(+), 36 deletions(-) diff --git a/js/common/view/BothHandsInteractionListener.ts b/js/common/view/BothHandsInteractionListener.ts index 82ec59d0..59304aab 100644 --- a/js/common/view/BothHandsInteractionListener.ts +++ b/js/common/view/BothHandsInteractionListener.ts @@ -20,10 +20,11 @@ import Range from '../../../../dot/js/Range.js'; import BoundarySoundClip from './sound/BoundarySoundClip.js'; import TickMarkBumpSoundClip from './sound/TickMarkBumpSoundClip.js'; import optionize from '../../../../phet-core/js/optionize.js'; +import RatioInputModality from './describers/RatioInputModality.js'; const TOTAL_RANGE = rapConstants.TOTAL_RATIO_TERM_VALUE_RANGE; -type OnInputType = ( knockOutOfLock?: boolean ) => void; +type OnInputType = ( ratioInputModality: RatioInputModality, knockOutOfLock?: boolean ) => void; const onInputDefault = () => {}; type getIdealTermType = ( ratioTerm: RatioTerm ) => number; @@ -139,7 +140,7 @@ class BothHandsInteractionListener { this.tickMarkBumpSoundClip.onInteract( this.ratioTupleProperty.value.getForTerm( ratioTerm ) ); - this.onInput(); + this.onInput( ratioTerm ); } blur(): void { @@ -223,7 +224,7 @@ class BothHandsInteractionListener { this.consequentInteractedWithProperty.value = true; // Extra arg here because this input can "knock" the ratio out of locked mode, see https://github.com/phetsims/ratio-and-proportion/issues/227 - this.onInput( wasLocked && !this.ratioLockedProperty.value ); + this.onInput( RatioInputModality.BOTH_HANDS, wasLocked && !this.ratioLockedProperty.value ); break; } diff --git a/js/common/view/BothHandsPDOMNode.ts b/js/common/view/BothHandsPDOMNode.ts index 8cf5f66b..c73992a6 100644 --- a/js/common/view/BothHandsPDOMNode.ts +++ b/js/common/view/BothHandsPDOMNode.ts @@ -27,6 +27,7 @@ import TickMarkView from './TickMarkView.js'; import EnumerationProperty from '../../../../axon/js/EnumerationProperty.js'; import IReadOnlyProperty from '../../../../axon/js/IReadOnlyProperty.js'; import TickMarkDescriber from './describers/TickMarkDescriber.js'; +import RatioInputModality from './describers/RatioInputModality.js'; // constants const OBJECT_RESPONSE_DELAY = 500; @@ -96,7 +97,6 @@ class BothHandsPDOMNode extends Node { innerContent: ratioAndProportionStrings.a11y.bothHands.bothHands, voicingNameResponse: ratioAndProportionStrings.a11y.bothHands.bothHands, voicingObjectResponse: () => this.voicingBothHandsDescriber.getBothHandsObjectResponse(), - voicingContextResponse: () => this.voicingBothHandsDescriber.getBothHandsContextResponse(), interactiveHighlight: 'invisible', ariaLabel: ratioAndProportionStrings.a11y.bothHands.bothHands } @@ -181,7 +181,7 @@ class BothHandsPDOMNode extends Node { targetRatioProperty: options.targetRatioProperty, getIdealTerm: options.getIdealTerm, inProportionProperty: options.inProportionProperty, - onInput: ( knockedOutOfLock?: boolean ) => { + onInput: ( ratioInputModality, knockedOutOfLock? ) => { if ( knockedOutOfLock ) { this.alertDescriptionUtterance( this.ratioUnlockedFromBothHandsUtterance ); @@ -190,10 +190,15 @@ class BothHandsPDOMNode extends Node { } ); } - this.alertBothHandsContextResponse( knockedOutOfLock ); + this.alertBothHandsDescriptionContextResponse( ratioInputModality, knockedOutOfLock ); + + // Voicing object/context response interactiveNode.voicingSpeakFullResponse( { nameResponse: null, - hintResponse: null + hintResponse: null, + + // ratioInputModality is needed for the context response so we can't set that through voicingContextResponse + contextResponse: this.voicingBothHandsDescriber.getBothHandsContextResponse( ratioInputModality ) } ); } } ); @@ -292,12 +297,13 @@ class BothHandsPDOMNode extends Node { this.alertDescriptionUtterance( utterance ); } - private alertBothHandsContextResponse( knockedOutOfLock?: boolean ): void { + // Just for description, not voicing + private alertBothHandsDescriptionContextResponse( ratioInputModality: RatioInputModality, knockedOutOfLock?: boolean ): void { if ( knockedOutOfLock ) { this.contextResponseUtterance.alert = this.descriptionBothHandsDescriber.getBothHandsObjectResponse(); } else { - this.contextResponseUtterance.alert = this.descriptionBothHandsDescriber.getBothHandsContextResponse(); + this.contextResponseUtterance.alert = this.descriptionBothHandsDescriber.getBothHandsContextResponse( ratioInputModality ); } this.alertDescriptionUtterance( this.contextResponseUtterance ); diff --git a/js/common/view/RAPScreenView.ts b/js/common/view/RAPScreenView.ts index 7469b070..aff37a27 100644 --- a/js/common/view/RAPScreenView.ts +++ b/js/common/view/RAPScreenView.ts @@ -54,6 +54,7 @@ import IReadOnlyProperty from '../../../../axon/js/IReadOnlyProperty.js'; import RAPMediaPipe from './RAPMediaPipe.js'; import Utterance from '../../../../utterance-queue/js/Utterance.js'; import ResponsePacket from '../../../../utterance-queue/js/ResponsePacket.js'; +import RatioInputModality from './describers/RatioInputModality.js'; // constants const LAYOUT_BOUNDS = ScreenView.DEFAULT_LAYOUT_BOUNDS; @@ -265,7 +266,7 @@ class RAPScreenView extends ScreenView { const mediaPipeVoicingUtterance = new Utterance( { alert: new ResponsePacket( { objectResponse: () => mediaPipeBothHandsDescriber.getBothHandsObjectResponse(), - contextResponse: () => mediaPipeBothHandsDescriber.getBothHandsContextResponse() + contextResponse: () => mediaPipeBothHandsDescriber.getBothHandsContextResponse( RatioInputModality.BOTH_HANDS ) } ), // This number should be small, so that the most recent alert in the queue will immediately play once the announcer diff --git a/js/common/view/RatioHalf.ts b/js/common/view/RatioHalf.ts index b5f09de1..0bac18ab 100644 --- a/js/common/view/RatioHalf.ts +++ b/js/common/view/RatioHalf.ts @@ -154,8 +154,13 @@ class RatioHalf extends Rectangle { this.ratio = options.ratio; const tickMarkDescriber = new TickMarkDescriber( options.tickMarkRangeProperty, options.tickMarkViewProperty ); - this.descriptionHandPositionsDescriber = new HandPositionsDescriber( this.ratio.tupleProperty, tickMarkDescriber, options.inProportionProperty, this.ratio.enabledRatioTermsRangeProperty ); - this.voicingHandPositionsDescriber = new HandPositionsDescriber( this.ratio.tupleProperty, tickMarkDescriber, options.inProportionProperty, this.ratio.enabledRatioTermsRangeProperty ); + + this.descriptionHandPositionsDescriber = new HandPositionsDescriber( this.ratio.tupleProperty, tickMarkDescriber, + options.inProportionProperty, this.ratio.enabledRatioTermsRangeProperty, this.ratio.lockedProperty ); + + this.voicingHandPositionsDescriber = new HandPositionsDescriber( this.ratio.tupleProperty, tickMarkDescriber, + options.inProportionProperty, this.ratio.enabledRatioTermsRangeProperty, this.ratio.lockedProperty ); + this.tickMarkViewProperty = options.tickMarkViewProperty; this.ratioTerm = options.ratioTerm; @@ -502,7 +507,7 @@ class RatioHalf extends Rectangle { // When locked, give a description of both-hands, instead of just a single one. if ( this.ratio.lockedProperty.value ) { - return bothHandsDescriber.getBothHandsContextResponse( options ); + return bothHandsDescriber.getBothHandsContextResponse( this.ratioTerm, options ); } return handPositionsDescriber.getSingleHandContextResponse( this.ratioTerm, this.tickMarkViewProperty.value, options ); diff --git a/js/common/view/describers/BothHandsDescriber.ts b/js/common/view/describers/BothHandsDescriber.ts index 19d74ee9..54528d5f 100644 --- a/js/common/view/describers/BothHandsDescriber.ts +++ b/js/common/view/describers/BothHandsDescriber.ts @@ -42,13 +42,14 @@ class BothHandsDescriber { this.enabledRatioTermsRangeProperty = enabledRatioTermsRangeProperty; this.tickMarkViewProperty = tickMarkViewProperty; this.ratioDescriber = ratioDescriber; - this.handPositionsDescriber = new HandPositionsDescriber( ratioTupleProperty, tickMarkDescriber, inProportionProperty, enabledRatioTermsRangeProperty ); this.ratioLockedProperty = ratioLockedProperty; + this.handPositionsDescriber = new HandPositionsDescriber( ratioTupleProperty, tickMarkDescriber, + inProportionProperty, enabledRatioTermsRangeProperty, this.ratioLockedProperty ); } - getBothHandsContextResponse( options?: HandContextResponseOptions ): string { - - const ratioLockedEdgeResponse = this.handPositionsDescriber.getGoBeyondContextResponse( this.ratioTupleProperty.value, RatioInputModality.BOTH_HANDS ); + getBothHandsContextResponse( recentlyMovedRatioTerm: RatioInputModality, options: HandContextResponseOptions = {} ): string { + const ratioLockedEdgeResponse = this.handPositionsDescriber.getGoBeyondContextResponse( + this.ratioTupleProperty.value, recentlyMovedRatioTerm ); if ( ratioLockedEdgeResponse ) { return ratioLockedEdgeResponse; } diff --git a/js/common/view/describers/HandPositionsDescriber.ts b/js/common/view/describers/HandPositionsDescriber.ts index e075fda7..e9fce359 100644 --- a/js/common/view/describers/HandPositionsDescriber.ts +++ b/js/common/view/describers/HandPositionsDescriber.ts @@ -125,6 +125,7 @@ class HandPositionsDescriber { private ratioTupleProperty: Property; private tickMarkDescriber: TickMarkDescriber; private inProportionProperty: IReadOnlyProperty; + private ratioLockedProperty: IReadOnlyProperty; // keep track of previous distance regions to track repetition, and alter description accordingly. This // is used for any modality getting a distance region in a context response. @@ -137,9 +138,11 @@ class HandPositionsDescriber { enabledRatioTermsRangeProperty: IReadOnlyProperty; constructor( ratioTupleProperty: Property, tickMarkDescriber: TickMarkDescriber, - inProportionProperty: IReadOnlyProperty, enabledRatioTermsRangeProperty: IReadOnlyProperty ) { + inProportionProperty: IReadOnlyProperty, enabledRatioTermsRangeProperty: IReadOnlyProperty, + ratioLockedProperty: IReadOnlyProperty ) { this.ratioTupleProperty = ratioTupleProperty; + this.ratioLockedProperty = ratioLockedProperty; this.tickMarkDescriber = tickMarkDescriber; this.inProportionProperty = inProportionProperty; this.enabledRatioTermsRangeProperty = enabledRatioTermsRangeProperty; @@ -256,7 +259,7 @@ class HandPositionsDescriber { distanceResponseType: DistanceResponseType.COMBO }, providedOptions ); - const ratioLockedEdgeResponse = this.getGoBeyondContextResponse( this.ratioTupleProperty.value, RatioInputModality.ANTECEDENT ); + const ratioLockedEdgeResponse = this.getGoBeyondContextResponse( this.ratioTupleProperty.value, ratioTerm ); if ( ratioLockedEdgeResponse ) { return ratioLockedEdgeResponse; } @@ -425,7 +428,20 @@ class HandPositionsDescriber { return distanceProgressString; } - getGoBeyondContextResponse( currentTuple: RAPRatioTuple, inputModality: RatioInputModality ): string | null { + /** + * Create a response trigger when at an edge, and you try to move beyond it. This takes into consideration, what the + * previous ratio value was, the current value, and what modality was most recently used. If just moving the antecedent + * or consequent, then the response is simpler about what hand moved, but we need to hand the "both hands moved at the + * same time" case as well. + * + * @param currentTuple + * @param mostRecentlyMoved - By specifying the RatioTerm last moved, handle the case where both terms are at the edge + * but only the changed one should reflect in the response. "Moved" is a misnomer because if at edge and trying to + * "go beyond", the position won't change value. + * + * @returns - null if there is no go-beyond-edge response + */ + getGoBeyondContextResponse( currentTuple: RAPRatioTuple, mostRecentlyMoved: RatioInputModality ): string | null { const enabledRange = this.enabledRatioTermsRangeProperty.value; @@ -434,37 +450,58 @@ class HandPositionsDescriber { const previousConsequentAtMin = this.previousEdgeCheckTuple.consequent === enabledRange.min; const previousConsequentAtMax = this.previousEdgeCheckTuple.consequent === enabledRange.max; - // No previous value was at an extremity. - if ( !( previousAntecedentAtMin || previousAntecedentAtMax || previousConsequentAtMin || previousConsequentAtMax ) ) { + // If moved both terms, and either were previously at an extremity + const movedEitherFromBothHandsAndPreviousAtEdge = mostRecentlyMoved === RatioInputModality.BOTH_HANDS && ( + previousAntecedentAtMin || previousAntecedentAtMax || previousConsequentAtMin || previousConsequentAtMax ); + + // If moved the antecedent, and it was previously at an extremity. + const movedAntecedentAndPreviousAtEdge = mostRecentlyMoved === RatioInputModality.ANTECEDENT && + ( previousAntecedentAtMin || previousAntecedentAtMax ); + + // If moved the consequen, and it was previously at an extremity. + const movedConsequentAndPreviousAtEdge = mostRecentlyMoved === RatioInputModality.CONSEQUENT && + ( previousConsequentAtMin || previousConsequentAtMax ); + + // No previous value was at an extremity when moving that modality. + if ( !( movedEitherFromBothHandsAndPreviousAtEdge || movedAntecedentAndPreviousAtEdge || + movedConsequentAndPreviousAtEdge ) ) { this.previousEdgeCheckTuple = currentTuple; return null; } - let handAtExtremity = null; // what hand? + let handAtEdge = null; // what hand? let extremityPosition = null; // where are we now? let direction = null; // where to go from here? - if ( previousAntecedentAtMin && this.ratioTupleProperty.value.antecedent === enabledRange.min ) { - handAtExtremity = ratioAndProportionStrings.a11y.leftHand; + // If we should look at the term as a possible "go beyond edge" case. + const antecedentPossibleBeyond = movedAntecedentAndPreviousAtEdge || movedEitherFromBothHandsAndPreviousAtEdge; + const consequentPossibleBeyond = movedConsequentAndPreviousAtEdge || movedEitherFromBothHandsAndPreviousAtEdge; + + if ( antecedentPossibleBeyond && previousAntecedentAtMin && + this.ratioTupleProperty.value.antecedent === enabledRange.min ) { + handAtEdge = ratioAndProportionStrings.a11y.leftHand; extremityPosition = enabledRange.min === rapConstants.TOTAL_RATIO_TERM_VALUE_RANGE.min ? ratioAndProportionStrings.a11y.handPosition.atBottom : ratioAndProportionStrings.a11y.handPosition.nearBottom; direction = ratioAndProportionStrings.a11y.up; } - else if ( previousAntecedentAtMax && this.ratioTupleProperty.value.antecedent === enabledRange.max ) { - handAtExtremity = ratioAndProportionStrings.a11y.leftHand; + else if ( antecedentPossibleBeyond && previousAntecedentAtMax && + this.ratioTupleProperty.value.antecedent === enabledRange.max ) { + handAtEdge = ratioAndProportionStrings.a11y.leftHand; extremityPosition = ratioAndProportionStrings.a11y.handPosition.atTop; direction = ratioAndProportionStrings.a11y.down; } - else if ( previousConsequentAtMin && this.ratioTupleProperty.value.consequent === enabledRange.min ) { - handAtExtremity = ratioAndProportionStrings.a11y.rightHand; + else if ( consequentPossibleBeyond && previousConsequentAtMin + && this.ratioTupleProperty.value.consequent === enabledRange.min ) { + handAtEdge = ratioAndProportionStrings.a11y.rightHand; extremityPosition = enabledRange.min === rapConstants.TOTAL_RATIO_TERM_VALUE_RANGE.min ? ratioAndProportionStrings.a11y.handPosition.atBottom : ratioAndProportionStrings.a11y.handPosition.nearBottom; direction = ratioAndProportionStrings.a11y.up; } - else if ( previousConsequentAtMax && this.ratioTupleProperty.value.consequent === enabledRange.max ) { - handAtExtremity = ratioAndProportionStrings.a11y.rightHand; + else if ( consequentPossibleBeyond && previousConsequentAtMax + && this.ratioTupleProperty.value.consequent === enabledRange.max ) { + handAtEdge = ratioAndProportionStrings.a11y.rightHand; extremityPosition = ratioAndProportionStrings.a11y.handPosition.atTop; direction = ratioAndProportionStrings.a11y.down; } @@ -472,16 +509,19 @@ class HandPositionsDescriber { this.previousEdgeCheckTuple = currentTuple; // Detect if we are at the edge of the range - if ( handAtExtremity && extremityPosition && direction ) { + if ( handAtEdge && extremityPosition && direction ) { + + // if hands move together, then the response will reflect both hands in the same direction + const handsMoveTogether = this.ratioLockedProperty.value; // Basically the difference between "Move hands" and "Move hand" - const pattern = inputModality === RatioInputModality.BOTH_HANDS ? + const pattern = handsMoveTogether ? ratioAndProportionStrings.a11y.ratio.bothHandsGoBeyondEdgeContextResponse : ratioAndProportionStrings.a11y.ratio.singleHandGoBeyondEdgeContextResponse; return StringUtils.fillIn( pattern, { position: extremityPosition, - hand: handAtExtremity, + hand: handAtEdge, direction: direction } ); } diff --git a/js/create/view/CreateScreenView.ts b/js/create/view/CreateScreenView.ts index c5f5d486..f30e9e37 100644 --- a/js/create/view/CreateScreenView.ts +++ b/js/create/view/CreateScreenView.ts @@ -55,7 +55,7 @@ class CreateScreenView extends RAPScreenView { const handPositionsDescriber = new HandPositionsDescriber( model.ratio.tupleProperty, new TickMarkDescriber( this.tickMarkRangeProperty, this.tickMarkViewProperty ), - model.inProportionProperty, model.ratio.enabledRatioTermsRangeProperty ); + model.inProportionProperty, model.ratio.enabledRatioTermsRangeProperty, model.ratio.lockedProperty ); this.createScreenSummaryNode = new CreateScreenSummaryNode( model.ratioFitnessProperty, diff --git a/js/discover/view/DiscoverScreenView.ts b/js/discover/view/DiscoverScreenView.ts index 4d7c034f..36c503e5 100644 --- a/js/discover/view/DiscoverScreenView.ts +++ b/js/discover/view/DiscoverScreenView.ts @@ -51,7 +51,7 @@ class DiscoverScreenView extends RAPScreenView { this.pdomPlayAreaNode.pdomOrder = this.pdomPlayAreaNode.pdomOrder!.concat( [ this.comboBoxContainer, comboBoxListBoxParent ] ); const handPositionsDescriber = new HandPositionsDescriber( model.ratio.tupleProperty, new TickMarkDescriber( this.tickMarkRangeProperty, this.tickMarkViewProperty ), model.inProportionProperty, - model.ratio.enabledRatioTermsRangeProperty ); + model.ratio.enabledRatioTermsRangeProperty, model.ratio.lockedProperty ); // set this after the supertype has initialized the view code needed to create the screen summary this.discoverScreenSummaryNode = new DiscoverScreenSummaryNode(