diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts index 18fe1692..652526b2 100644 --- a/js/common/view/CAVScreenView.ts +++ b/js/common/view/CAVScreenView.ts @@ -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'; @@ -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; @@ -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()( {}, providedOptions ); // The ground is at y=0 @@ -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; @@ -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: { @@ -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, @@ -318,9 +316,6 @@ 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 @@ -328,12 +323,8 @@ export default class CAVScreenView extends ScreenView { 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; } /** @@ -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(); } } diff --git a/js/mean-and-median/view/MeanAndMedianScreenView.ts b/js/mean-and-median/view/MeanAndMedianScreenView.ts index 00efc827..894825ab 100644 --- a/js/mean-and-median/view/MeanAndMedianScreenView.ts +++ b/js/mean-and-median/view/MeanAndMedianScreenView.ts @@ -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; @@ -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; } ); diff --git a/js/median/view/MedianScreenView.ts b/js/median/view/MedianScreenView.ts index e268136c..b4fedc5b 100644 --- a/js/median/view/MedianScreenView.ts +++ b/js/median/view/MedianScreenView.ts @@ -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 = @@ -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( [ diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts index 6dd6a3f4..2e011db5 100644 --- a/js/variability/view/VariabilityScreenView.ts +++ b/js/variability/view/VariabilityScreenView.ts @@ -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; @@ -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 @@ -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; } );