Skip to content

Commit

Permalink
remove TODOs related to typescript, #404
Browse files Browse the repository at this point in the history
  • Loading branch information
zepumph committed Mar 23, 2022
1 parent f525961 commit a726893
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 65 deletions.
23 changes: 13 additions & 10 deletions js/common/model/RAPRatio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,38 @@ const LOCK_RATIO_RANGE_MIN = rapConstants.NO_SUCCESS_VALUE_THRESHOLD + Number.EP

class RAPRatio {

enabledRatioTermsRangeProperty: Property<Range>;
// Keep two references so that this can be public readonly, AND changed internally.
readonly enabledRatioTermsRangeProperty: IReadOnlyProperty<Range>;
private readonly _enabledRatioTermsRangeProperty: Property<Range>;

// Central Property that holds the value of the ratio. Using a tuple that holds
// both the antecedent and consequent values as a single data structure is vital for changing both hands at once, and
// in supporting the "locked ratio" state. Otherwise there are complicated reentrant cases where changing the
// antecedent cascades to the consequent to snap it back into ratio. Thus the creation of RAPRatioTuple.
tupleProperty: Property<RAPRatioTuple>;
readonly tupleProperty: Property<RAPRatioTuple>;

// when true, moving one ratio value will maintain the current ratio by updating the other value Property
lockedProperty: BooleanProperty;
private antecedentVelocityTracker: VelocityTracker;
private consequentVelocityTracker: VelocityTracker;
readonly lockedProperty: BooleanProperty;
private readonly antecedentVelocityTracker: VelocityTracker;
private readonly consequentVelocityTracker: VelocityTracker;

// if the ratio is in the "moving in direction" state: whether or not the two hands are moving fast
// enough together in the same direction. This indicates, among other things a bimodal interaction.
movingInDirectionProperty: IReadOnlyProperty<boolean>;
readonly movingInDirectionProperty: IReadOnlyProperty<boolean>;

// To avoid an infinite loop as setting the tupleProperty from inside its lock-ratio-support
// listener. This is predominately needed because even same antecedent/consequent values get wrapped in a new
// RAPRatioTuple instance.
private ratioLockListenerEnabled: boolean;


constructor( initialAntecedent: number, initialConsequent: number, tandem: Tandem ) {

// TODO: public readonly, https://github.com/phetsims/ratio-and-proportion/issues/404
this.enabledRatioTermsRangeProperty = new Property( DEFAULT_TERM_VALUE_RANGE, {
this._enabledRatioTermsRangeProperty = new Property( DEFAULT_TERM_VALUE_RANGE, {
tandem: tandem.createTandem( 'enabledRatioTermsRangeProperty' ),
phetioType: Property.PropertyIO( Range.RangeIO )
} );
this.enabledRatioTermsRangeProperty = this._enabledRatioTermsRangeProperty;

this.tupleProperty = new Property( new RAPRatioTuple( initialAntecedent, initialConsequent ), {
valueType: RAPRatioTuple,
Expand Down Expand Up @@ -140,7 +143,7 @@ class RAPRatio {
} );

this.lockedProperty.link( ratioLocked => {
this.enabledRatioTermsRangeProperty.value = new Range( ratioLocked ? LOCK_RATIO_RANGE_MIN : DEFAULT_TERM_VALUE_RANGE.min, DEFAULT_TERM_VALUE_RANGE.max );
this._enabledRatioTermsRangeProperty.value = new Range( ratioLocked ? LOCK_RATIO_RANGE_MIN : DEFAULT_TERM_VALUE_RANGE.min, DEFAULT_TERM_VALUE_RANGE.max );
} );

this.enabledRatioTermsRangeProperty.link( enabledRange => {
Expand Down Expand Up @@ -218,7 +221,7 @@ class RAPRatio {

this.tupleProperty.reset();

this.enabledRatioTermsRangeProperty.reset();
this._enabledRatioTermsRangeProperty.reset();
this.antecedentVelocityTracker.reset();
this.consequentVelocityTracker.reset();
this.ratioLockListenerEnabled = true;
Expand Down
18 changes: 9 additions & 9 deletions js/common/view/BothHandsPDOMNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,21 @@ class BothHandsPDOMNode extends Node {
private bothHandsDescriber: BothHandsDescriber;

// To support proper cue arrow logic
private antecedentInteractedWithProperty: Property<boolean>;
private consequentInteractedWithProperty: Property<boolean>;
private bothHandsFocusedProperty: Property<boolean>;
private readonly antecedentInteractedWithProperty: Property<boolean>;
private readonly consequentInteractedWithProperty: Property<boolean>;
private readonly bothHandsFocusedProperty: Property<boolean>;

private viewSounds: ViewSounds;
private objectResponseUtterance: Utterance;
private readonly viewSounds: ViewSounds;
private readonly objectResponseUtterance: Utterance;

// just to fire on focus, make this polite for https://github.com/phetsims/ratio-and-proportion/issues/347
private objectResponseOnFocusUtterance: Utterance;
private contextResponseUtterance: Utterance;
private ratioUnlockedFromBothHandsUtterance: Utterance;
private readonly objectResponseOnFocusUtterance: Utterance;
private readonly contextResponseUtterance: Utterance;
private readonly ratioUnlockedFromBothHandsUtterance: Utterance;

// TODO: wish I know how to mark it as a @public (read-only) (but set internal to this file).https://github.com/phetsims/ratio-and-proportion/issues/404
isBeingInteractedWithProperty: Property<boolean>;
private bothHandsInteractionListener: BothHandsInteractionListener;
private readonly bothHandsInteractionListener: BothHandsInteractionListener;

constructor( providedOptions: BothHandsPDOMNodeOptions ) {

Expand Down
81 changes: 44 additions & 37 deletions js/common/view/RatioHalf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,23 +55,14 @@ const getModelBoundsFromRange = ( range: Range ) => new Bounds2( -1 * X_MODEL_DR
const MIN_HAND_SCALE = 1.2;
const MAX_HAND_SCALE = 2.5;

function ratioHalfAccessibleNameBehavior( node: Node, options: NodeOptions, accessibleName: string, callbacksForOtherNodes: { (): void }[] ) {
assert && assert( node instanceof RatioHalf );

callbacksForOtherNodes.push( () => {
( node as RatioHalf ).ratioHandNode.accessibleName = accessibleName;
} );
return options;
}

const TOTAL_RANGE = rapConstants.TOTAL_RATIO_TERM_VALUE_RANGE;

type LayoutFunction = ( bounds: Bounds2, heightScalar: number ) => void;

type SelfOptions = {
ratioTerm: RatioTerm;
ratioTupleProperty: Property<RAPRatioTuple>;
enabledRatioTermsRangeProperty: Property<Range>; // the current range that the hand can move
enabledRatioTermsRangeProperty: IReadOnlyProperty<Range>; // the current range that the hand can move
displayBothHandsCueProperty: Property<boolean>;
cueArrowsState: CueArrowsState; // interaction state to determine the interaction cue to display
bounds: Bounds2; // the initial bounds that the Node takes up
Expand Down Expand Up @@ -113,24 +104,24 @@ type RatioHalfOptions = SelfOptions & RectangleOptions;
class RatioHalf extends Rectangle {

// the height of the framing rectangles, updated in layout function
// TODO: should be @public (read-only) if possible (but set internally) https://github.com/phetsims/ratio-and-proportion/issues/404
framingRectangleHeight: number;
_framingRectangleHeight: number;

// TODO: should be @public (read-only) if possible https://github.com/phetsims/ratio-and-proportion/issues/404
// this behaves a bit differently depending on modality. For mouse/touch, any time you are
// This behaves a bit differently depending on modality. For mouse/touch, any time you are
// dragging this will be considered interaction, for keyboard, you must press a key before the interaction starts.
readonly isBeingInteractedWithProperty: BooleanProperty;
private ratioLockedProperty: Property<boolean>;
private bothHandsDescriber: BothHandsDescriber;
private handPositionsDescriber: HandPositionsDescriber;
private voicingHandPositionsDescriber: HandPositionsDescriber;
private tickMarkViewProperty: EnumerationProperty<TickMarkView>;
private ratioTerm: RatioTerm;
private ratioTupleProperty: Property<RAPRatioTuple>;
// Note both members to keep a public readonly interface.
readonly isBeingInteractedWithProperty: IReadOnlyProperty<boolean>;
private readonly _isBeingInteractedWithProperty: BooleanProperty;

private readonly ratioLockedProperty: Property<boolean>;
private readonly bothHandsDescriber: BothHandsDescriber;
private readonly handPositionsDescriber: HandPositionsDescriber;
private readonly voicingHandPositionsDescriber: HandPositionsDescriber;
private readonly tickMarkViewProperty: EnumerationProperty<TickMarkView>;
private readonly ratioTerm: RatioTerm;
private readonly ratioTupleProperty: Property<RAPRatioTuple>;

// The draggable element inside the Node framed with thick rectangles on the top and bottom.
// TODO: should be "private to the file" if possible https://github.com/phetsims/ratio-and-proportion/issues/404
ratioHandNode: RatioHandNode;
private ratioHandNode: RatioHandNode;
private layoutRatioHalf: LayoutFunction;
private resetRatioHalf: () => void;

Expand All @@ -151,16 +142,19 @@ class RatioHalf extends Rectangle {

// pdom
tagName: 'div',
accessibleNameBehavior: ratioHalfAccessibleNameBehavior,
accessibleNameBehavior: RatioHalf.RATIO_HALF_ACCESSIBLE_NAME_BEHAVIOR,
accessibleName: null
}, providedOptions );

super( 0, 0, options.bounds.width, options.bounds.height );

this.framingRectangleHeight = MIN_FRAMING_RECTANGLE_HEIGHT;
this.isBeingInteractedWithProperty = new BooleanProperty( false, {
this._framingRectangleHeight = MIN_FRAMING_RECTANGLE_HEIGHT;

// Tandem is a different name to keep a `public readonly` interface without depending on getters and setters.
this._isBeingInteractedWithProperty = new BooleanProperty( false, {
tandem: options.tandem.createTandem( 'isBeingInteractedWithProperty' )
} );
this.isBeingInteractedWithProperty = this._isBeingInteractedWithProperty;

this.ratioLockedProperty = options.ratioLockedProperty;
this.bothHandsDescriber = options.bothHandsDescriber;
Expand Down Expand Up @@ -217,7 +211,7 @@ class RatioHalf extends Rectangle {
options.inProportionProperty, {
startDrag: () => {
options.cueArrowsState.interactedWithKeyboardProperty.value = true;
this.isBeingInteractedWithProperty.value = true;
this._isBeingInteractedWithProperty.value = true;
this.viewSounds.boundarySoundClip.onStartInteraction();
},
drag: () => {
Expand Down Expand Up @@ -311,7 +305,7 @@ class RatioHalf extends Rectangle {
} );
},
drag: () => {
this.isBeingInteractedWithProperty.value = true;
this._isBeingInteractedWithProperty.value = true;

if ( typeof startingX === 'number' ) {
positionProperty.value.setX( startingX );
Expand Down Expand Up @@ -345,7 +339,7 @@ class RatioHalf extends Rectangle {

startingX = null;
this.viewSounds.releaseSoundClip.play();
this.isBeingInteractedWithProperty.value = false;
this._isBeingInteractedWithProperty.value = false;
options.setJumpingOverProportionShouldTriggerSound( false );
this.viewSounds.boundarySoundClip.onEndInteraction( positionProperty.value.y );

Expand Down Expand Up @@ -377,7 +371,7 @@ class RatioHalf extends Rectangle {
const newBounds = getModelBoundsFromRange( enabledRange );

// offset the bounds to account for the ratioHandNode's size, since the center of the ratioHandNode is controlled by the drag bounds.
const modelHalfPointerPointer = modelViewTransform.viewToModelDeltaXY( this.ratioHandNode.width / 2, -this.framingRectangleHeight );
const modelHalfPointerPointer = modelViewTransform.viewToModelDeltaXY( this.ratioHandNode.width / 2, -this._framingRectangleHeight );

// constrain x dimension inside the RatioHalf so that this.ratioHandNode doesn't go beyond the width. Height is constrained
// via the modelViewTransform.
Expand All @@ -393,7 +387,7 @@ class RatioHalf extends Rectangle {
blur: () => {
options.cueArrowsState.keyboardFocusedProperty.value = false;
this.viewSounds.releaseSoundClip.play();
this.isBeingInteractedWithProperty.value = false;
this._isBeingInteractedWithProperty.value = false;
},
down: () => {

Expand All @@ -403,11 +397,11 @@ class RatioHalf extends Rectangle {
} );

// "Framing" rectangles on the top and bottom of the drag area of the ratio half
const topRect = new Rectangle( 0, 0, 10, this.framingRectangleHeight, { fill: options.colorProperty } );
const bottomRect = new Rectangle( 0, 0, 10, this.framingRectangleHeight, { fill: options.colorProperty } );
const topRect = new Rectangle( 0, 0, 10, this._framingRectangleHeight, { fill: options.colorProperty } );
const bottomRect = new Rectangle( 0, 0, 10, this._framingRectangleHeight, { fill: options.colorProperty } );

const tickMarksNode = new RatioHalfTickMarksNode( options.tickMarkViewProperty, options.tickMarkRangeProperty,
options.bounds.width, options.bounds.height - 2 * this.framingRectangleHeight,
options.bounds.width, options.bounds.height - 2 * this._framingRectangleHeight,
options.colorProperty );

const updatePointer = ( position: Vector2 ) => {
Expand All @@ -429,7 +423,7 @@ class RatioHalf extends Rectangle {
this.rectWidth = newBounds.width;
this.rectHeight = newBounds.height;

this.framingRectangleHeight = topRect.rectHeight = bottomRect.rectHeight = heightScalar * MIN_FRAMING_RECTANGLE_HEIGHT + ( MAX_FRAMING_RECTANGLE_HEIGHT - MIN_FRAMING_RECTANGLE_HEIGHT );
this._framingRectangleHeight = topRect.rectHeight = bottomRect.rectHeight = heightScalar * MIN_FRAMING_RECTANGLE_HEIGHT + ( MAX_FRAMING_RECTANGLE_HEIGHT - MIN_FRAMING_RECTANGLE_HEIGHT );

// Scale depending on how tall the ratio half is. This is to support narrow and tall layouts where the hand needs
// to be scaled up more to support touch interaction, see https://github.com/phetsims/ratio-and-proportion/issues/217.
Expand All @@ -443,7 +437,7 @@ class RatioHalf extends Rectangle {
topRect.top = 0;
bottomRect.bottom = newBounds.height;

const boundsNoFramingRects = newBounds.erodedY( this.framingRectangleHeight );
const boundsNoFramingRects = newBounds.erodedY( this._framingRectangleHeight );

// Don't count the space the framing rectangles take up as part of the draggableArea.
modelViewTransform = ModelViewTransform2.createRectangleInvertedYMapping(
Expand All @@ -468,6 +462,11 @@ class RatioHalf extends Rectangle {
};
}

// Provide a getter but not a setter to denote "public readonly"
get framingRectangleHeight(): number {
return this._framingRectangleHeight;
}

/**
* Generate and send an alert to the UtteranceQueue that describes the movement of this object and the subsequent change
* in ratio. This is the context response for the individual ratio half hand (slider) interaction. Returning
Expand Down Expand Up @@ -506,6 +505,14 @@ class RatioHalf extends Rectangle {
reset(): void {
this.resetRatioHalf();
}

private static RATIO_HALF_ACCESSIBLE_NAME_BEHAVIOR( node: Node, options: NodeOptions, accessibleName: string, callbacksForOtherNodes: { (): void }[] ) {

callbacksForOtherNodes.push( () => {
( node as RatioHalf ).ratioHandNode.accessibleName = accessibleName;
} );
return options;
}
}

ratioAndProportion.register( 'RatioHalf', RatioHalf );
Expand Down
2 changes: 1 addition & 1 deletion js/common/view/RatioHandNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class RatioHandNode extends AccessibleSlider( Node, 0 ) {
private resetRatioHandNode: () => void;

constructor( valueProperty: Property<number>,
enabledRatioTermsRangeProperty: Property<Range>,
enabledRatioTermsRangeProperty: IReadOnlyProperty<Range>,
tickMarkViewProperty: EnumerationProperty<TickMarkView>,
keyboardStep: number,
colorProperty: IPaint,
Expand Down
10 changes: 4 additions & 6 deletions js/common/view/TickMarkViewRadioButtonGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,24 @@
* @author Michael Kauzmann (PhET Interactive Simulations)
*/

import merge from '../../../../phet-core/js/merge.js';
import { ParallelDOM, Path, PathOptions } from '../../../../scenery/js/imports.js';
import eyeSlashSolidShape from '../../../../sherpa/js/fontawesome-5/eyeSlashSolidShape.js';
import RectangularRadioButtonGroup from '../../../../sun/js/buttons/RectangularRadioButtonGroup.js';
import ActivationUtterance from '../../../../utterance-queue/js/ActivationUtterance.js';
import ratioAndProportion from '../../ratioAndProportion.js';
import ratioAndProportionStrings from '../../ratioAndProportionStrings.js';
import TickMarkView from './TickMarkView.js';
import EnumerationProperty from '../../../../axon/js/EnumerationProperty.js';
import optionize from '../../../../phet-core/js/optionize.js';
import RectangularRadioButtonGroup, { RectangularRadioButtonGroupOptions } from '../../../../sun/js/buttons/RectangularRadioButtonGroup.js';

// constants
const ICON_SCALE = 0.45;

class TickMarkViewRadioButtonGroup extends RectangularRadioButtonGroup<TickMarkView> {

constructor( tickMarkViewProperty: EnumerationProperty<TickMarkView>, options?: any ) {
constructor( tickMarkViewProperty: EnumerationProperty<TickMarkView>, providedOptions?: RectangularRadioButtonGroupOptions ) {

// TODO: convert to optionize once RectangularRadioButtonGroup is typescript https://github.com/phetsims/ratio-and-proportion/issues/404
options = merge( {
const options = optionize<RectangularRadioButtonGroupOptions, {}>( {
orientation: 'horizontal',
baseColor: 'white',
buttonContentYMargin: 14,
Expand All @@ -34,7 +32,7 @@ class TickMarkViewRadioButtonGroup extends RectangularRadioButtonGroup<TickMarkV
// pdom
labelContent: ratioAndProportionStrings.a11y.tickMark.heading,
helpTextBehavior: ParallelDOM.HELP_TEXT_BEFORE_CONTENT
}, options );
}, providedOptions );

const radioButtonItemData = [ {
node: new Path( eyeSlashSolidShape, { scale: 0.05, fill: 'black' } ),
Expand Down
5 changes: 3 additions & 2 deletions js/common/view/describers/BothHandsDescriber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ import RatioDescriber from './RatioDescriber.js';
import HandPositionsDescriber from './HandPositionsDescriber.js';
import TickMarkView from '../TickMarkView.js';
import EnumerationProperty from '../../../../../axon/js/EnumerationProperty.js';
import IReadOnlyProperty from '../../../../../axon/js/IReadOnlyProperty.js';

const ratioDistancePositionContextResponsePatternString = ratioAndProportionStrings.a11y.ratio.distancePositionContextResponse;

class BothHandsDescriber {

private ratioTupleProperty: Property<RAPRatioTuple>;
private enabledRatioTermsRangeProperty: Property<Range>;
private enabledRatioTermsRangeProperty: IReadOnlyProperty<Range>;
private tickMarkViewProperty: EnumerationProperty<TickMarkView>;
private ratioDescriber: RatioDescriber;
private handPositionsDescriber: HandPositionsDescriber;
Expand All @@ -33,7 +34,7 @@ class BothHandsDescriber {
private previousAntecedentAtExtremity: boolean;
private previousConsequentAtExtremity: boolean;

constructor( ratioTupleProperty: Property<RAPRatioTuple>, enabledRatioTermsRangeProperty: Property<Range>,
constructor( ratioTupleProperty: Property<RAPRatioTuple>, enabledRatioTermsRangeProperty: IReadOnlyProperty<Range>,
ratioLockedProperty: Property<boolean>, tickMarkViewProperty: EnumerationProperty<TickMarkView>,
ratioDescriber: RatioDescriber, handPositionsDescriber: HandPositionsDescriber ) {

Expand Down

0 comments on commit a726893

Please sign in to comment.