From d0baf2daf57d8cd29c43e510f917f94a1d1e4a54 Mon Sep 17 00:00:00 2001 From: Sam Reid Date: Tue, 25 Apr 2023 16:33:29 -0600 Subject: [PATCH] Move CAVAccordionBox.ts instantiation to CAVScreenView subclasses, see https://github.com/phetsims/center-and-variability/issues/146 --- js/common/CAVConstants.ts | 8 +- js/common/view/CAVAccordionBox.ts | 3 + js/common/view/CAVScreenView.ts | 93 ++------------ js/mean-and-median/MeanAndMedianScreen.ts | 24 +--- .../view/MeanAndMedianScreenView.ts | 70 +++++++++-- js/median/view/MedianScreenView.ts | 51 +++++--- js/variability/view/VariabilityScreenView.ts | 118 +++++++++++------- 7 files changed, 192 insertions(+), 175 deletions(-) diff --git a/js/common/CAVConstants.ts b/js/common/CAVConstants.ts index 64ac3d18..85d86e30 100644 --- a/js/common/CAVConstants.ts +++ b/js/common/CAVConstants.ts @@ -13,6 +13,7 @@ import Tandem from '../../../tandem/js/Tandem.js'; import centerAndVariability from '../centerAndVariability.js'; import CAVQueryParameters from './CAVQueryParameters.js'; import PlotType from './model/PlotType.js'; +import ScreenView from '../../../joist/js/ScreenView.js'; // Right skewed means most of the data is on the left, see https://github.com/phetsims/center-and-variability/issues/112 const RIGHT_SKEWED_DATA = [ @@ -21,6 +22,8 @@ const RIGHT_SKEWED_DATA = [ 5, 5, 5, 5, 5 ]; +const NUMBER_LINE_MARGIN_X = 207; + const CAVConstants = { SCREEN_VIEW_X_MARGIN: 15, SCREEN_VIEW_Y_MARGIN: 15, @@ -37,7 +40,10 @@ const CAVConstants = { } ), ARROW_LINE_WIDTH: 0.5, RIGHT_SKEWED_DATA: RIGHT_SKEWED_DATA, - LEFT_SKEWED_DATA: RIGHT_SKEWED_DATA.slice().reverse() + LEFT_SKEWED_DATA: RIGHT_SKEWED_DATA.slice().reverse(), + + CHART_VIEW_WIDTH: ScreenView.DEFAULT_LAYOUT_BOUNDS.width - NUMBER_LINE_MARGIN_X * 2, + NUMBER_LINE_MARGIN_X: NUMBER_LINE_MARGIN_X }; centerAndVariability.register( 'CAVConstants', CAVConstants ); diff --git a/js/common/view/CAVAccordionBox.ts b/js/common/view/CAVAccordionBox.ts index b33a5dce..33c14877 100644 --- a/js/common/view/CAVAccordionBox.ts +++ b/js/common/view/CAVAccordionBox.ts @@ -84,6 +84,9 @@ export default class CAVAccordionBox extends AccordionBox { // TODO: we are mutating the position of things being passed in checkboxPanel.centerY = fullBackgroundBounds.centerY; + + // TODO: Maybe every subclass doesn't want a checkboxPanel + checkboxPanel.right = backgroundNode.right - CONTENT_MARGIN; backgroundNode.addChild( checkboxPanel ); // TODO: SR says: Perhaps use x and y instead of center in order to vertically center the content, then diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts index 170e1da5..f937dfa5 100644 --- a/js/common/view/CAVScreenView.ts +++ b/js/common/view/CAVScreenView.ts @@ -16,7 +16,7 @@ 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, ManualConstraint, Node, Text } from '../../../../scenery/js/imports.js'; +import { AlignBox, Node } from '../../../../scenery/js/imports.js'; import CAVObjectType from '../model/CAVObjectType.js'; import CAVObject from '../model/CAVObject.js'; import BottomRepresentationCheckboxGroup, { BottomRepresentationCheckboxGroupOptions } from './BottomRepresentationCheckboxGroup.js'; @@ -31,7 +31,6 @@ 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 TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import CAVAccordionBox from './CAVAccordionBox.js'; import NumberLineNode from './NumberLineNode.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; @@ -40,17 +39,8 @@ import SoccerPlayerNode from './SoccerPlayerNode.js'; import SoccerPlayer from '../model/SoccerPlayer.js'; import merge from '../../../../phet-core/js/merge.js'; import KickButtonGroup from './KickButtonGroup.js'; -import CardNodeContainer from './CardNodeContainer.js'; -import MedianModel from '../../median/model/MedianModel.js'; -import VariabilityPlotNode from '../../variability/view/VariabilityPlotNode.js'; -import VariabilityModel from '../../variability/model/VariabilityModel.js'; -import CAVPlotNodeWithMedianBar from '../../mean-and-median/view/CAVPlotNodeWithMedianBar.js'; -import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; -import VariabilityReadoutsNode from '../../variability/view/VariabilityReadoutsNode.js'; -import ValueReadoutsNode from './ValueReadoutsNode.js'; type SelfOptions = { - createAccordionBoxControlNode: ( tandem: Tandem ) => Node; bottomCheckboxGroupOptions?: StrictOmit; questionBarOptions: QuestionBarOptions; @@ -58,15 +48,12 @@ type SelfOptions = { // TODO: If we are sticking with this pattern, switch to screen: 'median' | 'meanAndMedian' | 'variability' etc, see https://github.com/phetsims/center-and-variability/issues/153 isMedianScreen: boolean; isVariabilityScreen: boolean; - accordionBoxTitleStringProperty: TReadOnlyProperty; - }; export type CAVScreenViewOptions = SelfOptions & ScreenViewOptions; // constants const GROUND_POSITION_Y = 500; -const NUMBER_LINE_MARGIN_X = 207; export default class CAVScreenView extends ScreenView { @@ -88,25 +75,23 @@ export default class CAVScreenView extends ScreenView { protected readonly medianPredictionNode: PredictionSlider; protected readonly meanPredictionNode: PredictionSlider; - protected readonly accordionBoxControlNode: Node; private readonly accordionBox: CAVAccordionBox; - protected readonly accordionBoxContents: Node; protected readonly questionBar: QuestionBar; - protected readonly chartViewWidth: number; protected readonly playAreaNumberLineNode: NumberLineNode; - public constructor( model: CAVModel, providedOptions: CAVScreenViewOptions ) { + 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 chartViewWidth = ScreenView.DEFAULT_LAYOUT_BOUNDS.width - NUMBER_LINE_MARGIN_X * 2; - // The ground is at y=0 const modelViewTransform = ModelViewTransform2.createRectangleInvertedYMapping( new Bounds2( model.physicalRange.min, 0, model.physicalRange.max, model.physicalRange.getLength() ), - new Bounds2( NUMBER_LINE_MARGIN_X, GROUND_POSITION_Y - chartViewWidth, NUMBER_LINE_MARGIN_X + chartViewWidth, GROUND_POSITION_Y ) + new Bounds2( CAVConstants.NUMBER_LINE_MARGIN_X, GROUND_POSITION_Y - CAVConstants.CHART_VIEW_WIDTH, CAVConstants.NUMBER_LINE_MARGIN_X + CAVConstants.CHART_VIEW_WIDTH, GROUND_POSITION_Y ) ); super( options ); @@ -192,8 +177,6 @@ export default class CAVScreenView extends ScreenView { map.delete( cavObject ); } ); - this.accordionBoxControlNode = options.createAccordionBoxControlNode( options.tandem.createTandem( 'accordionBoxControl' ) ); - this.bottomCheckboxGroup = new BottomRepresentationCheckboxGroup( model, combineOptions( { tandem: options.tandem.createTandem( 'bottomCheckboxGroup' ) @@ -300,20 +283,20 @@ export default class CAVScreenView extends ScreenView { this.addChild( this.eraseButton ); this.addChild( this.resetAllButton ); - this.chartViewWidth = chartViewWidth; - this.contentLayer.addChild( new BackgroundNode( GROUND_POSITION_Y, this.visibleBoundsProperty ) ); this.playAreaNumberLineNode = new NumberLineNode( model.physicalRange, - chartViewWidth, + + // TODO: Should not be parameter if constant + CAVConstants.CHART_VIEW_WIDTH, model.meanValueProperty, model.isShowingPlayAreaMeanProperty, model.dataRangeProperty, { includeXAxis: false, includeMeanStroke: true, tandem: options.tandem.createTandem( 'playAreaNumberLineNode' ), - x: NUMBER_LINE_MARGIN_X, + x: CAVConstants.NUMBER_LINE_MARGIN_X, y: GROUND_POSITION_Y } ); this.contentLayer.addChild( this.playAreaNumberLineNode ); @@ -353,6 +336,9 @@ export default class CAVScreenView extends ScreenView { tandem: options.tandem.createTandem( 'questionBar' ) }, 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, @@ -365,62 +351,9 @@ export default class CAVScreenView extends ScreenView { // Soccer balls go behind the accordion box after they land this.contentLayer.addChild( this.backObjectLayer ); - - const accordionBoxTandem = options.tandem.createTandem( 'accordionBox' ); - - // TODO: Better logic for this, or better ordering - if ( options.isMedianScreen ) { - this.accordionBoxContents = new CardNodeContainer( this.model as MedianModel, { - - // Expose this intermediate layer to make it so that clients can hide the number cards with one call - tandem: accordionBoxTandem.createTandem( 'cardNodeContainer' ) - } ); - } - else if ( options.isVariabilityScreen ) { - this.accordionBoxContents = new VariabilityPlotNode( this.model as VariabilityModel, this.chartViewWidth, { - tandem: accordionBoxTandem.createTandem( 'plotNode' ) - } ); - } - else { - this.accordionBoxContents = new CAVPlotNodeWithMedianBar( this.model, this.chartViewWidth, { - tandem: accordionBoxTandem.createTandem( 'plotNode' ) - } ); - } - - const titleNode = new Text( options.accordionBoxTitleStringProperty, { - font: new PhetFont( 16 ), - maxWidth: 300 - } ); - - this.accordionBox = new CAVAccordionBox( this.model, this.accordionBoxContents, this.accordionBoxControlNode, - titleNode, - this.layoutBounds, { - leftMargin: options.isVariabilityScreen ? 70 : 0, - tandem: accordionBoxTandem, - contentNodeOffsetY: options.isMedianScreen ? -6 : 0, - top: this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, - - // TODO: Better pattern for this - valueReadoutsNode: model instanceof VariabilityModel ? new VariabilityReadoutsNode( model ) : - options.isMedianScreen ? null : - new ValueReadoutsNode( model ), - - ...( options.isVariabilityScreen ? { - right: this.layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN - } : { - centerX: this.layoutBounds.centerX - } ), - infoShowingProperty: this.model instanceof VariabilityModel ? this.model.isInfoShowingProperty : null - } ); this.contentLayer.addChild( this.accordionBox ); this.accordionBox.expandedProperty.link( updateMedianNode ); - // TODO: What if positioning the bottomCheckboxGroup.right forces the topCheckboxGroup to the right of the accordion box bounds? - ManualConstraint.create( this, [ this.bottomCheckboxGroup, this.accordionBoxControlNode ], - ( bottomCheckboxGroupWrapper, accordionBoxControlNodeWrapper ) => { - accordionBoxControlNodeWrapper.x = bottomCheckboxGroupWrapper.x; - } ); - // Add in the same order as the checkboxes, so the z-order matches the checkbox order if ( !options.isMedianScreen ) { this.contentLayer.addChild( this.meanPredictionNode ); diff --git a/js/mean-and-median/MeanAndMedianScreen.ts b/js/mean-and-median/MeanAndMedianScreen.ts index f525d5df..36b6e7aa 100644 --- a/js/mean-and-median/MeanAndMedianScreen.ts +++ b/js/mean-and-median/MeanAndMedianScreen.ts @@ -17,11 +17,6 @@ import MeanAndMedianScreenView from './view/MeanAndMedianScreenView.js'; import CenterAndVariabilityStrings from '../CenterAndVariabilityStrings.js'; import ScreenIcon from '../../../joist/js/ScreenIcon.js'; import meanAndMedianScreenIcon_png from '../../images/meanAndMedianScreenIcon_png.js'; -import DynamicProperty from '../../../axon/js/DynamicProperty.js'; -import CAVConstants from '../common/CAVConstants.js'; -import DerivedProperty from '../../../axon/js/DerivedProperty.js'; -import PlotType from '../common/model/PlotType.js'; -import TopRepresentationCheckboxGroup from '../common/view/TopRepresentationCheckboxGroup.js'; type MeanAndMedianScreenOptions = CAVScreenOptions; @@ -38,13 +33,6 @@ export default class MeanAndMedianScreen extends CAVScreen { - return plotType === PlotType.LINE_PLOT ? CenterAndVariabilityStrings.linePlotStringProperty : - CenterAndVariabilityStrings.dotPlotStringProperty; - } ); - - const accordionBoxTitleProperty = new DynamicProperty( currentProperty ); - super( () => new MeanAndMedianModel( { tandem: options.tandem.createTandem( 'model' ), @@ -53,17 +41,7 @@ export default class MeanAndMedianScreen extends CAVScreen new MeanAndMedianScreenView( model, { isVariabilityScreen: false, - tandem: options.tandem.createTandem( 'view' ), - accordionBoxTitleStringProperty: accordionBoxTitleProperty, - createAccordionBoxControlNode: tandem => new TopRepresentationCheckboxGroup( model, { - medianBarIconOptions: { - notchDirection: 'down', - barStyle: 'continuous', - arrowScale: 0.75 - }, - showMedianCheckboxIcon: true, - tandem: tandem.createTandem( 'topRepresentationCheckboxGroup' ) - } ) + tandem: options.tandem.createTandem( 'view' ) } ), options ); diff --git a/js/mean-and-median/view/MeanAndMedianScreenView.ts b/js/mean-and-median/view/MeanAndMedianScreenView.ts index ebaf1c2b..9d605cc3 100644 --- a/js/mean-and-median/view/MeanAndMedianScreenView.ts +++ b/js/mean-and-median/view/MeanAndMedianScreenView.ts @@ -12,9 +12,17 @@ import centerAndVariability from '../../centerAndVariability.js'; import MeanAndMedianModel from '../model/MeanAndMedianModel.js'; import CAVColors from '../../common/CAVColors.js'; import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; -import { ManualConstraint } from '../../../../scenery/js/imports.js'; +import { ManualConstraint, Text, 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 CAVPlotNodeWithMedianBar from './CAVPlotNodeWithMedianBar.js'; +import CAVAccordionBox from '../../common/view/CAVAccordionBox.js'; +import CAVConstants from '../../common/CAVConstants.js'; +import ValueReadoutsNode from '../../common/view/ValueReadoutsNode.js'; +import Tandem from '../../../../tandem/js/Tandem.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; +import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js'; type MeanAndMedianScreenViewOptions = StrictOmit; @@ -30,17 +38,55 @@ export default class MeanAndMedianScreenView extends CAVScreenView { } }, providedOptions ); - super( model, options ); - - // 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.accordionBoxContents ], - ( lowerNumberLineWrapper, contentsWrapper ) => { - contentsWrapper.x = lowerNumberLineWrapper.x; - } ); + // TODO: Improve this pattern? + let afterInit: ( () => void ) | null = null; + + super( model, ( tandem: Tandem, top: number, layoutBounds: Bounds2, playAreaNumberLineNode: Node ) => { + + const accordionBoxContents = new CAVPlotNodeWithMedianBar( model, + + // TODO: it's a constant, should not be a parameter + CAVConstants.CHART_VIEW_WIDTH, { + tandem: tandem.createTandem( 'plotNode' ) + } ); + + afterInit = () => { + // 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, [ playAreaNumberLineNode, accordionBoxContents ], + ( lowerNumberLineWrapper, contentsWrapper ) => { + contentsWrapper.x = lowerNumberLineWrapper.x; + } ); + }; + + return new CAVAccordionBox( model, accordionBoxContents, new TopRepresentationCheckboxGroup( model, { + medianBarIconOptions: { + notchDirection: 'down', + barStyle: 'continuous', + arrowScale: 0.75 + }, + showMedianCheckboxIcon: true, + tandem: tandem.createTandem( 'topRepresentationCheckboxGroup' ) + } ), + new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, { + font: new PhetFont( 16 ), + maxWidth: 300 + } ), + layoutBounds, { + leftMargin: 0, + tandem: tandem, + contentNodeOffsetY: 0, + top: top, + valueReadoutsNode: new ValueReadoutsNode( model ), + centerX: layoutBounds.centerX, + infoShowingProperty: null + } ); + }, options ); + + afterInit!(); } } diff --git a/js/median/view/MedianScreenView.ts b/js/median/view/MedianScreenView.ts index eaf3e07e..17a84430 100644 --- a/js/median/view/MedianScreenView.ts +++ b/js/median/view/MedianScreenView.ts @@ -15,11 +15,17 @@ import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js'; import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreenView.js'; +import CAVAccordionBox from '../../common/view/CAVAccordionBox.js'; +import { Text } from '../../../../scenery/js/imports.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; +import CardNodeContainer from '../../common/view/CardNodeContainer.js'; +import Bounds2 from '../../../../dot/js/Bounds2.js'; +import Tandem from '../../../../tandem/js/Tandem.js'; type SelfOptions = EmptySelfOptions; type MedianScreenViewOptions = SelfOptions - & StrictOmit; + & StrictOmit; export default class MedianScreenView extends CAVScreenView { @@ -32,24 +38,41 @@ export default class MedianScreenView extends CAVScreenView { barFill: CAVColors.medianQuestionBarFillColorProperty, questionString: CenterAndVariabilityStrings.medianQuestionStringProperty }, - createAccordionBoxControlNode: tandem => new TopRepresentationCheckboxGroup( model, { - includeSortData: true, - includeMean: false, - medianBarIconOptions: { - notchDirection: 'up', - barStyle: 'continuous' - }, - showMedianCheckboxIcon: false, - tandem: tandem.createTandem( 'checkboxGroup' ) - } ), bottomCheckboxGroupOptions: { includeMean: false, includePredictMean: false - }, - accordionBoxTitleStringProperty: CenterAndVariabilityStrings.distanceInMetersStringProperty + } }, providedOptions ); - super( model, options ); + super( model, ( tandem: Tandem, top: number, layoutBounds: Bounds2 ) => { + return new CAVAccordionBox( model, new CardNodeContainer( model, { + + // Expose this intermediate layer to make it so that clients can hide the number cards with one call + tandem: tandem.createTandem( 'cardNodeContainer' ) + } ), new TopRepresentationCheckboxGroup( model, { + includeSortData: true, + includeMean: false, + medianBarIconOptions: { + notchDirection: 'up', + barStyle: 'continuous' + }, + showMedianCheckboxIcon: false, + tandem: tandem.createTandem( 'checkboxGroup' ) + } ), + new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, { + font: new PhetFont( 16 ), + maxWidth: 300 + } ), + layoutBounds, { + leftMargin: 0, + tandem: tandem, + contentNodeOffsetY: -6, + top: top, + valueReadoutsNode: null, + centerX: layoutBounds.centerX, + infoShowingProperty: null + } ); + }, options ); } } diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts index 5309da25..8a60ec6e 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, Text } from '../../../../scenery/js/imports.js'; +import { ManualConstraint, Text, Node } from '../../../../scenery/js/imports.js'; import DistributionRadioButtonGroup from './DistributionRadioButtonGroup.js'; import VariabilityMeasureRadioButtonGroup from './VariabilityMeasureRadioButtonGroup.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; @@ -25,6 +25,12 @@ import Checkbox from '../../../../sun/js/Checkbox.js'; import CAVConstants from '../../common/CAVConstants.js'; import InfoDialog from './InfoDialog.js'; import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreenView.js'; +import VariabilityPlotNode from './VariabilityPlotNode.js'; +import Bounds2 from '../../../../dot/js/Bounds2.js'; +import CAVAccordionBox from '../../common/view/CAVAccordionBox.js'; +import VariabilityReadoutsNode from './VariabilityReadoutsNode.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; +import Tandem from '../../../../tandem/js/Tandem.js'; // TODO: Copied from somewhere. What's the best pattern? const TEXT_OPTIONS = { @@ -33,7 +39,7 @@ const TEXT_OPTIONS = { }; type SelfOptions = EmptySelfOptions; -type VariabilityScreenViewOptions = SelfOptions & StrictOmit; +type VariabilityScreenViewOptions = SelfOptions & StrictOmit; export default class VariabilityScreenView extends CAVScreenView { @@ -54,46 +60,78 @@ export default class VariabilityScreenView extends CAVScreenView { barFill: CAVColors.variabilityQuestionBarFillColorProperty, questionString: CenterAndVariabilityStrings.variabilityQuestionStringProperty }, - createAccordionBoxControlNode: tandem => new ToggleNode( model.selectedVariabilityProperty, [ { - value: VariabilityMeasure.RANGE, - - // TODO: Different string value? For now, use the same string for the accordion box title and checkbox, and a different one for the value equals pattern - createNode: tandem => new Checkbox( model.isShowingRangeProperty, new Text( CenterAndVariabilityStrings.rangeStringProperty, TEXT_OPTIONS ), { - tandem: tandem.createTandem( 'rangeCheckbox' ) - } ) - }, { - value: VariabilityMeasure.IQR, - createNode: tandem => new Checkbox( model.isShowingIQRProperty, new Text( CenterAndVariabilityStrings.iqrStringProperty, TEXT_OPTIONS ), { - tandem: tandem.createTandem( 'iqrCheckbox' ) - } ) - }, { - value: VariabilityMeasure.MAD, - createNode: tandem => new Checkbox( model.isShowingMADProperty, new Text( CenterAndVariabilityStrings.madStringProperty, TEXT_OPTIONS ), { - tandem: tandem.createTandem( 'madCheckbox' ) - } ) - } - ] ), bottomCheckboxGroupOptions: { includeVariability: true, includePredictMean: false, includePredictMedian: false - }, - accordionBoxTitleStringProperty: accordionBoxTitleProperty + } }, providedOptions ); - super( model, options ); - - // 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 - // TODO: Can this be combine in a parent class? See https://github.com/phetsims/center-and-variability/issues/152 - ManualConstraint.create( this, [ this.playAreaNumberLineNode, this.accordionBoxContents ], - ( lowerNumberLineWrapper, contentsWrapper ) => { - contentsWrapper.x = lowerNumberLineWrapper.x; + let afterInit: ( () => void ) | null = null; + + const variabilityMeasureRadioButtonGroup = new VariabilityMeasureRadioButtonGroup( model.selectedVariabilityProperty, { + left: 10, + tandem: options.tandem.createTandem( 'variabilityMeasureRadioButtonGroup' ) + } ); + + super( model, ( tandem: Tandem, top: number, layoutBounds: Bounds2, playAreaNumberLineNode: Node ) => { + const accordionBoxContents = new VariabilityPlotNode( model, CAVConstants.CHART_VIEW_WIDTH, { + tandem: tandem.createTandem( 'plotNode' ) } ); + afterInit = () => { // 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 + // TODO: Can this be combine in a parent class? See https://github.com/phetsims/center-and-variability/issues/152 + ManualConstraint.create( this, [ playAreaNumberLineNode, accordionBoxContents ], + ( lowerNumberLineWrapper, contentsWrapper ) => { + contentsWrapper.x = lowerNumberLineWrapper.x; + } ); + + ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, accordionBoxContents ], + ( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => { + variabilityRadioButtonGroupWrapper.top = accordionBoxWrapper.top; + } ); + }; + return new CAVAccordionBox( model, accordionBoxContents, new ToggleNode( model.selectedVariabilityProperty, [ { + value: VariabilityMeasure.RANGE, + + // TODO: Different string value? For now, use the same string for the accordion box title and checkbox, and a different one for the value equals pattern + createNode: tandem => new Checkbox( model.isShowingRangeProperty, new Text( CenterAndVariabilityStrings.rangeStringProperty, TEXT_OPTIONS ), { + tandem: tandem.createTandem( 'rangeCheckbox' ) + } ) + }, { + value: VariabilityMeasure.IQR, + createNode: tandem => new Checkbox( model.isShowingIQRProperty, new Text( CenterAndVariabilityStrings.iqrStringProperty, TEXT_OPTIONS ), { + tandem: tandem.createTandem( 'iqrCheckbox' ) + } ) + }, { + value: VariabilityMeasure.MAD, + createNode: tandem => new Checkbox( model.isShowingMADProperty, new Text( CenterAndVariabilityStrings.madStringProperty, TEXT_OPTIONS ), { + tandem: tandem.createTandem( 'madCheckbox' ) + } ) + } + ] ), + new Text( accordionBoxTitleProperty, { + font: new PhetFont( 16 ), + maxWidth: 300 + } ), + layoutBounds, { + leftMargin: 70, + tandem: tandem, + contentNodeOffsetY: 0, + top: top, + valueReadoutsNode: new VariabilityReadoutsNode( model ), + right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN, + infoShowingProperty: model.isInfoShowingProperty + } ); + + }, options ); + + afterInit!(); + const distributionRadioButtonGroup = new DistributionRadioButtonGroup( model.selectedDistributionProperty, { left: 10, tandem: options.tandem.createTandem( 'distributionRadioButtonGroup' ) @@ -106,19 +144,9 @@ export default class VariabilityScreenView extends CAVScreenView { } ); this.addChild( distributionRadioButtonGroup ); - - const variabilityMeasureRadioButtonGroup = new VariabilityMeasureRadioButtonGroup( model.selectedVariabilityProperty, { - left: 10, - tandem: options.tandem.createTandem( 'variabilityMeasureRadioButtonGroup' ) - } ); this.addChild( variabilityMeasureRadioButtonGroup ); - ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBoxContents ], - ( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => { - variabilityRadioButtonGroupWrapper.top = accordionBoxWrapper.top; - } ); - - const infoDialog = new InfoDialog( model, this.chartViewWidth, { + const infoDialog = new InfoDialog( model, CAVConstants.CHART_VIEW_WIDTH, { tandem: options.tandem.createTandem( 'infoDialog' ) } ); model.isInfoShowingProperty.link( isInfoShowing => {