Skip to content

Commit

Permalink
Update TODOs and cleanup, see #45 and #46
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisklus committed Feb 27, 2022
1 parent be7f1e7 commit d5136e0
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 12 deletions.
1 change: 1 addition & 0 deletions js/common/CASConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const CASConstants = {
NUMBER_OF_OBJECTS_LARGE: 20, // the number of objects used on the Spread and Lab screens

// TODO: Should this be declared in main and passed through?
// TODO: This should be instrumented
PLOT_TYPE_PROPERTY: new EnumerationProperty( CASQueryParameters.plotType === 'dotPlot' ? PlotType.DOT_PLOT : PlotType.LINE_PLOT )
};

Expand Down
2 changes: 1 addition & 1 deletion js/common/view/CASAccordionBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class CASAccordionBox extends AccordionBox {
backgroundNode.addChild( checkboxPanel );

// TODO for CK: content has no height at time of instantiation, so does not end up in the correct place.
// TODO: SR says: Perhaps use x and y instead of center which requires bounds
// TODO-UX: SR says: Perhaps use x and y instead of center which requires bounds
contentNode.centerY = fullBackgroundBounds.centerY;
backgroundNode.addChild( contentNode );

Expand Down
1 change: 1 addition & 0 deletions js/common/view/CardNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class CardNode extends Node {
start: () => {
this.moveToFront();
},
// TODO-UX: This emits for dragging the leftmost card to the left
drag: ( event, listener ) => this.dragDistanceEmitter.emit( Math.abs( listener.modelDelta.x ) )
} );
this.addInputListener( this.dragListener );
Expand Down
2 changes: 0 additions & 2 deletions js/common/view/DotPlotNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
* Shows the dot plot on the "Mean & Median" Screen, including the legends/readouts to the left.
* The plot is non-interactive.
*
* TODO-UX: If the median bar shows at min=median=max=1, the dot plot shifts
*
* @author Chris Klusendorf (PhET Interactive Simulations)
* @author Sam Reid (PhET Interactive Simulations)
*/
Expand Down
11 changes: 3 additions & 8 deletions js/common/view/GlobalOptionsNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ class GlobalOptionsNode extends VBox {
// TODO: Tandem in the options?
constructor( tandem: Tandem ) {

// TODO: Use translated strings
const title = new Text( 'Plot Type', {
font: new PhetFont( 24 )
} );

const TEXT_OPTIONS = {
font: new PhetFont( 18 )
};
// TODO: Use translated strings
const radioButtonGroup = new VerticalAquaRadioButtonGroup<PlotType>( CASConstants.PLOT_TYPE_PROPERTY, [ {
node: new Text( 'Line Plot', TEXT_OPTIONS ),
value: PlotType.LINE_PLOT
Expand All @@ -44,16 +46,9 @@ class GlobalOptionsNode extends VBox {
align: 'left'
} );

// @private
this.disposeGlobalOptionsNode = () => {
radioButtonGroup.dispose();
};
this.disposeGlobalOptionsNode = () => radioButtonGroup.dispose();
}

/**
* @public
* @override
*/
dispose() {
this.disposeGlobalOptionsNode();
super.dispose();
Expand Down
1 change: 0 additions & 1 deletion js/common/view/MeanOrMedianScreenView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ class MeanOrMedianScreenView extends SoccerScreenView {
}
} );

// TODO: CK - float to top of visibleBounds to certain aspect ratio, see https://github.com/phetsims/center-and-spread/issues/50
this.accordionBox = new CASAccordionBox( this.model, this.accordionBoxContents, this.topCheckboxPanel,
titleNode,
this.layoutBounds, {
Expand Down
1 change: 1 addition & 0 deletions js/common/view/NumberCardContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class NumberCardContainer extends Node {
// Fires if the cardNodeCells may have changed
this.cardNodeCellsChangedEmitter = new Emitter<[]>();

// TODO-UX: maybe this should be converted to track distance for individual cards
// Accumulated card drag distance, for purposes of hiding the drag indicator node
this.totalDragDistanceProperty = new NumberProperty( 0 );

Expand Down

0 comments on commit d5136e0

Please sign in to comment.