Skip to content

Commit

Permalink
Create accordion box in subtypes, see #153
Browse files Browse the repository at this point in the history
  • Loading branch information
@samreid
samreid committed Apr 26, 2023
1 parent 03d318a commit 610abb5
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 46 deletions.
51 changes: 26 additions & 25 deletions js/common/view/CAVScreenView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
import DragIndicatorArrowNode from './DragIndicatorArrowNode.js';
import Property from '../../../../axon/js/Property.js';
import Range from '../../../../dot/js/Range.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import QuestionBar, { QuestionBarOptions } from '../../../../scenery-phet/js/QuestionBar.js';
import CAVAccordionBox from './CAVAccordionBox.js';
import NumberLineNode from './NumberLineNode.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import BackgroundNode from './BackgroundNode.js';
Expand All @@ -38,6 +36,7 @@ import SoccerPlayer from '../model/SoccerPlayer.js';
import merge from '../../../../phet-core/js/merge.js';
import KickButtonGroup from './KickButtonGroup.js';
import PlayAreaMedianIndicatorNode from './PlayAreaMedianIndicatorNode.js';
import CAVAccordionBox from './CAVAccordionBox.js';

type SelfOptions = {
questionBarOptions: QuestionBarOptions;
Expand Down Expand Up @@ -67,14 +66,13 @@ export default class CAVScreenView extends ScreenView {
protected readonly medianPredictionNode: PredictionSlider;
protected readonly meanPredictionNode: PredictionSlider;

protected readonly accordionBox: CAVAccordionBox;
protected accordionBox: CAVAccordionBox | null = null;

protected readonly questionBar: QuestionBar;
protected readonly playAreaNumberLineNode: NumberLineNode;
private readonly updateMedianNode: () => void;

public constructor( model: CAVModel,
// TODO: Structure in options?
createAccordionBox: ( tandem: Tandem, top: number, layoutBounds: Bounds2, playAreaNumberLineNode: Node ) => CAVAccordionBox, providedOptions: CAVScreenViewOptions ) {
public constructor( model: CAVModel, providedOptions: CAVScreenViewOptions ) {
const options = optionize<CAVScreenViewOptions, SelfOptions, ScreenViewOptions>()( {}, providedOptions );

// The ground is at y=0
Expand Down Expand Up @@ -171,7 +169,7 @@ export default class CAVScreenView extends ScreenView {
this.playAreaMedianIndicatorNode = new PlayAreaMedianIndicatorNode();
this.addChild( this.playAreaMedianIndicatorNode );

const updateMedianNode = () => {
this.updateMedianNode = () => {
const medianValue = model.medianValueProperty.value;
const visible = medianValue !== null && model.isShowingPlayAreaMedianProperty.value;

Expand All @@ -187,16 +185,18 @@ export default class CAVScreenView extends ScreenView {
this.playAreaMedianIndicatorNode.bottom = this.modelViewTransform.modelToViewY( 0 ) + viewHeight;

// The arrow shouldn't overlap the accordion box
const accordionBoxHeight = this.accordionBox.expandedProperty.value ? this.accordionBox.getExpandedBoxHeight() : this.accordionBox.getCollapsedBoxHeight();
if ( this.playAreaMedianIndicatorNode.top < this.accordionBox.top + accordionBoxHeight ) {
this.playAreaMedianIndicatorNode.top = this.accordionBox.top + accordionBoxHeight + 4;
if ( this.accordionBox ) {
const accordionBoxHeight = this.accordionBox.expandedProperty.value ? this.accordionBox.getExpandedBoxHeight() : this.accordionBox.getCollapsedBoxHeight();
if ( this.playAreaMedianIndicatorNode.top < this.accordionBox.top + accordionBoxHeight ) {
this.playAreaMedianIndicatorNode.top = this.accordionBox.top + accordionBoxHeight + 4;
}
}
}
this.playAreaMedianIndicatorNode.visible = visible;
};
model.medianValueProperty.link( updateMedianNode );
model.objectChangedEmitter.addListener( updateMedianNode );
model.isShowingPlayAreaMedianProperty.link( updateMedianNode );
model.medianValueProperty.link( this.updateMedianNode );
model.objectChangedEmitter.addListener( this.updateMedianNode );
model.isShowingPlayAreaMedianProperty.link( this.updateMedianNode );

this.medianPredictionNode = new PredictionSlider( model.medianPredictionProperty, this.modelViewTransform, model.physicalRange, {
predictionThumbNodeOptions: {
Expand Down Expand Up @@ -304,8 +304,6 @@ export default class CAVScreenView extends ScreenView {
}, options.questionBarOptions ) );
this.contentLayer.addChild( this.questionBar );

this.accordionBox = createAccordionBox( options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.layoutBounds, this.playAreaNumberLineNode );

this.contentLayer.addChild( new KickButtonGroup( model, {
left: 25,

Expand All @@ -318,22 +316,15 @@ export default class CAVScreenView extends ScreenView {
// Soccer balls go behind the accordion box after they land
this.contentLayer.addChild( this.backObjectLayer );

this.contentLayer.addChild( this.accordionBox );
this.accordionBox.expandedProperty.link( updateMedianNode );

// Add in the same order as the checkboxes, so the z-order matches the checkbox order

// TODO: meanPredictionNode should only exist in MeanAndMedianScreenView
this.contentLayer.addChild( this.meanPredictionNode );
this.contentLayer.addChild( this.medianPredictionNode );
}

/**
* Steps the view.
* @param dt - time step, in seconds
*/
public override step( dt: number ): void {
// See subclass for implementation
private updateAccordionBoxPosition(): void {
this.accordionBox!.top = this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN;
}

/**
Expand All @@ -347,7 +338,17 @@ export default class CAVScreenView extends ScreenView {
} );
this.visibleBoundsProperty.value = this.parentToLocalBounds( viewBounds );

this.accordionBox.top = this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN;
this.accordionBox && this.updateAccordionBoxPosition();
}

/**
* Called by subtype constructors to finish initialization.
*/
protected setAccordionBox( accordionBox: CAVAccordionBox ): void {
this.accordionBox = accordionBox;
this.contentLayer.addChild( this.accordionBox );
this.accordionBox.expandedProperty.link( this.updateMedianNode );
this.updateAccordionBoxPosition();
}
}

Expand Down
12 changes: 6 additions & 6 deletions js/mean-and-median/view/MeanAndMedianScreenView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import centerAndVariability from '../../centerAndVariability.js';
import MeanAndMedianModel from '../model/MeanAndMedianModel.js';
import CAVColors from '../../common/CAVColors.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import { AlignBox, AlignGroup, ManualConstraint, Node } from '../../../../scenery/js/imports.js';
import { AlignBox, AlignGroup, ManualConstraint } from '../../../../scenery/js/imports.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreenView.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import MeanAndMedianAccordionBox from './MeanAndMedianAccordionBox.js';
import BottomRepresentationCheckboxGroup from '../../common/view/BottomRepresentationCheckboxGroup.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
import CAVConstants from '../../common/CAVConstants.js';

type MeanAndMedianScreenViewOptions = StrictOmit<CAVScreenViewOptions, 'questionBarOptions'>;

Expand All @@ -34,15 +33,16 @@ export default class MeanAndMedianScreenView extends CAVScreenView {
}
}, providedOptions );

super( model, ( tandem: Tandem, top: number, layoutBounds: Bounds2, playAreaNumberLineNode: Node ) =>
new MeanAndMedianAccordionBox( model, layoutBounds, tandem, top, playAreaNumberLineNode ), options );
super( model, options );

this.setAccordionBox( new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode ) );

// NOTE: This assumes that the NumberLineNode in the play area and in the dot plot have the same characteristics:
// * Same font
// * Same offset and scale
// But given those assumptions, this code moves the dot plot so that its number line matches the play area one.
// TODO: Consider something more robust. Using globalToLocal to exactly align based on the position of the tick marks
ManualConstraint.create( this, [ this.playAreaNumberLineNode, this.accordionBox.contentNode ],
ManualConstraint.create( this, [ this.playAreaNumberLineNode, this.accordionBox!.contentNode ],
( playAreaNumberLineNodeWrapper, contentsWrapper ) => {
contentsWrapper.x = playAreaNumberLineNodeWrapper.x;
} );
Expand Down
10 changes: 3 additions & 7 deletions js/median/view/MedianScreenView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ import CAVColors from '../../common/CAVColors.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreenView.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import MedianAccordionBox from './MedianAccordionBox.js';
import BottomRepresentationCheckboxGroup from '../../common/view/BottomRepresentationCheckboxGroup.js';
import { AlignBox, AlignGroup } from '../../../../scenery/js/imports.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
import CAVConstants from '../../common/CAVConstants.js';

type SelfOptions = EmptySelfOptions;
type MedianScreenViewOptions =
Expand All @@ -37,12 +36,9 @@ export default class MedianScreenView extends CAVScreenView {
}
}, providedOptions );

// TODO: Arg order looks scrambled, should it be unified or optionized?
super( model,
super( model, options );

// TODO: Can we just add this after super()? Why does it have to be here?
( tandem: Tandem, top: number, layoutBounds: Bounds2 ) =>
new MedianAccordionBox( model, layoutBounds, tandem, top ), options );
this.setAccordionBox( new MedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN ) );

const iconGroup = new AlignGroup();
const bottomCheckboxGroup = new VerticalCheckboxGroup( [
Expand Down
15 changes: 7 additions & 8 deletions js/variability/view/VariabilityScreenView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@ import centerAndVariability from '../../centerAndVariability.js';
import VariabilityModel from '../model/VariabilityModel.js';
import CAVColors from '../../common/CAVColors.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import { AlignBox, AlignGroup, ManualConstraint, Node } from '../../../../scenery/js/imports.js';
import { AlignBox, AlignGroup, ManualConstraint } from '../../../../scenery/js/imports.js';
import DistributionRadioButtonGroup from './DistributionRadioButtonGroup.js';
import VariabilityMeasureRadioButtonGroup from './VariabilityMeasureRadioButtonGroup.js';
import InfoDialog from './InfoDialog.js';
import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreenView.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import VariabilityAccordionBox from './VariabilityAccordionBox.js';
import BottomRepresentationCheckboxGroup from '../../common/view/BottomRepresentationCheckboxGroup.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
import CAVConstants from '../../common/CAVConstants.js';

type SelfOptions = EmptySelfOptions;
type VariabilityScreenViewOptions = SelfOptions & StrictOmit<CAVScreenViewOptions, 'questionBarOptions'>;
Expand All @@ -44,9 +43,9 @@ export default class VariabilityScreenView extends CAVScreenView {
tandem: options.tandem.createTandem( 'variabilityMeasureRadioButtonGroup' )
} );

super( model, ( tandem: Tandem, top: number, layoutBounds: Bounds2, playAreaNumberLineNode: Node ) => {
return new VariabilityAccordionBox( model, layoutBounds, tandem, top );
}, options );
super( model, options );

this.setAccordionBox( new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN ) );

// NOTE: This assumes that the NumberLineNode in the play area and in the dot plot have the same characteristics:
// * Same font
Expand All @@ -55,12 +54,12 @@ export default class VariabilityScreenView extends CAVScreenView {
// TODO: Consider something more robust. Using globalToLocal to exactly align based on the position of the tick marks
// TODO: Can this be combine in a parent class? See https://github.com/phetsims/center-and-variability/issues/152
// TODO: Maybe if the accordion box was a subclass we could do this?
ManualConstraint.create( this, [ this.playAreaNumberLineNode, this.accordionBox.contentNode ],
ManualConstraint.create( this, [ this.playAreaNumberLineNode, this.accordionBox!.contentNode ],
( lowerNumberLineWrapper, contentsWrapper ) => {
contentsWrapper.x = lowerNumberLineWrapper.x;
} );

ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBox ],
ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBox! ],
( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => {
variabilityRadioButtonGroupWrapper.centerY = accordionBoxWrapper.centerY;
} );
Expand Down

0 comments on commit 610abb5

Please sign in to comment.