Skip to content

Commit

Permalink
REVIEW comments, #32
Browse files Browse the repository at this point in the history
  • Loading branch information
pixelzoom committed Mar 4, 2024
1 parent 7c8143c commit c0697f1
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 6 deletions.
2 changes: 2 additions & 0 deletions js/common-vsm/model/VSMModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export type VSMModelOptions<T extends VSMField> = SelfOptions & StrictOmit<PDLMo

export default class VSMModel<T extends VSMField> extends PDLModel<T> {

//REVIEW Declarations of BooleanProperty should be Property<boolean>

// Static tool visibility
public readonly isLaunchAngleVisibleProperty: BooleanProperty;
public readonly isLaunchSpeedVisibleProperty: BooleanProperty;
Expand Down
2 changes: 1 addition & 1 deletion js/common-vsm/view/InteractiveToolPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export type InteractiveToolPanelOptions = SelfOptions & WithRequired<PDLPanelOpt
export default class InteractiveToolPanel extends PDLPanel {

public constructor(
isMeasuringTapeVisibleProperty: BooleanProperty,
isMeasuringTapeVisibleProperty: BooleanProperty, //REVIEW should be Property<boolean>
isStopwatchVisibleProperty: PhetioProperty<boolean>,
providedOptions: InteractiveToolPanelOptions ) {

Expand Down
2 changes: 1 addition & 1 deletion js/common/model/Field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export default abstract class Field extends PhetioObject {
public readonly launchHeightProperty: TReadOnlyProperty<number>;

// True if the continuous mode is set to launching or false if not launching
public readonly isContinuousLaunchingProperty: BooleanProperty;
public readonly isContinuousLaunchingProperty: BooleanProperty; //REVIEW should be Property<boolean>

// An array of projectiles that are currently in the air. Note: No projectile should be in the airborneProjectiles and landedProjectiles array simultaneously.
public readonly airborneProjectiles: Projectile[] = [];
Expand Down
2 changes: 1 addition & 1 deletion js/common/model/PDLEventTimer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default class PDLEventTimer {
private readonly period: number;

// Whether the timer is running
private readonly isRunningProperty: BooleanProperty;
private readonly isRunningProperty: BooleanProperty; //REVIEW should be Property<boolean>

// Remaining time until the next event, in seconds
private readonly timeRemainingProperty: NumberProperty;
Expand Down
2 changes: 1 addition & 1 deletion js/common/model/PDLModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default abstract class PDLModel<T extends Field> implements TModel {
public readonly singleOrContinuousProperty: Property<SingleOrContinuous>;

// Setting for whether the user wants to see the paths of the projectiles
public readonly isPathsVisibleProperty: BooleanProperty;
public readonly isPathsVisibleProperty: BooleanProperty; //REVIEW should be Property<boolean>

// Abstract Property that indicates the selected Launcher
public readonly abstract launcherProperty: TReadOnlyProperty<Launcher>;
Expand Down
2 changes: 2 additions & 0 deletions js/measures/model/MeasuresModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export default class MeasuresModel extends SMModel<MeasuresField> {
// Whether the launcher is custom or mystery
public readonly mysteryOrCustomProperty: DynamicProperty<MysteryOrCustom, MysteryOrCustom, MeasuresField>;

//REVIEW Declarations of BooleanProperty should be Property<boolean>

// Whether the mean is visible on the field and histogram
public readonly isMeanVisibleProperty: BooleanProperty;

Expand Down
1 change: 1 addition & 0 deletions js/measures/view/DataMeasuresOverlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export default class DataMeasuresOverlay extends Node {
public constructor( modelViewTransform: ModelViewTransform2 | ChartTransform,
meanDistanceProperty: PhetioProperty<number | null>,
standardDeviationDistanceProperty: PhetioProperty<number | null>,
//REVIEW next 3 params of type BooleanProperty should be TReadOnlyProperty<boolean>
isMeanDisplayedProperty: BooleanProperty,
isStandardDeviationDisplayedProperty: BooleanProperty,
isValuesDisplayedProperty: BooleanProperty,
Expand Down
1 change: 1 addition & 0 deletions js/measures/view/MeasuresHistogramNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export default class MeasuresHistogramNode extends VSMHistogramNode {
numberOfLandedProjectilesProperty: TReadOnlyProperty<number>,
histogram: Histogram,
horizontalAxisLabelText: TReadOnlyProperty<string>,
//REVIEW next 3 params of type BooleanProperty should be TReadOnlyProperty<boolean>
isMeanVisibleProperty: BooleanProperty,
isStandardDeviationVisibleProperty: BooleanProperty,
isValuesVisibleProperty: BooleanProperty,
Expand Down
4 changes: 2 additions & 2 deletions js/measures/view/MeasuresInteractiveToolPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ type MeasuresInteractiveToolPanelOptions = SelfOptions & InteractiveToolPanelOpt

export default class MeasuresInteractiveToolPanel extends InteractiveToolPanel {
public constructor(
isMeasuringTapeVisibleProperty: BooleanProperty,
isMeasuringTapeVisibleProperty: BooleanProperty, //REVIEW should be Property<boolean>
isStopwatchVisibleProperty: PhetioProperty<boolean>,
isIntervalToolVisibleProperty: BooleanProperty,
isIntervalToolVisibleProperty: BooleanProperty, //REVIEW should be Property<boolean>
providedOptions: MeasuresInteractiveToolPanelOptions ) {

class IntervalToolIcon extends Node {
Expand Down

1 comment on commit c0697f1

@pixelzoom
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.