From ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b Mon Sep 17 00:00:00 2001 From: Marla Schulz Date: Thu, 27 Apr 2023 12:10:25 -0700 Subject: [PATCH] Restructure TopRepresentationCheckboxGroup to reflect same API in BottomRepresentationCheckboxGroup, see: https://github.com/phetsims/center-and-variability/issues/153 --- js/common/view/CAVAccordionBox.ts | 15 ++- js/common/view/CardNodeContainer.ts | 13 --- .../view/TopRepresentationCheckboxGroup.ts | 102 +++++++++--------- js/mean-and-median/view/MedianPlotNode.ts | 21 ++-- js/median/view/MedianAccordionBox.ts | 20 +++- 5 files changed, 89 insertions(+), 82 deletions(-) diff --git a/js/common/view/CAVAccordionBox.ts b/js/common/view/CAVAccordionBox.ts index 43aa8fb6..69bf1a5e 100644 --- a/js/common/view/CAVAccordionBox.ts +++ b/js/common/view/CAVAccordionBox.ts @@ -9,7 +9,7 @@ import Bounds2 from '../../../../dot/js/Bounds2.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; -import { Node, Rectangle } from '../../../../scenery/js/imports.js'; +import { AlignBox, Node, Rectangle } from '../../../../scenery/js/imports.js'; import AccordionBox, { AccordionBoxOptions } from '../../../../sun/js/AccordionBox.js'; import CAVConstants from '../CAVConstants.js'; import optionize from '../../../../phet-core/js/optionize.js'; @@ -17,6 +17,7 @@ import centerAndVariability from '../../centerAndVariability.js'; import CAVModel from '../model/CAVModel.js'; import { Shape } from '../../../../kite/js/imports.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; +import VerticalCheckboxGroup, { VerticalCheckboxGroupItem } from '../../../../sun/js/VerticalCheckboxGroup.js'; type SelfOptions = { leftMargin: number; @@ -87,6 +88,18 @@ export default class CAVAccordionBox extends AccordionBox { this.contentNode = contentNode; } + + protected setCheckboxGroup( items: VerticalCheckboxGroupItem[] ): void { + const accordionCheckboxGroup = new VerticalCheckboxGroup( items, { + tandem: this.tandem.createTandem( 'accordionCheckboxGroup' ) + } ); + this.addChild( new AlignBox( accordionCheckboxGroup, { + alignBounds: this.getVisibleBounds(), + xAlign: 'right', + xMargin: 80, + yAlign: 'top' + } ) ); + } } centerAndVariability.register( 'CAVAccordionBox', CAVAccordionBox ); \ No newline at end of file diff --git a/js/common/view/CardNodeContainer.ts b/js/common/view/CardNodeContainer.ts index b10d386e..2617e0ef 100644 --- a/js/common/view/CardNodeContainer.ts +++ b/js/common/view/CardNodeContainer.ts @@ -41,7 +41,6 @@ import DragIndicatorArrowNode from './DragIndicatorArrowNode.js'; import TEmitter from '../../../../axon/js/TEmitter.js'; import MedianModel from '../../median/model/MedianModel.js'; import PatternStringProperty from '../../../../axon/js/PatternStringProperty.js'; -import TopRepresentationCheckboxGroup from './TopRepresentationCheckboxGroup.js'; // constants const CARD_SPACING = 10; @@ -85,18 +84,6 @@ export default class CardNodeContainer extends Node { super( options ); - const checkboxGroup = new TopRepresentationCheckboxGroup( model, { - includeSortData: true, - includeMean: false, - medianBarIconOptions: { - notchDirection: 'up', - barStyle: 'continuous' - }, - showMedianCheckboxIcon: false, - tandem: options.tandem.createTandem( 'checkboxGroup' ) - } ); - this.addChild( checkboxGroup ); - this.model = model; // TODO-UX: maybe this should be converted to track distance for individual cards, see https://github.com/phetsims/center-and-variability/issues/111 diff --git a/js/common/view/TopRepresentationCheckboxGroup.ts b/js/common/view/TopRepresentationCheckboxGroup.ts index ce6d0ba0..3c5297cc 100644 --- a/js/common/view/TopRepresentationCheckboxGroup.ts +++ b/js/common/view/TopRepresentationCheckboxGroup.ts @@ -7,31 +7,16 @@ * @author Sam Reid (PhET Interactive Simulations) */ -import optionize from '../../../../phet-core/js/optionize.js'; import centerAndVariability from '../../centerAndVariability.js'; -import VerticalCheckboxGroup, { VerticalCheckboxGroupItem, VerticalCheckboxGroupOptions } from '../../../../sun/js/VerticalCheckboxGroup.js'; -import { HBox, Line, Node, Text } from '../../../../scenery/js/imports.js'; -import CAVModel from '../model/CAVModel.js'; +import VerticalCheckboxGroup, { VerticalCheckboxGroupItem } from '../../../../sun/js/VerticalCheckboxGroup.js'; +import { AlignGroup, GridBox, Line, Node, Text } from '../../../../scenery/js/imports.js'; import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js'; import CAVConstants from '../CAVConstants.js'; import NumberLineNode from './NumberLineNode.js'; -import MedianBarNode, { MedianBarNodeOptions } from './MedianBarNode.js'; +import MedianBarNode from './MedianBarNode.js'; import CAVColors from '../CAVColors.js'; -import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import Tandem from '../../../../tandem/js/Tandem.js'; -import MedianModel from '../../median/model/MedianModel.js'; - -type SelfOptions = { - - // TODO: Refactor this like we did for BottomRepresentationCheckboxGroup - includeSortData?: boolean; - includeMedian?: boolean; - includeMean?: boolean; - medianBarIconOptions: MedianBarNodeOptions; - showMedianCheckboxIcon: boolean; -}; -export type TopRepresentationCheckboxGroupOptions = SelfOptions & VerticalCheckboxGroupOptions & - PickRequired; +import Property from '../../../../axon/js/Property.js'; // constants const ICON_WIDTH = 24; @@ -41,41 +26,52 @@ const LINE_WIDTH = 2; export default class TopRepresentationCheckboxGroup extends VerticalCheckboxGroup { - public constructor( model: CAVModel, providedOptions?: TopRepresentationCheckboxGroupOptions ) { - - const options = optionize()( { - includeSortData: false, - includeMean: true, - includeMedian: true - }, providedOptions ); + private static createGridBox( text: Node, icon: Node, iconGroup: AlignGroup ): GridBox { + return new GridBox( { + rows: [ [ new Node( { children: [ text ], layoutOptions: { align: 'left' } } ), + iconGroup.createBox( icon, { xAlign: 'center', layoutOptions: { xAlign: 'right' } } ) ] + ] + } ); + } - const items: VerticalCheckboxGroupItem[] = []; - options.includeSortData && items.push( { + public static getSortDataCheckboxItem( isSortingDataProperty: Property ): VerticalCheckboxGroupItem { + return { createNode: ( tandem: Tandem ) => new Text( CenterAndVariabilityStrings.sortDataStringProperty, CAVConstants.CHECKBOX_TEXT_OPTIONS ), - property: ( model as MedianModel ).isSortingDataProperty, + property: isSortingDataProperty, tandemName: 'sortDataCheckbox' - } ); - options.includeMedian && items.push( { - createNode: ( tandem: Tandem ) => new HBox( { - spacing: 12, - children: [ + }; + } + + public static getMedianCheckboxWithIconItem( iconGroup: AlignGroup, isShowingTopMedianProperty: Property ): VerticalCheckboxGroupItem { + return { + createNode: ( tandem: Tandem ) => { + return TopRepresentationCheckboxGroup.createGridBox( new Text( CenterAndVariabilityStrings.medianStringProperty, CAVConstants.CHECKBOX_TEXT_OPTIONS ), - ...options.showMedianCheckboxIcon ? [ - new MedianBarNode( options.medianBarIconOptions ) - .setMedianBarShape( 0, 0, ICON_WIDTH / 2 - LINE_WIDTH / 2, ICON_WIDTH - LINE_WIDTH, true ) - ] : [] - ] - } ), - property: model.isShowingTopMedianProperty, + new MedianBarNode( { + notchDirection: 'up', + barStyle: 'continuous' + } ) + .setMedianBarShape( 0, 0, ICON_WIDTH / 2 - LINE_WIDTH / 2, ICON_WIDTH - LINE_WIDTH, true ), + iconGroup + ); + }, + property: isShowingTopMedianProperty, tandemName: 'medianCheckbox' - } ); + }; + } - options.includeMean && items.push( { - createNode: ( tandem: Tandem ) => new HBox( { + public static getMedianCheckboxWithoutIconItem( isShowingTopMedianProperty: Property ): VerticalCheckboxGroupItem { + return { + createNode: ( tandem: Tandem ) => new Text( CenterAndVariabilityStrings.medianStringProperty, CAVConstants.CHECKBOX_TEXT_OPTIONS ), + property: isShowingTopMedianProperty, + tandemName: 'medianCheckbox' + }; + } - // TODO: align icons - spacing: 24.5, - children: [ + public static getMeanCheckboxWithIconItem( iconGroup: AlignGroup, isShowingTopMeanProperty: Property ): VerticalCheckboxGroupItem { + return { + createNode: ( tandem: Tandem ) => { + return TopRepresentationCheckboxGroup.createGridBox( new Text( CenterAndVariabilityStrings.meanStringProperty, CAVConstants.CHECKBOX_TEXT_OPTIONS ), new Node( { children: [ @@ -89,13 +85,13 @@ export default class TopRepresentationCheckboxGroup extends VerticalCheckboxGrou // Triangle NumberLineNode.createMeanIndicatorNode( false, true ) ] - } ) - ] - } ), - property: model.isShowingTopMeanProperty, + } ), + iconGroup + ); + }, + property: isShowingTopMeanProperty, tandemName: 'meanCheckbox' - } ); - super( items, options ); + }; } } diff --git a/js/mean-and-median/view/MedianPlotNode.ts b/js/mean-and-median/view/MedianPlotNode.ts index 4a260252..81116527 100644 --- a/js/mean-and-median/view/MedianPlotNode.ts +++ b/js/mean-and-median/view/MedianPlotNode.ts @@ -15,7 +15,6 @@ import MedianBarNode from '../../common/view/MedianBarNode.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import CAVPlotNode from '../../common/view/CAVPlotNode.js'; import MeanAndMedianModel from '../model/MeanAndMedianModel.js'; -import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js'; export type CAVPlotOptions = NodeOptions & PickRequired; @@ -35,16 +34,16 @@ export default class MedianPlotNode extends CAVPlotNode { ...providedOptions } ); - const checkboxGroup = new TopRepresentationCheckboxGroup( model, { - medianBarIconOptions: { - notchDirection: 'down', - barStyle: 'continuous', - arrowScale: 0.75 - }, - showMedianCheckboxIcon: true, - tandem: providedOptions.tandem.createTandem( 'topRepresentationCheckboxGroup' ) - } ); - this.addChild( checkboxGroup ); + // const checkboxGroup = new TopRepresentationCheckboxGroup( model, { + // medianBarIconOptions: { + // notchDirection: 'down', + // barStyle: 'continuous', + // arrowScale: 0.75 + // }, + // showMedianCheckboxIcon: true, + // tandem: providedOptions.tandem.createTandem( 'topRepresentationCheckboxGroup' ) + // } ); + // this.addChild( checkboxGroup ); this.addChild( this.medianBarNode ); diff --git a/js/median/view/MedianAccordionBox.ts b/js/median/view/MedianAccordionBox.ts index 7f2bffa1..205d0b1d 100644 --- a/js/median/view/MedianAccordionBox.ts +++ b/js/median/view/MedianAccordionBox.ts @@ -9,15 +9,19 @@ import MedianModel from '../model/MedianModel.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import Tandem from '../../../../tandem/js/Tandem.js'; import centerAndVariability from '../../centerAndVariability.js'; +import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js'; export default class MedianAccordionBox extends CAVAccordionBox { public constructor( model: MedianModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) { - super( 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' ) - } ), + const cardNodeContainer = 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' ) + } ); + + super( model, cardNodeContainer, new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, { font: new PhetFont( 16 ), maxWidth: 300 @@ -28,7 +32,15 @@ export default class MedianAccordionBox extends CAVAccordionBox { top: top, centerX: layoutBounds.centerX } ); + + this.setCheckboxGroup( [ + TopRepresentationCheckboxGroup.getSortDataCheckboxItem( model.isSortingDataProperty ), + TopRepresentationCheckboxGroup.getMedianCheckboxWithoutIconItem( model.isShowingTopMedianProperty ) + ] + ); } + + } centerAndVariability.register( 'MedianAccordionBox', MedianAccordionBox ); \ No newline at end of file