From 8c193b35542afd7e0c5b9ac86942dc37236ac740 Mon Sep 17 00:00:00 2001 From: Sam Reid Date: Tue, 25 Apr 2023 08:41:49 -0600 Subject: [PATCH] Move MeanOrMedianScreenView.ts into SoccerScreenView.ts, see https://github.com/phetsims/center-and-variability/issues/152 --- js/common/view/MeanOrMedianScreenView.ts | 126 ------------------ js/common/view/SoccerScreenView.ts | 99 +++++++++++++- .../view/MeanAndMedianScreenView.ts | 8 +- js/median/view/MedianScreenView.ts | 8 +- js/variability/view/VariabilityScreenView.ts | 9 +- 5 files changed, 110 insertions(+), 140 deletions(-) delete mode 100644 js/common/view/MeanOrMedianScreenView.ts diff --git a/js/common/view/MeanOrMedianScreenView.ts b/js/common/view/MeanOrMedianScreenView.ts deleted file mode 100644 index 5dcf981b..00000000 --- a/js/common/view/MeanOrMedianScreenView.ts +++ /dev/null @@ -1,126 +0,0 @@ -// Copyright 2022-2023, University of Colorado Boulder - -/** - * Screen View for the "Median" and "Mean and Median" screen - * - * @author Chris Klusendorf (PhET Interactive Simulations) - * @author Sam Reid (PhET Interactive Simulations) - */ - -import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; -import centerAndVariability from '../../centerAndVariability.js'; -import SoccerScreenView, { SoccerScreenViewOptions } from '../../common/view/SoccerScreenView.js'; -import CAVAccordionBox from '../../common/view/CAVAccordionBox.js'; -import CAVConstants from '../../common/CAVConstants.js'; -import CardNodeContainer from '../../common/view/CardNodeContainer.js'; -import SoccerModel from '../model/SoccerModel.js'; -import ValueReadoutsNode from './ValueReadoutsNode.js'; -import { ManualConstraint, Node, Text } from '../../../../scenery/js/imports.js'; -import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; -import ScreenView from '../../../../joist/js/ScreenView.js'; -import Bounds2 from '../../../../dot/js/Bounds2.js'; -import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; -import VariabilityModel from '../../variability/model/VariabilityModel.js'; -import CAVPlotNodeWithMedianBar from '../../mean-and-median/view/CAVPlotNodeWithMedianBar.js'; -import VariabilityPlotNode from '../../variability/view/VariabilityPlotNode.js'; -import VariabilityReadoutsNode from '../../variability/view/VariabilityReadoutsNode.js'; -import MedianModel from '../../median/model/MedianModel.js'; - -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 MeanOrMedianScreenViewOptions = SelfOptions & SoccerScreenViewOptions; - -export default class MeanOrMedianScreenView extends SoccerScreenView { - private readonly accordionBox: CAVAccordionBox; - protected readonly accordionBoxContents: Node; - - public constructor( model: SoccerModel, providedOptions: MeanOrMedianScreenViewOptions ) { - - const options = optionize()( {}, providedOptions ); - - super( model, options ); - - 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 ); - - - // 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 ); - } - - this.contentLayer.addChild( this.medianPredictionNode ); - } - - /** - * Floating layout that keeps the ground near the ground, and accordion box near the question bar - */ - public override layout( viewBounds: Bounds2 ): void { - - // TODO: Duplicates effort with the parent implementation - this.matrix = ScreenView.getLayoutMatrix( this.layoutBounds, viewBounds, { - verticalAlign: 'bottom' - } ); - this.visibleBoundsProperty.value = this.parentToLocalBounds( viewBounds ); - - this.accordionBox.top = this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN; - } -} - -centerAndVariability.register( 'MeanOrMedianScreenView', MeanOrMedianScreenView ); \ No newline at end of file diff --git a/js/common/view/SoccerScreenView.ts b/js/common/view/SoccerScreenView.ts index 4f1ddcdf..cc3301ee 100644 --- a/js/common/view/SoccerScreenView.ts +++ b/js/common/view/SoccerScreenView.ts @@ -13,7 +13,7 @@ import CAVScreenView, { CAVScreenViewOptions } from './CAVScreenView.js'; import QuestionBar, { QuestionBarOptions } from '../../../../scenery-phet/js/QuestionBar.js'; import KickButtonGroup from './KickButtonGroup.js'; import BackgroundNode from './BackgroundNode.js'; -import { Node } from '../../../../scenery/js/imports.js'; +import { ManualConstraint, Node, Text } from '../../../../scenery/js/imports.js'; import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import ScreenView from '../../../../joist/js/ScreenView.js'; @@ -23,9 +23,25 @@ import PhetioGroup from '../../../../tandem/js/PhetioGroup.js'; import SoccerPlayer from '../model/SoccerPlayer.js'; import NumberLineNode from './NumberLineNode.js'; import merge from '../../../../phet-core/js/merge.js'; +import CAVAccordionBox from './CAVAccordionBox.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 CAVConstants from '../CAVConstants.js'; +import VariabilityReadoutsNode from '../../variability/view/VariabilityReadoutsNode.js'; +import ValueReadoutsNode from './ValueReadoutsNode.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; type SelfOptions = { questionBarOptions: QuestionBarOptions; + + // 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 SoccerScreenViewOptions = SelfOptions & CAVScreenViewOptions; @@ -34,6 +50,10 @@ const GROUND_POSITION_Y = 500; const NUMBER_LINE_MARGIN_X = 207; export default class SoccerScreenView extends CAVScreenView { + + private readonly accordionBox: CAVAccordionBox; + protected readonly accordionBoxContents: Node; + protected readonly questionBar: QuestionBar; protected readonly chartViewWidth: number; protected readonly playAreaNumberLineNode: NumberLineNode; @@ -116,6 +136,83 @@ export default class SoccerScreenView extends CAVScreenView { // 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 ); + + + // 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 ); + } + + this.contentLayer.addChild( this.medianPredictionNode ); + } + + /** + * Floating layout that keeps the ground near the ground, and accordion box near the question bar + */ + public override layout( viewBounds: Bounds2 ): void { + + // TODO: Duplicates effort with the parent implementation + this.matrix = ScreenView.getLayoutMatrix( this.layoutBounds, viewBounds, { + verticalAlign: 'bottom' + } ); + this.visibleBoundsProperty.value = this.parentToLocalBounds( viewBounds ); + + this.accordionBox.top = this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN; } } diff --git a/js/mean-and-median/view/MeanAndMedianScreenView.ts b/js/mean-and-median/view/MeanAndMedianScreenView.ts index fc7ddb83..8a5a44c0 100644 --- a/js/mean-and-median/view/MeanAndMedianScreenView.ts +++ b/js/mean-and-median/view/MeanAndMedianScreenView.ts @@ -13,16 +13,16 @@ 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 MeanOrMedianScreenView, { MeanOrMedianScreenViewOptions } from '../../common/view/MeanOrMedianScreenView.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; +import SoccerScreenView, { SoccerScreenViewOptions } from '../../common/view/SoccerScreenView.js'; -type MeanAndMedianScreenViewOptions = StrictOmit; +type MeanAndMedianScreenViewOptions = StrictOmit; -export default class MeanAndMedianScreenView extends MeanOrMedianScreenView { +export default class MeanAndMedianScreenView extends SoccerScreenView { public constructor( model: MeanAndMedianModel, providedOptions: MeanAndMedianScreenViewOptions ) { - const options = optionize()( { + const options = optionize()( { isMedianScreen: false, questionBarOptions: { barFill: CAVColors.meanAndMedianQuestionBarFillColorProperty, diff --git a/js/median/view/MedianScreenView.ts b/js/median/view/MedianScreenView.ts index e2332606..8bfa4181 100644 --- a/js/median/view/MedianScreenView.ts +++ b/js/median/view/MedianScreenView.ts @@ -12,20 +12,20 @@ import centerAndVariability from '../../centerAndVariability.js'; import MedianModel from '../model/MedianModel.js'; import CAVColors from '../../common/CAVColors.js'; import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; -import MeanOrMedianScreenView, { MeanOrMedianScreenViewOptions } from '../../common/view/MeanOrMedianScreenView.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js'; +import SoccerScreenView, { SoccerScreenViewOptions } from '../../common/view/SoccerScreenView.js'; type SelfOptions = EmptySelfOptions; type MedianScreenViewOptions = SelfOptions - & StrictOmit; + & StrictOmit; -export default class MedianScreenView extends MeanOrMedianScreenView { +export default class MedianScreenView extends SoccerScreenView { public constructor( model: MedianModel, providedOptions: MedianScreenViewOptions ) { - const options = optionize()( { + const options = optionize()( { isMedianScreen: true, isVariabilityScreen: false, questionBarOptions: { diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts index 71b9fedc..caf0af3c 100644 --- a/js/variability/view/VariabilityScreenView.ts +++ b/js/variability/view/VariabilityScreenView.ts @@ -12,10 +12,9 @@ import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize. import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import centerAndVariability from '../../centerAndVariability.js'; import VariabilityModel from '../model/VariabilityModel.js'; -import { SoccerScreenViewOptions } from '../../common/view/SoccerScreenView.js'; +import SoccerScreenView, { SoccerScreenViewOptions } from '../../common/view/SoccerScreenView.js'; import CAVColors from '../../common/CAVColors.js'; import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; -import MeanOrMedianScreenView, { MeanOrMedianScreenViewOptions } from '../../common/view/MeanOrMedianScreenView.js'; import { ManualConstraint, Text } from '../../../../scenery/js/imports.js'; import DistributionRadioButtonGroup from './DistributionRadioButtonGroup.js'; import VariabilityMeasureRadioButtonGroup from './VariabilityMeasureRadioButtonGroup.js'; @@ -34,9 +33,9 @@ const TEXT_OPTIONS = { }; type SelfOptions = EmptySelfOptions; -type VariabilityScreenViewOptions = SelfOptions & StrictOmit; +type VariabilityScreenViewOptions = SelfOptions & StrictOmit; -export default class VariabilityScreenView extends MeanOrMedianScreenView { +export default class VariabilityScreenView extends SoccerScreenView { public constructor( model: VariabilityModel, providedOptions: VariabilityScreenViewOptions ) { @@ -48,7 +47,7 @@ export default class VariabilityScreenView extends MeanOrMedianScreenView { const accordionBoxTitleProperty = new DynamicProperty( currentProperty ); - const options = optionize()( { + const options = optionize()( { isMedianScreen: false, isVariabilityScreen: true, questionBarOptions: {