Skip to content

Commit

Permalink
Both hands context responses to respect DistanceResponseType, add inP…
Browse files Browse the repository at this point in the history 
…roportionOverridesDistanceProgress and so that both hands still get distance progress, #459 #432
  • Loading branch information
@zepumph
zepumph committed Apr 14, 2022
1 parent b62fa42 commit a545c57
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 30 deletions.
6 changes: 3 additions & 3 deletions js/common/view/RatioHalf.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import RatioHandNode from './RatioHandNode.js';
import ViewSounds from './sound/ViewSounds.js';
import TickMarkView from './TickMarkView.js';
import BothHandsDescriber from './describers/BothHandsDescriber.js';
import HandPositionsDescriber, { SingleHandContextResponseOptions } from './describers/HandPositionsDescriber.js';
import HandPositionsDescriber, { HandContextResponseOptions } from './describers/HandPositionsDescriber.js';
import RAPRatioTuple from '../model/RAPRatioTuple.js';
import CueArrowsState from './CueArrowsState.js';
import RatioDescriber from './describers/RatioDescriber.js';
Expand Down Expand Up @@ -496,11 +496,11 @@ class RatioHalf extends Rectangle {
*/
getSingleHandContextResponse( handPositionsDescriber: HandPositionsDescriber,
bothHandsDescriber: BothHandsDescriber,
options?: SingleHandContextResponseOptions ): string {
options?: HandContextResponseOptions ): string {

// When locked, give a description of both-hands, instead of just a single one.
if ( this.ratio.lockedProperty.value ) {
return bothHandsDescriber.getBothHandsContextResponse();
return bothHandsDescriber.getBothHandsContextResponse( options );
}

return handPositionsDescriber.getSingleHandContextResponse( this.ratioTerm, this.tickMarkViewProperty.value, options );
Expand Down
11 changes: 7 additions & 4 deletions js/common/view/describers/BothHandsDescriber.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import Property from '../../../../../axon/js/Property.js';
import RAPRatioTuple from '../../model/RAPRatioTuple.js';
import Range from '../../../../../dot/js/Range.js';
import RatioDescriber from './RatioDescriber.js';
import HandPositionsDescriber from './HandPositionsDescriber.js';
import HandPositionsDescriber, { HandContextResponseOptions } from './HandPositionsDescriber.js';
import TickMarkView from '../TickMarkView.js';
import EnumerationProperty from '../../../../../axon/js/EnumerationProperty.js';
import IReadOnlyProperty from '../../../../../axon/js/IReadOnlyProperty.js';
import TickMarkDescriber from './TickMarkDescriber.js';
import DistanceResponseType from './DistanceResponseType.js';

const ratioDistancePositionContextResponsePatternString = ratioAndProportionStrings.a11y.ratio.distancePositionContextResponse;

Expand Down Expand Up @@ -50,7 +51,7 @@ class BothHandsDescriber {
this.previousConsequentAtExtremity = false;
}

getBothHandsContextResponse(): string {
getBothHandsContextResponse( options?: HandContextResponseOptions ): string {

// only applicable if the ratio is locked
const ratioLockedEdgeResponse = this.getRatioLockedEdgeCaseContextResponse();
Expand All @@ -59,7 +60,7 @@ class BothHandsDescriber {
}

return StringUtils.fillIn( ratioDistancePositionContextResponsePatternString, {
distance: this.handPositionsDescriber.getBothHandsDistance( true, true ),
distance: this.handPositionsDescriber.getBothHandsDistance( true, options ),
position: this.getBothHandsPosition()
} );
}
Expand All @@ -69,7 +70,9 @@ class BothHandsDescriber {
*/
getBothHandsDynamicDescription(): string {
return StringUtils.fillIn( ratioDistancePositionContextResponsePatternString, {
distance: this.handPositionsDescriber.getBothHandsDistance( false, true ),
distance: this.handPositionsDescriber.getBothHandsDistance( true, {
distanceResponseType: DistanceResponseType.DISTANCE_REGION
} ),
position: this.getBothHandsPosition()
} );
}
Expand Down
95 changes: 72 additions & 23 deletions js/common/view/describers/HandPositionsDescriber.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
* Description for the positions of each hand, as well as their positional relationship like the distance between each
* hand, and if they have gotten closer to or farther from each other ("distance progress").
*
* In general, responses are split into implementaiton based on the DistanceResponseType. Whether it be distance
* region (qualitative regions), distance progress (closer/farther), or a combo algorithm to default to distance region,
* but you distance progress to prevent repeating the same region many times.
*
* `getSingleHandContextResponse` and `getBothHandsDistance` use a similar algorithm to accomplish these variations, but
* could not be factored out completely due to natural language requirements.
*
* @author Michael Kauzmann (PhET Interactive Simulations)
*/

Expand Down Expand Up @@ -100,6 +107,9 @@ const POSITION_REGIONS_DATA: PositionRegionsData[] = [
];

type GetDistanceProgressStringOptions = {

// set to false if it is desired to get distanceProgress even when in proportion
inProportionOverridesDistanceProgress?: boolean;
closerString?: string;
fartherString?: string;
};
Expand Down Expand Up @@ -282,7 +292,6 @@ class HandPositionsDescriber {
* distance regions.
*/
getSingleHandComboDistance( ratioTerm: RatioTerm ): string {
const otherHand = ratioTerm === RatioTerm.ANTECEDENT ? rightHandLowerString : leftHandLowerString;

const distanceRegion = this.getDistanceRegion( false );

Expand All @@ -305,46 +314,86 @@ class HandPositionsDescriber {
return distanceRegion;
}

getBothHandsDistance( overrideWithDistanceProgress = false, capitalized = false ): string {
const distanceRegion = this.getDistanceRegion( true );
// TODO: capitalized is currently always used, but it would be nice to improve the implementation for voicing context responses, https://github.com/phetsims/ratio-and-proportion/issues/461
getBothHandsDistance( capitalized: boolean, providedOptions?: HandContextResponseOptions ): string {
const options = optionize<HandContextResponseOptions>( {

if ( overrideWithDistanceProgress ) {
if ( distanceRegion === this.previousDistanceRegionBoth ) {
assert && assert( capitalized, 'overriding with distance-progress not supported for capitalized strings' );
// By default, let the describer decide if we should have distance progress or region
distanceResponseType: DistanceResponseType.COMBO
}, providedOptions );

const distanceProgressPhrase = this.getDistanceProgressString( {
closerString: ratioAndProportionStrings.a11y.handPosition.closerTogether,
fartherString: ratioAndProportionStrings.a11y.handPosition.fartherApart
} );
if ( distanceProgressPhrase ) {
const distanceProgressDescription = StringUtils.fillIn( ratioAndProportionStrings.a11y.bothHands.handsDistanceProgressPattern, {
distanceProgress: distanceProgressPhrase
} );

// Count closer/farther as a previous so that we don't ever get two of them at the same time
this.previousDistanceRegionBoth = distanceProgressDescription;
return distanceProgressDescription;
}
}
this.previousDistanceRegionBoth = distanceRegion;
switch( options.distanceResponseType ) {
case DistanceResponseType.COMBO:
return this.getBothHandsComboDistance( capitalized );
case DistanceResponseType.DISTANCE_PROGRESS:
return this.getBothHandsDistanceProgress( capitalized );
case DistanceResponseType.DISTANCE_REGION:
return this.getBothHandsDistanceRegion( capitalized );
default:
assert && assert( false, 'This is not how enums work' );
}
assert && assert( false, 'We should always have a distance case above' );
return 'A serious logic error occurred';
}

getBothHandsDistanceProgress( capitalized: boolean ): string {
const distanceProgressPhrase = this.getDistanceProgressString( {
inProportionOverridesDistanceProgress: false,
closerString: ratioAndProportionStrings.a11y.handPosition.closerTogether,
fartherString: ratioAndProportionStrings.a11y.handPosition.fartherApart
} );
if ( distanceProgressPhrase ) {
return StringUtils.fillIn( ratioAndProportionStrings.a11y.bothHands.handsDistanceProgressPattern, {
distanceProgress: distanceProgressPhrase
} );
}
return this.getBothHandsDistanceRegion( capitalized );
}

getBothHandsDistanceRegion( capitalized: boolean ): string {
const distanceRegion = this.getDistanceRegion( true );

const pattern = capitalized ? ratioAndProportionStrings.a11y.bothHands.handsDistancePatternCapitalized :
ratioAndProportionStrings.a11y.bothHands.handsDistancePattern;

return StringUtils.fillIn( pattern, { distance: distanceRegion } );
}

getBothHandsComboDistance( capitalized = false ): string {
const distanceRegion = this.getDistanceRegion( true );

if ( distanceRegion === this.previousDistanceRegionBoth ) {
assert && assert( capitalized, 'overriding with distance-progress not supported for capitalized strings' );

const distanceProgressPhrase = this.getDistanceProgressString( {
inProportionOverridesDistanceProgress: false,
closerString: ratioAndProportionStrings.a11y.handPosition.closerTogether,
fartherString: ratioAndProportionStrings.a11y.handPosition.fartherApart
} );
if ( distanceProgressPhrase ) {
const distanceProgressDescription = StringUtils.fillIn( ratioAndProportionStrings.a11y.bothHands.handsDistanceProgressPattern, {
distanceProgress: distanceProgressPhrase
} );

// Count closer/farther as a previous so that we don't ever get two of them at the same time
this.previousDistanceRegionBoth = distanceProgressDescription;
return distanceProgressDescription;
}
}
this.previousDistanceRegionBoth = distanceRegion;
return this.getBothHandsDistanceRegion( capitalized );
}

private getDistanceProgressString( providedOptions?: GetDistanceProgressStringOptions ): null | string {

const options = optionize<GetDistanceProgressStringOptions, GetDistanceProgressStringOptions>( {
inProportionOverridesDistanceProgress: true,
closerString: ratioAndProportionStrings.a11y.handPosition.closerTo,
fartherString: ratioAndProportionStrings.a11y.handPosition.fartherFrom
}, providedOptions );


// No distance progress if in proportion TODO: this shouldn't occur for both hands in description, not clear if the same in voicing. https://github.com/phetsims/ratio-and-proportion/issues/459
if ( this.inProportionProperty.value ) {
if ( options.inProportionOverridesDistanceProgress && this.inProportionProperty.value ) {
return null;
}

Expand Down

0 comments on commit a545c57

Please sign in to comment.