From 1c862004a43432ec4ee964ffe393c094d7aaed9a Mon Sep 17 00:00:00 2001 From: Sam Reid Date: Wed, 26 Apr 2023 06:55:31 -0600 Subject: [PATCH] Move BottomRepresentationCheckboxGroup instantiation to subclasses, see https://github.com/phetsims/center-and-variability/issues/153 --- js/common/view/CAVScreenView.ts | 26 ++--------------- .../view/MeanAndMedianScreenView.ts | 29 +++++++++++++------ js/median/view/MedianScreenView.ts | 29 ++++++++++++------- js/variability/view/VariabilityScreenView.ts | 29 +++++++++++++------ 4 files changed, 62 insertions(+), 51 deletions(-) diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts index 915a7454..18fe1692 100644 --- a/js/common/view/CAVScreenView.ts +++ b/js/common/view/CAVScreenView.ts @@ -8,7 +8,7 @@ */ import ScreenView, { ScreenViewOptions } from '../../../../joist/js/ScreenView.js'; -import optionize, { combineOptions } from '../../../../phet-core/js/optionize.js'; +import optionize from '../../../../phet-core/js/optionize.js'; import centerAndVariability from '../../centerAndVariability.js'; import CAVModel from '../model/CAVModel.js'; import CAVConstants from '../CAVConstants.js'; @@ -16,17 +16,15 @@ import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.j import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js'; import PhetioGroup from '../../../../tandem/js/PhetioGroup.js'; import CAVObjectNode from './CAVObjectNode.js'; -import { AlignBox, Node } from '../../../../scenery/js/imports.js'; +import { Node } from '../../../../scenery/js/imports.js'; import CAVObjectType from '../model/CAVObjectType.js'; import CAVObject from '../model/CAVObject.js'; -import BottomRepresentationCheckboxGroup, { BottomRepresentationCheckboxGroupOptions } from './BottomRepresentationCheckboxGroup.js'; import ArrowNode from '../../../../scenery-phet/js/ArrowNode.js'; import EraserButton from '../../../../scenery-phet/js/buttons/EraserButton.js'; import PredictionSlider from './PredictionSlider.js'; import CAVColors from '../CAVColors.js'; import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import DragIndicatorArrowNode from './DragIndicatorArrowNode.js'; -import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import Property from '../../../../axon/js/Property.js'; import Range from '../../../../dot/js/Range.js'; import Tandem from '../../../../tandem/js/Tandem.js'; @@ -42,7 +40,6 @@ import KickButtonGroup from './KickButtonGroup.js'; import PlayAreaMedianIndicatorNode from './PlayAreaMedianIndicatorNode.js'; type SelfOptions = { - bottomCheckboxGroupOptions: StrictOmit; questionBarOptions: QuestionBarOptions; }; @@ -53,8 +50,6 @@ const GROUND_POSITION_Y = 500; export default class CAVScreenView extends ScreenView { - protected readonly bottomCheckboxGroup: BottomRepresentationCheckboxGroup; - protected readonly resetAllButton: ResetAllButton; protected readonly modelViewTransform: ModelViewTransform2; protected readonly model: CAVModel; @@ -80,8 +75,7 @@ export default class CAVScreenView extends ScreenView { public constructor( model: CAVModel, // TODO: Structure in options? createAccordionBox: ( tandem: Tandem, top: number, layoutBounds: Bounds2, playAreaNumberLineNode: Node ) => CAVAccordionBox, providedOptions: CAVScreenViewOptions ) { - const options = optionize, ScreenViewOptions>()( {}, providedOptions ); + const options = optionize()( {}, providedOptions ); // The ground is at y=0 const modelViewTransform = ModelViewTransform2.createRectangleInvertedYMapping( @@ -174,20 +168,6 @@ export default class CAVScreenView extends ScreenView { map.delete( cavObject ); } ); - this.bottomCheckboxGroup = new BottomRepresentationCheckboxGroup( model, - combineOptions( { - tandem: options.tandem.createTandem( 'bottomCheckboxGroup' ) - }, options.bottomCheckboxGroupOptions ) ); - - const BOTTOM_CHECKBOX_PANEL_MARGIN = 12.5; - - // In order to use the AlignBox we need to know the distance from the top of the screen, to the top of the grass. - const BOTTOM_CHECKBOX_PANEL_Y_MARGIN = this.layoutBounds.maxY - this.modelViewTransform.modelToViewY( 0 ) + BOTTOM_CHECKBOX_PANEL_MARGIN; - - - const checkboxAlignBox = new AlignBox( this.bottomCheckboxGroup, { alignBounds: this.layoutBounds, xAlign: 'right', yAlign: 'bottom', xMargin: BOTTOM_CHECKBOX_PANEL_MARGIN, yMargin: BOTTOM_CHECKBOX_PANEL_Y_MARGIN } ); - this.addChild( checkboxAlignBox ); - this.playAreaMedianIndicatorNode = new PlayAreaMedianIndicatorNode(); this.addChild( this.playAreaMedianIndicatorNode ); diff --git a/js/mean-and-median/view/MeanAndMedianScreenView.ts b/js/mean-and-median/view/MeanAndMedianScreenView.ts index f1ea832a..09f91024 100644 --- a/js/mean-and-median/view/MeanAndMedianScreenView.ts +++ b/js/mean-and-median/view/MeanAndMedianScreenView.ts @@ -12,14 +12,15 @@ import centerAndVariability from '../../centerAndVariability.js'; import MeanAndMedianModel from '../model/MeanAndMedianModel.js'; import CAVColors from '../../common/CAVColors.js'; import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; -import { ManualConstraint, Node } from '../../../../scenery/js/imports.js'; +import { AlignBox, ManualConstraint, Node } 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'; -type MeanAndMedianScreenViewOptions = StrictOmit; +type MeanAndMedianScreenViewOptions = StrictOmit; export default class MeanAndMedianScreenView extends CAVScreenView { @@ -29,13 +30,6 @@ export default class MeanAndMedianScreenView extends CAVScreenView { questionBarOptions: { barFill: CAVColors.meanAndMedianQuestionBarFillColorProperty, questionString: CenterAndVariabilityStrings.meanAndMedianQuestionStringProperty - }, - bottomCheckboxGroupOptions: { - includeMean: true, - includePredictMean: true, - includePredictMedian: true, - includeMedian: true, - includeVariability: false } }, providedOptions ); @@ -51,6 +45,23 @@ export default class MeanAndMedianScreenView extends CAVScreenView { ( playAreaNumberLineNodeWrapper, contentsWrapper ) => { contentsWrapper.x = playAreaNumberLineNodeWrapper.x; } ); + + const bottomCheckboxGroup = new BottomRepresentationCheckboxGroup( model, { + includeMean: true, + includePredictMean: true, + includePredictMedian: true, + includeMedian: true, + includeVariability: false, + tandem: options.tandem.createTandem( 'bottomCheckboxGroup' ) + } ); + + // TODO: A bit of duplication across screen views + // In order to use the AlignBox we need to know the distance from the top of the screen, to the top of the grass. + const BOTTOM_CHECKBOX_PANEL_MARGIN = 12.5; + const BOTTOM_CHECKBOX_PANEL_Y_MARGIN = this.layoutBounds.maxY - this.modelViewTransform.modelToViewY( 0 ) + BOTTOM_CHECKBOX_PANEL_MARGIN; + + const checkboxAlignBox = new AlignBox( bottomCheckboxGroup, { alignBounds: this.layoutBounds, xAlign: 'right', yAlign: 'bottom', xMargin: BOTTOM_CHECKBOX_PANEL_MARGIN, yMargin: BOTTOM_CHECKBOX_PANEL_Y_MARGIN } ); + this.addChild( checkboxAlignBox ); } } diff --git a/js/median/view/MedianScreenView.ts b/js/median/view/MedianScreenView.ts index bc8db767..d80b6219 100644 --- a/js/median/view/MedianScreenView.ts +++ b/js/median/view/MedianScreenView.ts @@ -17,11 +17,13 @@ import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreen 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 } from '../../../../scenery/js/imports.js'; type SelfOptions = EmptySelfOptions; type MedianScreenViewOptions = SelfOptions - & StrictOmit; + & StrictOmit; export default class MedianScreenView extends CAVScreenView { @@ -31,15 +33,6 @@ export default class MedianScreenView extends CAVScreenView { questionBarOptions: { barFill: CAVColors.medianQuestionBarFillColorProperty, questionString: CenterAndVariabilityStrings.medianQuestionStringProperty - }, - - // TODO: Remove this option and just create things correctly in the AccordionBox subclass - bottomCheckboxGroupOptions: { - includeMean: false, - includePredictMean: false, - includePredictMedian: true, - includeMedian: true, - includeVariability: false } }, providedOptions ); @@ -49,6 +42,22 @@ export default class MedianScreenView extends CAVScreenView { // 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 ); + + const bottomCheckboxGroup = new BottomRepresentationCheckboxGroup( model, { + includeMean: false, + includePredictMean: false, + includePredictMedian: true, + includeMedian: true, + includeVariability: false, + tandem: options.tandem.createTandem( 'bottomCheckboxGroup' ) + } ); + + // In order to use the AlignBox we need to know the distance from the top of the screen, to the top of the grass. + const BOTTOM_CHECKBOX_PANEL_MARGIN = 12.5; + const BOTTOM_CHECKBOX_PANEL_Y_MARGIN = this.layoutBounds.maxY - this.modelViewTransform.modelToViewY( 0 ) + BOTTOM_CHECKBOX_PANEL_MARGIN; + + const checkboxAlignBox = new AlignBox( bottomCheckboxGroup, { alignBounds: this.layoutBounds, xAlign: 'right', yAlign: 'bottom', xMargin: BOTTOM_CHECKBOX_PANEL_MARGIN, yMargin: BOTTOM_CHECKBOX_PANEL_Y_MARGIN } ); + this.addChild( checkboxAlignBox ); } } diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts index a1607489..5efb8830 100644 --- a/js/variability/view/VariabilityScreenView.ts +++ b/js/variability/view/VariabilityScreenView.ts @@ -14,7 +14,7 @@ import centerAndVariability from '../../centerAndVariability.js'; import VariabilityModel from '../model/VariabilityModel.js'; import CAVColors from '../../common/CAVColors.js'; import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; -import { ManualConstraint, Node } from '../../../../scenery/js/imports.js'; +import { AlignBox, ManualConstraint, Node } from '../../../../scenery/js/imports.js'; import DistributionRadioButtonGroup from './DistributionRadioButtonGroup.js'; import VariabilityMeasureRadioButtonGroup from './VariabilityMeasureRadioButtonGroup.js'; import InfoDialog from './InfoDialog.js'; @@ -22,9 +22,10 @@ import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreen 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'; type SelfOptions = EmptySelfOptions; -type VariabilityScreenViewOptions = SelfOptions & StrictOmit; +type VariabilityScreenViewOptions = SelfOptions & StrictOmit; export default class VariabilityScreenView extends CAVScreenView { @@ -34,13 +35,6 @@ export default class VariabilityScreenView extends CAVScreenView { questionBarOptions: { barFill: CAVColors.variabilityQuestionBarFillColorProperty, questionString: CenterAndVariabilityStrings.variabilityQuestionStringProperty - }, - bottomCheckboxGroupOptions: { - includeVariability: true, - includePredictMean: false, - includePredictMedian: false, - includeMean: true, - includeMedian: true } }, providedOptions ); @@ -95,6 +89,23 @@ export default class VariabilityScreenView extends CAVScreenView { infoDialog.hide(); } } ); + + const bottomCheckboxGroup = new BottomRepresentationCheckboxGroup( model, { + includeVariability: true, + includePredictMean: false, + includePredictMedian: false, + includeMean: true, + includeMedian: true, + tandem: options.tandem.createTandem( 'bottomCheckboxGroup' ) + } ); + + // TODO: A bit of duplication across screen views + // In order to use the AlignBox we need to know the distance from the top of the screen, to the top of the grass. + const BOTTOM_CHECKBOX_PANEL_MARGIN = 12.5; + const BOTTOM_CHECKBOX_PANEL_Y_MARGIN = this.layoutBounds.maxY - this.modelViewTransform.modelToViewY( 0 ) + BOTTOM_CHECKBOX_PANEL_MARGIN; + + const checkboxAlignBox = new AlignBox( bottomCheckboxGroup, { alignBounds: this.layoutBounds, xAlign: 'right', yAlign: 'bottom', xMargin: BOTTOM_CHECKBOX_PANEL_MARGIN, yMargin: BOTTOM_CHECKBOX_PANEL_Y_MARGIN } ); + this.addChild( checkboxAlignBox ); } }