AccordionBox layout is problematic #170
Assignees
Comments
|
More ideas in this patch: Subject: [PATCH] Update TODOs, see https://github.com/phetsims/center-and-variability/issues/45
---
Index: main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts
--- a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts (date 1683210576999)
@@ -37,7 +37,10 @@
super( model, options );
- this.setAccordionBoxWithAlignedContent( new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode ) );
+ this.setAccordionBoxWithAlignedContent(
+ new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode ),
+ this.plotNode
+ );
const iconGroup = new AlignGroup();
super.setBottomCheckboxGroup( [
Index: main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts
--- a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (date 1683206737157)
@@ -13,6 +13,7 @@
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
export default class MeanAndMedianAccordionBox extends CAVAccordionBox {
+ public readonly plotNode: MedianPlotNode;
public constructor( model: MeanAndMedianModel, layoutBounds: Bounds2, tandem: Tandem, top: number, playAreaNumberLineNode: Node ) {
const iconGroup = new AlignGroup();
@@ -35,14 +36,15 @@
maxWidth: 300
} ),
layoutBounds,
- checkboxGroup,
- {
+ checkboxGroup, {
leftMargin: 0,
tandem: tandem,
top: top,
centerX: layoutBounds.centerX
}
);
+
+ this.plotNode = plotNode;
}
}
Index: main/center-and-variability/js/variability/view/VariabilityPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts b/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts
--- a/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts (date 1683211207001)
@@ -33,21 +33,24 @@
} ),
tandemName: 'rangeNode',
value: VariabilityMeasure.RANGE
- }, {
- createNode: tandem => new IQRNode( model, sceneModel, {
- parentContext: 'accordion',
- tandem: tandem.createTandem( 'iqrNode' )
- } ),
- tandemName: 'iqrNode',
- value: VariabilityMeasure.IQR
- }, {
- createNode: tandem => new MADNode( model, sceneModel, {
- parentContext: 'accordion',
- tandem: tandem.createTandem( 'madNode' )
- } ),
- tandemName: 'madNode',
- value: VariabilityMeasure.MAD
- } ], {
+ }
+ // , {
+ // createNode: tandem => new IQRNode( model, sceneModel, {
+ // parentContext: 'accordion',
+ // tandem: tandem.createTandem( 'iqrNode' )
+ // } ),
+ // tandemName: 'iqrNode',
+ // value: VariabilityMeasure.IQR
+ // }
+ , {
+ createNode: tandem => new MADNode( model, sceneModel, {
+ parentContext: 'accordion',
+ tandem: tandem.createTandem( 'madNode' )
+ } ),
+ tandemName: 'madNode',
+ value: VariabilityMeasure.MAD
+ }
+ ], {
tandem: providedOptions.tandem.createTandem( 'toggleNode' )
} );
this.addChild( toggleNode );
Index: main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
--- a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (date 1683211751744)
@@ -1,7 +1,7 @@
// Copyright 2023, University of Colorado Boulder
import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
-import { AlignGroup, Text } from '../../../../scenery/js/imports.js';
+import { AlignGroup, Color, Node, Rectangle, Text } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
@@ -11,7 +11,6 @@
import VariabilityPlotNode from './VariabilityPlotNode.js';
import InfoButton from '../../../../scenery-phet/js/buttons/InfoButton.js';
import VariabilityMeasure from '../model/VariabilityMeasure.js';
-import CAVConstants from '../../common/CAVConstants.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
import ToggleNode from '../../../../sun/js/ToggleNode.js';
@@ -19,6 +18,7 @@
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
export default class VariabilityAccordionBox extends CAVAccordionBox {
+ public readonly plotNode: ToggleNode<Node>;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
@@ -39,24 +39,10 @@
};
} );
- const accordionBoxContents = new ToggleNode( model.selectedSceneModelProperty, contents );
-
- const infoButton = new InfoButton( {
- iconFill: 'cornflowerblue',
- scale: 0.5,
- touchAreaDilation: 5,
- tandem: tandem.createTandem( 'infoButton' ),
- listener: () => {
- model.isInfoShowingProperty.value = true;
- },
-
- // TODO: How to position this properly? Can we use AlignBox?
- top: 20,
- right: accordionBoxContents.right - 10
- } );
- accordionBoxContents.addChild( infoButton );
-
const iconGroup = new AlignGroup();
+
+ const RECT_WIDTH = 900;
+ const RECT_HEIGHT = 150;
const checkboxToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
{
createNode: tandem => new VerticalCheckboxGroup( [
@@ -79,7 +65,37 @@
tandemName: 'madAccordionCheckboxGroup',
value: VariabilityMeasure.MAD
}
- ] );
+ ], {
+ right: RECT_WIDTH - 10,
+ centerY: RECT_HEIGHT / 2
+ } );
+
+ const plotNodes = new ToggleNode( model.selectedSceneModelProperty, contents, {
+ alignChildren: ToggleNode.NONE,
+ // excludeInvisibleChildrenFromBounds: true
+ // left: 0,
+ // top: 0
+ } );
+
+ const accordionBoxContents = new Rectangle( 0, 0, RECT_WIDTH, RECT_HEIGHT, {
+ fill: new Color( 0, 0, 255, 0.1 ),
+ children: [
+ // plotNodes,
+ new InfoButton( {
+ iconFill: 'cornflowerblue',
+ scale: 0.5,
+ touchAreaDilation: 5,
+ tandem: tandem.createTandem( 'infoButton' ),
+ listener: () => {
+ model.isInfoShowingProperty.value = true;
+ },
+
+ top: 0,
+ right: RECT_WIDTH - 10
+ } ),
+ checkboxToggleNode
+ ]
+ } );
super( model.resetEmitter, accordionBoxContents,
new Text( accordionBoxTitleProperty, {
@@ -87,13 +103,19 @@
maxWidth: 300
} ),
layoutBounds,
- checkboxToggleNode,
+ // checkboxToggleNode,
{
- leftMargin: 70,
+ // leftMargin: 70,
tandem: tandem,
top: top,
- right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
+ right: layoutBounds.right
} );
+
+ // ManualConstraint.create( this, [ infoButton, this.expandedTitleBar ], ( infoButtonWrapper, expandedTitleBarWrapper ) => {
+ // infoButtonWrapper.rightTop = expandedTitleBarWrapper.rightTop;
+ // } );
+
+ this.plotNode = plotNodes;
}
}
Index: main/center-and-variability/js/common/view/CAVAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/view/CAVAccordionBox.ts b/main/center-and-variability/js/common/view/CAVAccordionBox.ts
--- a/main/center-and-variability/js/common/view/CAVAccordionBox.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/common/view/CAVAccordionBox.ts (date 1683207764659)
@@ -9,17 +9,15 @@
import Bounds2 from '../../../../dot/js/Bounds2.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import { AlignBox, Node, Rectangle } from '../../../../scenery/js/imports.js';
+import { Node } 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';
import centerAndVariability from '../../centerAndVariability.js';
-import { Shape } from '../../../../kite/js/imports.js';
import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
import TEmitter from '../../../../axon/js/TEmitter.js';
type SelfOptions = {
- leftMargin: number;
+ // leftMargin: number;
};
export type CAVAccordionBoxOptions =
SelfOptions
@@ -32,11 +30,11 @@
export default class CAVAccordionBox extends AccordionBox {
- public readonly plotNode: Node;
+ // public readonly plotNode: Node;
// NOTE: The positions of the passed-in nodes are modified directly, so they cannot be used in the scenery DAG
- public constructor( resetEmitter: TEmitter, plotNode: Node, titleNode: Node,
- layoutBounds: Bounds2, checkboxGroup: Node, providedOptions: CAVAccordionBoxOptions ) {
+ public constructor( resetEmitter: TEmitter, contentNode: Node, titleNode: Node,
+ layoutBounds: Bounds2, providedOptions: CAVAccordionBoxOptions ) {
const options = optionize<CAVAccordionBoxOptions, SelfOptions, AccordionBoxOptions>()( {
titleAlignX: 'left',
@@ -59,40 +57,43 @@
titleNode: titleNode
}, providedOptions );
- const backgroundNode = new Rectangle( {
- rectHeight: 140,
- rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
- } );
+ // const backgroundNode = new Rectangle( {
+ // rectHeight: 140,
+ // rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
+ // } );
// Since the title is visible while the accordion box is open, this background will not any area above the bottom of
// the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
// Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
// horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
- const fullBackgroundBounds =
- backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
-
- // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
- backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
+ // const fullBackgroundBounds =
+ // backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
+ //
+ // // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
+ // backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
// Vertical positioning
- plotNode.centerY = fullBackgroundBounds.centerY;
- if ( plotNode.bottom > fullBackgroundBounds.bottom - 5 ) {
- plotNode.bottom = fullBackgroundBounds.bottom - 5;
- }
- backgroundNode.addChild( plotNode );
+ // contentNode.centerY = fullBackgroundBounds.centerY;
+ // if ( contentNode.bottom > fullBackgroundBounds.bottom - 5 ) {
+ // contentNode.bottom = fullBackgroundBounds.bottom - 5;
+ // }
+ // backgroundNode.addChild( contentNode );
- const checkboxAlignBox = new AlignBox( checkboxGroup,
- { xAlign: 'right', yAlign: 'center', rightMargin: 20, alignBounds: fullBackgroundBounds } );
+ // const checkboxAlignBox = new AlignBox( checkboxGroup, {
+ // xAlign: 'right',
+ // yAlign: 'center',
+ // rightMargin: 20
+ // alignBounds: fullBackgroundBounds
+ // } );
- backgroundNode.addChild( checkboxAlignBox );
+ // backgroundNode.addChild( checkboxAlignBox );
- super( backgroundNode, options );
+ super( contentNode, options );
resetEmitter.addListener( () => this.reset() );
- this.plotNode = plotNode;
+ this.plotNode = contentNode;
}
-
}
centerAndVariability.register( 'CAVAccordionBox', CAVAccordionBox );
\ No newline at end of file
Index: main/center-and-variability/js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/view/CAVScreenView.ts b/main/center-and-variability/js/common/view/CAVScreenView.ts
--- a/main/center-and-variability/js/common/view/CAVScreenView.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/common/view/CAVScreenView.ts (date 1683210635625)
@@ -31,6 +31,9 @@
import SceneView from './SceneView.js';
import KickButtonGroup from './KickButtonGroup.js';
import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
+import CAVPlotNode from './CAVPlotNode.js';
+import MeanAndMedianAccordionBox from '../../mean-and-median/view/MeanAndMedianAccordionBox.js';
+import VariabilityAccordionBox from '../../variability/view/VariabilityAccordionBox.js';
type SelfOptions = {
questionBarOptions: QuestionBarOptions;
@@ -189,12 +192,12 @@
* -Same offset and scale
* Given those assumptions, this code moves the dot plot so that its number line matches the play area one.
*/
- protected setAccordionBoxWithAlignedContent( accordionBox: CAVAccordionBox ): void {
+ protected setAccordionBoxWithAlignedContent( accordionBox: MeanAndMedianAccordionBox | VariabilityAccordionBox ): void {
this.setAccordionBox( accordionBox );
- ManualConstraint.create( this, [ this.playAreaNumberLineNode, accordionBox.plotNode ],
- ( lowerNumberLineWrapper, contentsWrapper ) => {
- contentsWrapper.x = lowerNumberLineWrapper.x;
- } );
+ // ManualConstraint.create( this, [ this.playAreaNumberLineNode, accordionBox.plotNode ],
+ // ( playAreaNumberLine, plotNode ) => {
+ // // plotNode.x = playAreaNumberLine.x;
+ // } );
}
protected setBottomCheckboxGroup( items: VerticalCheckboxGroupItem[] ): void {
Index: main/sun/js/ToggleNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ToggleNode.ts b/main/sun/js/ToggleNode.ts
--- a/main/sun/js/ToggleNode.ts (revision 8346e6d4343725b09c3556e51b66efc658e2a6a1)
+++ b/main/sun/js/ToggleNode.ts (date 1683204944669)
@@ -34,15 +34,6 @@
public constructor( valueProperty: TReadOnlyProperty<T>, elements: ToggleNodeElement<T>[], providedOptions?: ToggleNodeOptions ) {
- // assert && assert( Array.isArray( elements ), 'elements should be an array' );
- // if ( assert ) {
- // elements.forEach( element => {
- // const keys = _.keys( element );
- // assert && assert( keys.length === 2, 'each element should have two keys' );
- // assert && assert( keys[ 0 ] === 'value' || keys[ 1 ] === 'value', 'element should have a value key' );
- // } );
- // }
-
const options = optionize<ToggleNodeOptions, SelfOptions, NodeOptions>()( {
// SelfOptions
Index: main/center-and-variability/js/variability/view/VariabilityScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityScreenView.ts b/main/center-and-variability/js/variability/view/VariabilityScreenView.ts
--- a/main/center-and-variability/js/variability/view/VariabilityScreenView.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/variability/view/VariabilityScreenView.ts (date 1683211773992)
@@ -45,7 +45,17 @@
super( model, options );
- this.setAccordionBoxWithAlignedContent( new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN ) );
+ const variabilityAccordionBox = new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN );
+ this.setAccordionBox( variabilityAccordionBox );
+
+ // variabilityAccordionBox.plotNode.x = this.playAreaNumberLineNode.x;
+ this.addChild( variabilityAccordionBox.plotNode );
+
+ ManualConstraint.create( this, [ this.playAreaNumberLineNode, variabilityAccordionBox.plotNode, variabilityAccordionBox ],
+ ( playAreaNumberLine, plotNode, variabilityAccordionBox ) => {
+ plotNode.x = playAreaNumberLine.x;
+ plotNode.bottom = variabilityAccordionBox.bottom;
+ } );
ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBox! ],
( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => {
Index: main/sun/js/AccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/AccordionBox.ts b/main/sun/js/AccordionBox.ts
--- a/main/sun/js/AccordionBox.ts (revision 8346e6d4343725b09c3556e51b66efc658e2a6a1)
+++ b/main/sun/js/AccordionBox.ts (date 1683206150982)
@@ -105,7 +105,7 @@
private readonly expandedBox: Rectangle;
private readonly collapsedBox: Rectangle;
private readonly workaroundBox: Rectangle;
- private readonly expandedTitleBar: InteractiveHighlightPath;
+ protected readonly expandedTitleBar: InteractiveHighlightPath;
private readonly collapsedTitleBar: InteractiveHighlightRectangle;
private readonly containerNode: Node;
private readonly resetAccordionBox: () => void;
|
|
Is this issue related to: #166 ? |
|
Current patch: Subject: [PATCH] Update TODOs, see https://github.com/phetsims/center-and-variability/issues/45
---
Index: main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts
--- a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianScreenView.ts (date 1683210576999)
@@ -37,7 +37,10 @@
super( model, options );
- this.setAccordionBoxWithAlignedContent( new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode ) );
+ this.setAccordionBoxWithAlignedContent(
+ new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode ),
+ this.plotNode
+ );
const iconGroup = new AlignGroup();
super.setBottomCheckboxGroup( [
Index: main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts
--- a/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (date 1683206737157)
@@ -13,6 +13,7 @@
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
export default class MeanAndMedianAccordionBox extends CAVAccordionBox {
+ public readonly plotNode: MedianPlotNode;
public constructor( model: MeanAndMedianModel, layoutBounds: Bounds2, tandem: Tandem, top: number, playAreaNumberLineNode: Node ) {
const iconGroup = new AlignGroup();
@@ -35,14 +36,15 @@
maxWidth: 300
} ),
layoutBounds,
- checkboxGroup,
- {
+ checkboxGroup, {
leftMargin: 0,
tandem: tandem,
top: top,
centerX: layoutBounds.centerX
}
);
+
+ this.plotNode = plotNode;
}
}
Index: main/center-and-variability/js/variability/view/VariabilityPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts b/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts
--- a/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts (date 1683211207001)
@@ -33,21 +33,24 @@
} ),
tandemName: 'rangeNode',
value: VariabilityMeasure.RANGE
- }, {
- createNode: tandem => new IQRNode( model, sceneModel, {
- parentContext: 'accordion',
- tandem: tandem.createTandem( 'iqrNode' )
- } ),
- tandemName: 'iqrNode',
- value: VariabilityMeasure.IQR
- }, {
- createNode: tandem => new MADNode( model, sceneModel, {
- parentContext: 'accordion',
- tandem: tandem.createTandem( 'madNode' )
- } ),
- tandemName: 'madNode',
- value: VariabilityMeasure.MAD
- } ], {
+ }
+ // , {
+ // createNode: tandem => new IQRNode( model, sceneModel, {
+ // parentContext: 'accordion',
+ // tandem: tandem.createTandem( 'iqrNode' )
+ // } ),
+ // tandemName: 'iqrNode',
+ // value: VariabilityMeasure.IQR
+ // }
+ , {
+ createNode: tandem => new MADNode( model, sceneModel, {
+ parentContext: 'accordion',
+ tandem: tandem.createTandem( 'madNode' )
+ } ),
+ tandemName: 'madNode',
+ value: VariabilityMeasure.MAD
+ }
+ ], {
tandem: providedOptions.tandem.createTandem( 'toggleNode' )
} );
this.addChild( toggleNode );
Index: main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
--- a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (date 1683211751744)
@@ -1,7 +1,7 @@
// Copyright 2023, University of Colorado Boulder
import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
-import { AlignGroup, Text } from '../../../../scenery/js/imports.js';
+import { AlignGroup, Color, Node, Rectangle, Text } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
@@ -11,7 +11,6 @@
import VariabilityPlotNode from './VariabilityPlotNode.js';
import InfoButton from '../../../../scenery-phet/js/buttons/InfoButton.js';
import VariabilityMeasure from '../model/VariabilityMeasure.js';
-import CAVConstants from '../../common/CAVConstants.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
import ToggleNode from '../../../../sun/js/ToggleNode.js';
@@ -19,6 +18,7 @@
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
export default class VariabilityAccordionBox extends CAVAccordionBox {
+ public readonly plotNode: ToggleNode<Node>;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
@@ -39,24 +39,10 @@
};
} );
- const accordionBoxContents = new ToggleNode( model.selectedSceneModelProperty, contents );
-
- const infoButton = new InfoButton( {
- iconFill: 'cornflowerblue',
- scale: 0.5,
- touchAreaDilation: 5,
- tandem: tandem.createTandem( 'infoButton' ),
- listener: () => {
- model.isInfoShowingProperty.value = true;
- },
-
- // TODO: How to position this properly? Can we use AlignBox?
- top: 20,
- right: accordionBoxContents.right - 10
- } );
- accordionBoxContents.addChild( infoButton );
-
const iconGroup = new AlignGroup();
+
+ const RECT_WIDTH = 900;
+ const RECT_HEIGHT = 150;
const checkboxToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
{
createNode: tandem => new VerticalCheckboxGroup( [
@@ -79,7 +65,37 @@
tandemName: 'madAccordionCheckboxGroup',
value: VariabilityMeasure.MAD
}
- ] );
+ ], {
+ right: RECT_WIDTH - 10,
+ centerY: RECT_HEIGHT / 2
+ } );
+
+ const plotNodes = new ToggleNode( model.selectedSceneModelProperty, contents, {
+ alignChildren: ToggleNode.NONE,
+ // excludeInvisibleChildrenFromBounds: true
+ // left: 0,
+ // top: 0
+ } );
+
+ const accordionBoxContents = new Rectangle( 0, 0, RECT_WIDTH, RECT_HEIGHT, {
+ fill: new Color( 0, 0, 255, 0.1 ),
+ children: [
+ // plotNodes,
+ new InfoButton( {
+ iconFill: 'cornflowerblue',
+ scale: 0.5,
+ touchAreaDilation: 5,
+ tandem: tandem.createTandem( 'infoButton' ),
+ listener: () => {
+ model.isInfoShowingProperty.value = true;
+ },
+
+ top: 0,
+ right: RECT_WIDTH - 10
+ } ),
+ checkboxToggleNode
+ ]
+ } );
super( model.resetEmitter, accordionBoxContents,
new Text( accordionBoxTitleProperty, {
@@ -87,13 +103,19 @@
maxWidth: 300
} ),
layoutBounds,
- checkboxToggleNode,
+ // checkboxToggleNode,
{
- leftMargin: 70,
+ // leftMargin: 70,
tandem: tandem,
top: top,
- right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
+ right: layoutBounds.right
} );
+
+ // ManualConstraint.create( this, [ infoButton, this.expandedTitleBar ], ( infoButtonWrapper, expandedTitleBarWrapper ) => {
+ // infoButtonWrapper.rightTop = expandedTitleBarWrapper.rightTop;
+ // } );
+
+ this.plotNode = plotNodes;
}
}
Index: main/center-and-variability/js/common/view/CAVAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/view/CAVAccordionBox.ts b/main/center-and-variability/js/common/view/CAVAccordionBox.ts
--- a/main/center-and-variability/js/common/view/CAVAccordionBox.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/common/view/CAVAccordionBox.ts (date 1683207764659)
@@ -9,17 +9,15 @@
import Bounds2 from '../../../../dot/js/Bounds2.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import { AlignBox, Node, Rectangle } from '../../../../scenery/js/imports.js';
+import { Node } 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';
import centerAndVariability from '../../centerAndVariability.js';
-import { Shape } from '../../../../kite/js/imports.js';
import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
import TEmitter from '../../../../axon/js/TEmitter.js';
type SelfOptions = {
- leftMargin: number;
+ // leftMargin: number;
};
export type CAVAccordionBoxOptions =
SelfOptions
@@ -32,11 +30,11 @@
export default class CAVAccordionBox extends AccordionBox {
- public readonly plotNode: Node;
+ // public readonly plotNode: Node;
// NOTE: The positions of the passed-in nodes are modified directly, so they cannot be used in the scenery DAG
- public constructor( resetEmitter: TEmitter, plotNode: Node, titleNode: Node,
- layoutBounds: Bounds2, checkboxGroup: Node, providedOptions: CAVAccordionBoxOptions ) {
+ public constructor( resetEmitter: TEmitter, contentNode: Node, titleNode: Node,
+ layoutBounds: Bounds2, providedOptions: CAVAccordionBoxOptions ) {
const options = optionize<CAVAccordionBoxOptions, SelfOptions, AccordionBoxOptions>()( {
titleAlignX: 'left',
@@ -59,40 +57,43 @@
titleNode: titleNode
}, providedOptions );
- const backgroundNode = new Rectangle( {
- rectHeight: 140,
- rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
- } );
+ // const backgroundNode = new Rectangle( {
+ // rectHeight: 140,
+ // rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
+ // } );
// Since the title is visible while the accordion box is open, this background will not any area above the bottom of
// the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
// Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
// horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
- const fullBackgroundBounds =
- backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
-
- // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
- backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
+ // const fullBackgroundBounds =
+ // backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
+ //
+ // // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
+ // backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
// Vertical positioning
- plotNode.centerY = fullBackgroundBounds.centerY;
- if ( plotNode.bottom > fullBackgroundBounds.bottom - 5 ) {
- plotNode.bottom = fullBackgroundBounds.bottom - 5;
- }
- backgroundNode.addChild( plotNode );
+ // contentNode.centerY = fullBackgroundBounds.centerY;
+ // if ( contentNode.bottom > fullBackgroundBounds.bottom - 5 ) {
+ // contentNode.bottom = fullBackgroundBounds.bottom - 5;
+ // }
+ // backgroundNode.addChild( contentNode );
- const checkboxAlignBox = new AlignBox( checkboxGroup,
- { xAlign: 'right', yAlign: 'center', rightMargin: 20, alignBounds: fullBackgroundBounds } );
+ // const checkboxAlignBox = new AlignBox( checkboxGroup, {
+ // xAlign: 'right',
+ // yAlign: 'center',
+ // rightMargin: 20
+ // alignBounds: fullBackgroundBounds
+ // } );
- backgroundNode.addChild( checkboxAlignBox );
+ // backgroundNode.addChild( checkboxAlignBox );
- super( backgroundNode, options );
+ super( contentNode, options );
resetEmitter.addListener( () => this.reset() );
- this.plotNode = plotNode;
+ this.plotNode = contentNode;
}
-
}
centerAndVariability.register( 'CAVAccordionBox', CAVAccordionBox );
\ No newline at end of file
Index: main/center-and-variability/package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/package.json b/main/center-and-variability/package.json
--- a/main/center-and-variability/package.json (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/package.json (date 1683233984249)
@@ -33,7 +33,7 @@
"eslintConfig": {
"extends": "../chipper/eslint/sim_eslintrc.js",
"rules": {
- "todo-should-have-issue": "off"
+ "todo-should-have-issue": "error"
}
}
}
\ No newline at end of file
Index: main/center-and-variability/js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/view/CAVScreenView.ts b/main/center-and-variability/js/common/view/CAVScreenView.ts
--- a/main/center-and-variability/js/common/view/CAVScreenView.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/common/view/CAVScreenView.ts (date 1683210635625)
@@ -31,6 +31,9 @@
import SceneView from './SceneView.js';
import KickButtonGroup from './KickButtonGroup.js';
import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
+import CAVPlotNode from './CAVPlotNode.js';
+import MeanAndMedianAccordionBox from '../../mean-and-median/view/MeanAndMedianAccordionBox.js';
+import VariabilityAccordionBox from '../../variability/view/VariabilityAccordionBox.js';
type SelfOptions = {
questionBarOptions: QuestionBarOptions;
@@ -189,12 +192,12 @@
* -Same offset and scale
* Given those assumptions, this code moves the dot plot so that its number line matches the play area one.
*/
- protected setAccordionBoxWithAlignedContent( accordionBox: CAVAccordionBox ): void {
+ protected setAccordionBoxWithAlignedContent( accordionBox: MeanAndMedianAccordionBox | VariabilityAccordionBox ): void {
this.setAccordionBox( accordionBox );
- ManualConstraint.create( this, [ this.playAreaNumberLineNode, accordionBox.plotNode ],
- ( lowerNumberLineWrapper, contentsWrapper ) => {
- contentsWrapper.x = lowerNumberLineWrapper.x;
- } );
+ // ManualConstraint.create( this, [ this.playAreaNumberLineNode, accordionBox.plotNode ],
+ // ( playAreaNumberLine, plotNode ) => {
+ // // plotNode.x = playAreaNumberLine.x;
+ // } );
}
protected setBottomCheckboxGroup( items: VerticalCheckboxGroupItem[] ): void {
Index: main/center-and-variability/js/common/model/CAVModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/model/CAVModel.ts b/main/center-and-variability/js/common/model/CAVModel.ts
--- a/main/center-and-variability/js/common/model/CAVModel.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/common/model/CAVModel.ts (date 1683234235915)
@@ -18,7 +18,7 @@
public readonly isShowingPlayAreaMedianProperty: BooleanProperty; // Screens 1-3
- // TODO: Should some of these should move to subclasses?
+ // TODO: Should some of these should move to subclasses? https://github.com/phetsims/center-and-variability/issues/153
public readonly isShowingPlayAreaMeanProperty: BooleanProperty; // Screens 2-3
public readonly isShowingMedianPredictionProperty: BooleanProperty; // Screens 1-2
public readonly medianPredictionProperty: NumberProperty; // Screens 1-2
Index: main/sun/js/ToggleNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ToggleNode.ts b/main/sun/js/ToggleNode.ts
--- a/main/sun/js/ToggleNode.ts (revision 8346e6d4343725b09c3556e51b66efc658e2a6a1)
+++ b/main/sun/js/ToggleNode.ts (date 1683204944669)
@@ -34,15 +34,6 @@
public constructor( valueProperty: TReadOnlyProperty<T>, elements: ToggleNodeElement<T>[], providedOptions?: ToggleNodeOptions ) {
- // assert && assert( Array.isArray( elements ), 'elements should be an array' );
- // if ( assert ) {
- // elements.forEach( element => {
- // const keys = _.keys( element );
- // assert && assert( keys.length === 2, 'each element should have two keys' );
- // assert && assert( keys[ 0 ] === 'value' || keys[ 1 ] === 'value', 'element should have a value key' );
- // } );
- // }
-
const options = optionize<ToggleNodeOptions, SelfOptions, NodeOptions>()( {
// SelfOptions
Index: main/center-and-variability/js/variability/view/VariabilityScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityScreenView.ts b/main/center-and-variability/js/variability/view/VariabilityScreenView.ts
--- a/main/center-and-variability/js/variability/view/VariabilityScreenView.ts (revision c090d08e0531b8f980cb38f51954317cdcfad013)
+++ b/main/center-and-variability/js/variability/view/VariabilityScreenView.ts (date 1683211773992)
@@ -45,7 +45,17 @@
super( model, options );
- this.setAccordionBoxWithAlignedContent( new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN ) );
+ const variabilityAccordionBox = new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN );
+ this.setAccordionBox( variabilityAccordionBox );
+
+ // variabilityAccordionBox.plotNode.x = this.playAreaNumberLineNode.x;
+ this.addChild( variabilityAccordionBox.plotNode );
+
+ ManualConstraint.create( this, [ this.playAreaNumberLineNode, variabilityAccordionBox.plotNode, variabilityAccordionBox ],
+ ( playAreaNumberLine, plotNode, variabilityAccordionBox ) => {
+ plotNode.x = playAreaNumberLine.x;
+ plotNode.bottom = variabilityAccordionBox.bottom;
+ } );
ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBox! ],
( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => {
Index: main/sun/js/AccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/AccordionBox.ts b/main/sun/js/AccordionBox.ts
--- a/main/sun/js/AccordionBox.ts (revision 8346e6d4343725b09c3556e51b66efc658e2a6a1)
+++ b/main/sun/js/AccordionBox.ts (date 1683206150982)
@@ -105,7 +105,7 @@
private readonly expandedBox: Rectangle;
private readonly collapsedBox: Rectangle;
private readonly workaroundBox: Rectangle;
- private readonly expandedTitleBar: InteractiveHighlightPath;
+ protected readonly expandedTitleBar: InteractiveHighlightPath;
private readonly collapsedTitleBar: InteractiveHighlightRectangle;
private readonly containerNode: Node;
private readonly resetAccordionBox: () => void;
|
|
From related issue #166
backgroundNode.localBounds = backgroundNode.localBounds.copy();
|
|
This patch is doing much better: Subject: [PATCH] Fix reset, see https://github.com/phetsims/center-and-variability/issues/172
---
Index: main/center-and-variability/js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/RangeNode.ts b/main/center-and-variability/js/variability/view/RangeNode.ts
--- a/main/center-and-variability/js/variability/view/RangeNode.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/RangeNode.ts (date 1683397787912)
@@ -36,22 +36,22 @@
...options
} );
- if ( options.parentContext === 'accordion' ) {
- const rangeReadoutValueProperty = new DerivedProperty( [ sceneModel.rangeValueProperty ], rangeValue => {
- return rangeValue ? `${rangeValue}` : '?';
- } );
-
- const rangeReadoutText = new VariabilityReadoutText( rangeReadoutValueProperty,
- CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
- fill: CAVColors.meanColorProperty,
- visibleProperty: model.isShowingRangeProperty,
- right: this.left,
- y: this.centerY,
- tandem: options.tandem.createTandem( 'rangeReadoutText' )
- } );
-
- this.addChild( rangeReadoutText );
- }
+ // if ( options.parentContext === 'accordion' ) {
+ // const rangeReadoutValueProperty = new DerivedProperty( [ sceneModel.rangeValueProperty ], rangeValue => {
+ // return rangeValue ? `${rangeValue}` : '?';
+ // } );
+ //
+ // // const rangeReadoutText = new VariabilityReadoutText( rangeReadoutValueProperty,
+ // // CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
+ // // fill: CAVColors.meanColorProperty,
+ // // visibleProperty: model.isShowingRangeProperty,
+ // // right: this.left,
+ // // y: this.centerY,
+ // // tandem: options.tandem.createTandem( 'rangeReadoutText' )
+ // // } );
+ // //
+ // // this.addChild( rangeReadoutText );
+ // }
const needAtLeastOneKickText = new Text( CenterAndVariabilityStrings.needAtLeastOneKickStringProperty, {
fontSize: 18,
Index: main/center-and-variability/js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/MADNode.ts b/main/center-and-variability/js/variability/view/MADNode.ts
--- a/main/center-and-variability/js/variability/view/MADNode.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/MADNode.ts (date 1683397787918)
@@ -30,22 +30,22 @@
...options
} );
- if ( options.parentContext === 'accordion' ) {
- const madReadoutValueProperty = new DerivedProperty( [ sceneModel.meanValueProperty ], meanValue => {
- return meanValue ? `${meanValue}` : '?';
- } );
-
- const madReadoutText = new VariabilityReadoutText( madReadoutValueProperty,
- CenterAndVariabilityStrings.meanEqualsValuePatternStringProperty, {
- fill: CAVColors.meanColorProperty,
- visibleProperty: model.isShowingMADProperty,
- right: this.left,
- y: this.centerY,
- tandem: options.tandem.createTandem( 'rangeReadoutText' )
- } );
-
- this.addChild( madReadoutText );
- }
+ // if ( options.parentContext === 'accordion' ) {
+ // const madReadoutValueProperty = new DerivedProperty( [ sceneModel.meanValueProperty ], meanValue => {
+ // return meanValue ? `${meanValue}` : '?';
+ // } );
+ //
+ // const madReadoutText = new VariabilityReadoutText( madReadoutValueProperty,
+ // CenterAndVariabilityStrings.meanEqualsValuePatternStringProperty, {
+ // fill: CAVColors.meanColorProperty,
+ // visibleProperty: model.isShowingMADProperty,
+ // right: this.left,
+ // y: this.centerY,
+ // tandem: options.tandem.createTandem( 'rangeReadoutText' )
+ // } );
+ //
+ // this.addChild( madReadoutText );
+ // }
const needAtLeastOneKickText = new Text( CenterAndVariabilityStrings.needAtLeastOneKickStringProperty, {
Index: main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
--- a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (date 1683398144941)
@@ -1,9 +1,7 @@
// Copyright 2023, University of Colorado Boulder
-import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
-import { AlignGroup, Text } from '../../../../scenery/js/imports.js';
+import { AlignBox, AlignGroup, Color, Rectangle, Text, Node } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
-import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import centerAndVariability from '../../centerAndVariability.js';
@@ -11,17 +9,35 @@
import VariabilityPlotNode from './VariabilityPlotNode.js';
import InfoButton from '../../../../scenery-phet/js/buttons/InfoButton.js';
import VariabilityMeasure from '../model/VariabilityMeasure.js';
-import CAVConstants from '../../common/CAVConstants.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
import ToggleNode from '../../../../sun/js/ToggleNode.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
+import AccordionBox from '../../../../sun/js/AccordionBox.js';
+import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
+import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
+import CAVSceneModel from '../../common/model/CAVSceneModel.js';
+import { Shape } from '../../../../kite/js/imports.js';
-export default class VariabilityAccordionBox extends CAVAccordionBox {
+// constants
+const CONTENT_MARGIN = 10;
+const BUTTON_SIDE_LENGTH = 20;
+
+export default class VariabilityAccordionBox extends AccordionBox {
+ private plotNode: Node;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
+ const backgroundNode = new Rectangle( {
+ fill: new Color( 0, 0, 255, 0.1 ),
+ // opacity: 0.1,
+ rectHeight: 140,
+ cornerRadius: 10,
+ rectWidth: 900
+ } );
+
+ backgroundNode.clipArea = Shape.bounds( backgroundNode.localBounds.copy() );
+
const currentProperty = new DerivedProperty( [ model.selectedVariabilityProperty ], selectedVariability =>
selectedVariability === VariabilityMeasure.RANGE ? CenterAndVariabilityStrings.rangeStringProperty :
selectedVariability === VariabilityMeasure.IQR ? CenterAndVariabilityStrings.interquartileRangeIQRStringProperty :
@@ -39,7 +55,7 @@
};
} );
- const accordionBoxContents = new ToggleNode( model.selectedSceneModelProperty, contents );
+ const plotToggleNode = new ToggleNode( model.selectedSceneModelProperty, contents );
const infoButton = new InfoButton( {
iconFill: 'cornflowerblue',
@@ -51,14 +67,16 @@
},
// TODO: How to position this properly? Can we use AlignBox? See https://github.com/phetsims/center-and-variability/issues/170
- top: 20,
- right: accordionBoxContents.right - 10
+ top: 0,
+ right: backgroundNode.right
} );
- accordionBoxContents.addChild( infoButton );
+ backgroundNode.addChild( infoButton );
const iconGroup = new AlignGroup();
const checkboxToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
{
+
+ // TODO: Why should these be "group"?
createNode: tandem => new VerticalCheckboxGroup( [
TopRepresentationCheckboxGroup.getRangeCheckboxWithIconItem( iconGroup, model.isShowingRangeProperty )
], { tandem: tandem.createTandem( 'rangeAccordionCheckboxGroup' ) } ),
@@ -79,21 +97,84 @@
tandemName: 'madAccordionCheckboxGroup',
value: VariabilityMeasure.MAD
}
- ] );
+ ], {
+ rightCenter: backgroundNode.rightCenter,
+ alignChildren: ToggleNode.LEFT
+ } );
+
+ // Since the title is visible while the accordion box is open, this background will not any area above the bottom of
+ // the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
+ // Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
+ // horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
+ // const fullBackgroundBounds =
+ // backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
- super( model.resetEmitter, accordionBoxContents,
- new Text( accordionBoxTitleProperty, {
+ // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
+ // backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
+
+ // Vertical positioning
+ // plotNode.centerY = fullBackgroundBounds.centerY;
+ // if ( plotNode.bottom > fullBackgroundBounds.bottom - 5 ) {
+ // plotNode.bottom = fullBackgroundBounds.bottom - 5;
+ // }
+ plotToggleNode.bottom = backgroundNode.height;
+ plotToggleNode.left = backgroundNode.left;
+ backgroundNode.addChild( plotToggleNode );
+
+ // const checkboxAlignBox = new AlignBox( checkboxToggleNode, {
+ // xAlign: 'right',
+ // yAlign: 'center',
+ // rightMargin: 20
+ // } );
+
+ backgroundNode.addChild( checkboxToggleNode );
+
+ super( backgroundNode, {
+ titleAlignX: 'left',
+ titleXSpacing: 8,
+ cornerRadius: 6,
+ titleYMargin: CONTENT_MARGIN,
+ buttonXMargin: CONTENT_MARGIN,
+ buttonYMargin: CONTENT_MARGIN,
+ contentXMargin: CONTENT_MARGIN,
+ contentYMargin: 0,
+ contentYSpacing: 0,
+ contentAlign: 'left',
+ expandCollapseButtonOptions: {
+ sideLength: BUTTON_SIDE_LENGTH
+ },
+ // TODO: This is currently highlighting a layout issues with AccordionBox, see: https://github.com/phetsims/center-and-variability/issues/170
+ titleBarOptions: {
+ stroke: 'black',
+ opacity: 0.1
+ },
+ tandem: tandem,
+ titleNode: new Text( accordionBoxTitleProperty, {
font: new PhetFont( 16 ),
maxWidth: 300
} ),
- layoutBounds,
- checkboxToggleNode,
- {
- leftMargin: 70,
- tandem: tandem,
- top: top,
- right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
- } );
+ left: 100
+ } );
+
+ // resetEmitter.addListener( () => this.reset() );
+
+ // this.plotToggleNode = plotToggleNode;
+
+ // super( model.resetEmitter, plotToggleNode,
+ // new Text( accordionBoxTitleProperty, {
+ // font: new PhetFont( 16 ),
+ // maxWidth: 300
+ // } ),
+ // layoutBounds,
+ // checkboxToggleNode,
+ // {
+ // leftMargin: 70,
+ // tandem: tandem,
+ // top: top,
+ // right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
+ // } );
+
+ this.plotNode = plotToggleNode;
}
}
Index: main/center-and-variability/js/common/view/CAVAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/view/CAVAccordionBox.ts b/main/center-and-variability/js/common/view/CAVAccordionBox.ts
--- a/main/center-and-variability/js/common/view/CAVAccordionBox.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/common/view/CAVAccordionBox.ts (date 1683396210365)
@@ -32,7 +32,7 @@
export default class CAVAccordionBox extends AccordionBox {
- public readonly plotNode: Node;
+ // public readonly plotNode: Node;
// NOTE: The positions of the passed-in nodes are modified directly, so they cannot be used in the scenery DAG
public constructor( resetEmitter: TEmitter, plotNode: Node, titleNode: Node,
@@ -54,43 +54,14 @@
},
// TODO: This is currently highlighting a layout issues with AccordionBox, see: https://github.com/phetsims/center-and-variability/issues/170
titleBarOptions: {
- stroke: 'black'
+ stroke: 'black',
+ opacity: 0.1
},
titleNode: titleNode
}, providedOptions );
- const backgroundNode = new Rectangle( {
- rectHeight: 140,
- rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
- } );
-
- // Since the title is visible while the accordion box is open, this background will not any area above the bottom of
- // the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
- // Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
- // horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
- const fullBackgroundBounds =
- backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
-
- // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
- backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
-
- // Vertical positioning
- plotNode.centerY = fullBackgroundBounds.centerY;
- if ( plotNode.bottom > fullBackgroundBounds.bottom - 5 ) {
- plotNode.bottom = fullBackgroundBounds.bottom - 5;
- }
- backgroundNode.addChild( plotNode );
+ super()
- const checkboxAlignBox = new AlignBox( checkboxGroup,
- { xAlign: 'right', yAlign: 'center', rightMargin: 20, alignBounds: fullBackgroundBounds } );
-
- backgroundNode.addChild( checkboxAlignBox );
-
- super( backgroundNode, options );
-
- resetEmitter.addListener( () => this.reset() );
-
- this.plotNode = plotNode;
}
}
Index: main/center-and-variability/js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/IQRNode.ts b/main/center-and-variability/js/variability/view/IQRNode.ts
--- a/main/center-and-variability/js/variability/view/IQRNode.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/IQRNode.ts (date 1683397787906)
@@ -30,37 +30,37 @@
...options
} );
- if ( providedOptions.parentContext === 'accordion' ) {
-
- const textReadoutGroup = new VBox( {
- x: -110,
- y: this.centerY - 20,
- align: 'left',
- spacing: 10
- } );
- this.addChild( textReadoutGroup );
-
- const medianReadoutValueProperty = new DerivedProperty( [ sceneModel.numberOfDataPointsProperty, sceneModel.medianValueProperty ],
- numberOfDataPoints => {
- return numberOfDataPoints >= 1 ? `${sceneModel.medianValueProperty.value}` : '?';
- } );
- const iqrReadoutValueProperty = new DerivedProperty( [ sceneModel.iqrValueProperty ], iqrValue => {
- return iqrValue ? `${iqrValue}` : '?';
- } );
-
- const medianReadoutText = new VariabilityReadoutText( medianReadoutValueProperty, CenterAndVariabilityStrings.medianEqualsValuePatternStringProperty, {
- fill: CAVColors.medianColorProperty,
- tandem: options.tandem.createTandem( 'medianReadoutText' )
- } );
- const iqrReadoutText = new VariabilityReadoutText( iqrReadoutValueProperty, CenterAndVariabilityStrings.iqrEqualsValuePatternStringProperty, {
- fill: CAVColors.iqrColorProperty,
- visibleProperty: model.isShowingIQRProperty,
- tandem: options.tandem.createTandem( 'iqrReadoutText' )
- } );
-
- textReadoutGroup.addChild( medianReadoutText );
- textReadoutGroup.addChild( iqrReadoutText );
- }
+ // if ( providedOptions.parentContext === 'accordion' ) {
+ //
+ // const textReadoutGroup = new VBox( {
+ // x: -110,
+ // y: this.centerY - 20,
+ // align: 'left',
+ // spacing: 10
+ // } );
+ // this.addChild( textReadoutGroup );
+ //
+ // const medianReadoutValueProperty = new DerivedProperty( [ sceneModel.numberOfDataPointsProperty, sceneModel.medianValueProperty ],
+ // numberOfDataPoints => {
+ // return numberOfDataPoints >= 1 ? `${sceneModel.medianValueProperty.value}` : '?';
+ // } );
+ // const iqrReadoutValueProperty = new DerivedProperty( [ sceneModel.iqrValueProperty ], iqrValue => {
+ // return iqrValue ? `${iqrValue}` : '?';
+ // } );
+ //
+ // const medianReadoutText = new VariabilityReadoutText( medianReadoutValueProperty, CenterAndVariabilityStrings.medianEqualsValuePatternStringProperty, {
+ // fill: CAVColors.medianColorProperty,
+ // tandem: options.tandem.createTandem( 'medianReadoutText' )
+ // } );
+ // const iqrReadoutText = new VariabilityReadoutText( iqrReadoutValueProperty, CenterAndVariabilityStrings.iqrEqualsValuePatternStringProperty, {
+ // fill: CAVColors.iqrColorProperty,
+ // visibleProperty: model.isShowingIQRProperty,
+ // tandem: options.tandem.createTandem( 'iqrReadoutText' )
+ // } );
+ //
+ // textReadoutGroup.addChild( medianReadoutText );
+ // textReadoutGroup.addChild( iqrReadoutText );
+ // }
const needAtLeastFiveKicksOffsetY = options.parentContext === 'info' ? 90 : 20;
const needAtLeastFiveKicks = new Text( CenterAndVariabilityStrings.needAtLeastFiveKicksStringProperty, {
Index: main/sun/js/AccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/AccordionBox.ts b/main/sun/js/AccordionBox.ts
--- a/main/sun/js/AccordionBox.ts (revision 8346e6d4343725b09c3556e51b66efc658e2a6a1)
+++ b/main/sun/js/AccordionBox.ts (date 1683395710932)
@@ -198,7 +198,9 @@
options.titleBarOptions = combineOptions<RectangleOptions>( {
fill: null, // {Color|string|null} title bar fill
stroke: null // {Color|string|null} title bar stroke, used only for the expanded title bar
- }, options.titleBarOptions );
+ }, options.titleBarOptions, {
+ fill: null
+ } );
// expandCollapseButtonOptions defaults
options.expandCollapseButtonOptions = combineOptions<ExpandCollapseButtonOptions>( {
@@ -577,7 +579,7 @@
* expand/collapse button.
*/
private getTitleBarShape(): Shape {
- return Shape.roundedRectangleWithRadii( 0, 0, this.getBoxWidth(), this.getCollapsedBoxHeight(), {
+ return Shape.roundedRectangleWithRadii( 0, 0, this.getBoxWidth() - 100, this.getCollapsedBoxHeight(), {
topLeft: this._cornerRadius,
topRight: this._cornerRadius
} );
|
|
OK this patch is at a good point to review and discuss:
Problems:
Subject: [PATCH] Fix reset, see https://github.com/phetsims/center-and-variability/issues/172
---
Index: main/center-and-variability/js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/RangeNode.ts b/main/center-and-variability/js/variability/view/RangeNode.ts
--- a/main/center-and-variability/js/variability/view/RangeNode.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/RangeNode.ts (date 1683400892745)
@@ -36,23 +36,6 @@
...options
} );
- if ( options.parentContext === 'accordion' ) {
- const rangeReadoutValueProperty = new DerivedProperty( [ sceneModel.rangeValueProperty ], rangeValue => {
- return rangeValue ? `${rangeValue}` : '?';
- } );
-
- const rangeReadoutText = new VariabilityReadoutText( rangeReadoutValueProperty,
- CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
- fill: CAVColors.meanColorProperty,
- visibleProperty: model.isShowingRangeProperty,
- right: this.left,
- y: this.centerY,
- tandem: options.tandem.createTandem( 'rangeReadoutText' )
- } );
-
- this.addChild( rangeReadoutText );
- }
-
const needAtLeastOneKickText = new Text( CenterAndVariabilityStrings.needAtLeastOneKickStringProperty, {
fontSize: 18,
top: 100,
Index: main/center-and-variability/js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/MADNode.ts b/main/center-and-variability/js/variability/view/MADNode.ts
--- a/main/center-and-variability/js/variability/view/MADNode.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/MADNode.ts (date 1683397787918)
@@ -30,22 +30,22 @@
...options
} );
- if ( options.parentContext === 'accordion' ) {
- const madReadoutValueProperty = new DerivedProperty( [ sceneModel.meanValueProperty ], meanValue => {
- return meanValue ? `${meanValue}` : '?';
- } );
-
- const madReadoutText = new VariabilityReadoutText( madReadoutValueProperty,
- CenterAndVariabilityStrings.meanEqualsValuePatternStringProperty, {
- fill: CAVColors.meanColorProperty,
- visibleProperty: model.isShowingMADProperty,
- right: this.left,
- y: this.centerY,
- tandem: options.tandem.createTandem( 'rangeReadoutText' )
- } );
-
- this.addChild( madReadoutText );
- }
+ // if ( options.parentContext === 'accordion' ) {
+ // const madReadoutValueProperty = new DerivedProperty( [ sceneModel.meanValueProperty ], meanValue => {
+ // return meanValue ? `${meanValue}` : '?';
+ // } );
+ //
+ // const madReadoutText = new VariabilityReadoutText( madReadoutValueProperty,
+ // CenterAndVariabilityStrings.meanEqualsValuePatternStringProperty, {
+ // fill: CAVColors.meanColorProperty,
+ // visibleProperty: model.isShowingMADProperty,
+ // right: this.left,
+ // y: this.centerY,
+ // tandem: options.tandem.createTandem( 'rangeReadoutText' )
+ // } );
+ //
+ // this.addChild( madReadoutText );
+ // }
const needAtLeastOneKickText = new Text( CenterAndVariabilityStrings.needAtLeastOneKickStringProperty, {
Index: main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
--- a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (date 1683400962952)
@@ -1,9 +1,7 @@
// Copyright 2023, University of Colorado Boulder
-import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
-import { AlignGroup, Text } from '../../../../scenery/js/imports.js';
+import { AlignGroup, Color, Node, Rectangle, Text } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
-import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import centerAndVariability from '../../centerAndVariability.js';
@@ -11,17 +9,36 @@
import VariabilityPlotNode from './VariabilityPlotNode.js';
import InfoButton from '../../../../scenery-phet/js/buttons/InfoButton.js';
import VariabilityMeasure from '../model/VariabilityMeasure.js';
-import CAVConstants from '../../common/CAVConstants.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
import ToggleNode from '../../../../sun/js/ToggleNode.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
+import AccordionBox from '../../../../sun/js/AccordionBox.js';
+import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
+import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
+import { Shape } from '../../../../kite/js/imports.js';
+import VariabilityReadoutText from './VariabilityReadoutText.js';
+import CAVColors from '../../common/CAVColors.js';
-export default class VariabilityAccordionBox extends CAVAccordionBox {
+// constants
+const CONTENT_MARGIN = 10;
+const BUTTON_SIDE_LENGTH = 20;
+
+export default class VariabilityAccordionBox extends AccordionBox {
+ private plotNode: Node;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
+ const backgroundNode = new Rectangle( {
+ fill: new Color( 0, 0, 255, 0.1 ),
+ // opacity: 0.1,
+ rectHeight: 200,
+ cornerRadius: 10,
+ rectWidth: 900
+ } );
+
+ backgroundNode.clipArea = Shape.bounds( backgroundNode.localBounds.copy() );
+
const currentProperty = new DerivedProperty( [ model.selectedVariabilityProperty ], selectedVariability =>
selectedVariability === VariabilityMeasure.RANGE ? CenterAndVariabilityStrings.rangeStringProperty :
selectedVariability === VariabilityMeasure.IQR ? CenterAndVariabilityStrings.interquartileRangeIQRStringProperty :
@@ -39,7 +56,7 @@
};
} );
- const accordionBoxContents = new ToggleNode( model.selectedSceneModelProperty, contents );
+ const plotToggleNode = new ToggleNode( model.selectedSceneModelProperty, contents );
const infoButton = new InfoButton( {
iconFill: 'cornflowerblue',
@@ -51,14 +68,16 @@
},
// TODO: How to position this properly? Can we use AlignBox? See https://github.com/phetsims/center-and-variability/issues/170
- top: 20,
- right: accordionBoxContents.right - 10
+ top: 0,
+ right: backgroundNode.right
} );
- accordionBoxContents.addChild( infoButton );
+ backgroundNode.addChild( infoButton );
const iconGroup = new AlignGroup();
const checkboxToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
{
+
+ // TODO: Why should these be "group"?
createNode: tandem => new VerticalCheckboxGroup( [
TopRepresentationCheckboxGroup.getRangeCheckboxWithIconItem( iconGroup, model.isShowingRangeProperty )
], { tandem: tandem.createTandem( 'rangeAccordionCheckboxGroup' ) } ),
@@ -79,21 +98,89 @@
tandemName: 'madAccordionCheckboxGroup',
value: VariabilityMeasure.MAD
}
- ] );
+ ], {
+ rightCenter: backgroundNode.rightCenter,
+ alignChildren: ToggleNode.LEFT
+ } );
+
+ // Since the title is visible while the accordion box is open, this background will not any area above the bottom of
+ // the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
+ // Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
+ // horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
+ // const fullBackgroundBounds =
+ // backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
+
+ // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
+ // backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
+
+ plotToggleNode.bottom = backgroundNode.height;
- super( model.resetEmitter, accordionBoxContents,
- new Text( accordionBoxTitleProperty, {
+ // Will later be centered by a ManualConstraint to align with the one in the play area
+ plotToggleNode.left = backgroundNode.left;
+ backgroundNode.addChild( plotToggleNode );
+ backgroundNode.addChild( checkboxToggleNode );
+
+ const rangeReadoutValueProperty = new DerivedProperty( [ model.variabilitySceneModels[ 0 ].rangeValueProperty ], rangeValue => {
+ return rangeValue ? `${rangeValue}` : '?';
+ } );
+
+ const rangeReadoutText = new VariabilityReadoutText( rangeReadoutValueProperty,
+ CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
+ fill: CAVColors.meanColorProperty,
+ visibleProperty: model.isShowingRangeProperty,
+ left: 0,
+ centerY: backgroundNode.height / 2,
+ tandem: tandem.createTandem( 'rangeReadoutText' )
+ } );
+
+ backgroundNode.addChild( rangeReadoutText );
+
+ super( backgroundNode, {
+ titleAlignX: 'left',
+ titleXSpacing: 8,
+ cornerRadius: 6,
+ titleYMargin: CONTENT_MARGIN,
+ buttonXMargin: CONTENT_MARGIN,
+ buttonYMargin: CONTENT_MARGIN,
+ contentXMargin: 0,
+ contentYMargin: 0,
+ contentYSpacing: 0,
+ contentAlign: 'left',
+ expandCollapseButtonOptions: {
+ sideLength: BUTTON_SIDE_LENGTH
+ },
+ // TODO: This is currently highlighting a layout issues with AccordionBox, see: https://github.com/phetsims/center-and-variability/issues/170
+ titleBarOptions: {
+ stroke: 'black',
+ opacity: 0.1
+ },
+ tandem: tandem,
+ titleNode: new Text( accordionBoxTitleProperty, {
font: new PhetFont( 16 ),
maxWidth: 300
} ),
- layoutBounds,
- checkboxToggleNode,
- {
- leftMargin: 70,
- tandem: tandem,
- top: top,
- right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
- } );
+ left: 100
+ } );
+
+ // resetEmitter.addListener( () => this.reset() );
+
+ // this.plotToggleNode = plotToggleNode;
+
+ // super( model.resetEmitter, plotToggleNode,
+ // new Text( accordionBoxTitleProperty, {
+ // font: new PhetFont( 16 ),
+ // maxWidth: 300
+ // } ),
+ // layoutBounds,
+ // checkboxToggleNode,
+ // {
+ // leftMargin: 70,
+ // tandem: tandem,
+ // top: top,
+ // right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
+ // } );
+
+ this.plotNode = plotToggleNode;
}
}
Index: main/center-and-variability/js/common/view/CAVAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/common/view/CAVAccordionBox.ts b/main/center-and-variability/js/common/view/CAVAccordionBox.ts
--- a/main/center-and-variability/js/common/view/CAVAccordionBox.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/common/view/CAVAccordionBox.ts (date 1683400755779)
@@ -9,12 +9,10 @@
import Bounds2 from '../../../../dot/js/Bounds2.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import { AlignBox, Node, Rectangle } from '../../../../scenery/js/imports.js';
+import { Node } 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';
import centerAndVariability from '../../centerAndVariability.js';
-import { Shape } from '../../../../kite/js/imports.js';
import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
import TEmitter from '../../../../axon/js/TEmitter.js';
@@ -32,7 +30,7 @@
export default class CAVAccordionBox extends AccordionBox {
- public readonly plotNode: Node;
+ // public readonly plotNode: Node;
// NOTE: The positions of the passed-in nodes are modified directly, so they cannot be used in the scenery DAG
public constructor( resetEmitter: TEmitter, plotNode: Node, titleNode: Node,
@@ -45,7 +43,7 @@
titleYMargin: CONTENT_MARGIN,
buttonXMargin: CONTENT_MARGIN,
buttonYMargin: CONTENT_MARGIN,
- contentXMargin: CONTENT_MARGIN,
+ contentXMargin: 0,
contentYMargin: 0,
contentYSpacing: 0,
contentAlign: 'left',
@@ -54,43 +52,14 @@
},
// TODO: This is currently highlighting a layout issues with AccordionBox, see: https://github.com/phetsims/center-and-variability/issues/170
titleBarOptions: {
- stroke: 'black'
+ stroke: 'black',
+ opacity: 0.1
},
titleNode: titleNode
}, providedOptions );
- const backgroundNode = new Rectangle( {
- rectHeight: 140,
- rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
- } );
-
- // Since the title is visible while the accordion box is open, this background will not any area above the bottom of
- // the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
- // Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
- // horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
- const fullBackgroundBounds =
- backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
-
- // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
- backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
-
- // Vertical positioning
- plotNode.centerY = fullBackgroundBounds.centerY;
- if ( plotNode.bottom > fullBackgroundBounds.bottom - 5 ) {
- plotNode.bottom = fullBackgroundBounds.bottom - 5;
- }
- backgroundNode.addChild( plotNode );
+ super()
- const checkboxAlignBox = new AlignBox( checkboxGroup,
- { xAlign: 'right', yAlign: 'center', rightMargin: 20, alignBounds: fullBackgroundBounds } );
-
- backgroundNode.addChild( checkboxAlignBox );
-
- super( backgroundNode, options );
-
- resetEmitter.addListener( () => this.reset() );
-
- this.plotNode = plotNode;
}
}
Index: main/center-and-variability/js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/IQRNode.ts b/main/center-and-variability/js/variability/view/IQRNode.ts
--- a/main/center-and-variability/js/variability/view/IQRNode.ts (revision a75baad9ee1f62bcc8ddc896fdd2057946a42884)
+++ b/main/center-and-variability/js/variability/view/IQRNode.ts (date 1683397787906)
@@ -30,37 +30,37 @@
...options
} );
- if ( providedOptions.parentContext === 'accordion' ) {
-
- const textReadoutGroup = new VBox( {
- x: -110,
- y: this.centerY - 20,
- align: 'left',
- spacing: 10
- } );
- this.addChild( textReadoutGroup );
-
- const medianReadoutValueProperty = new DerivedProperty( [ sceneModel.numberOfDataPointsProperty, sceneModel.medianValueProperty ],
- numberOfDataPoints => {
- return numberOfDataPoints >= 1 ? `${sceneModel.medianValueProperty.value}` : '?';
- } );
- const iqrReadoutValueProperty = new DerivedProperty( [ sceneModel.iqrValueProperty ], iqrValue => {
- return iqrValue ? `${iqrValue}` : '?';
- } );
-
- const medianReadoutText = new VariabilityReadoutText( medianReadoutValueProperty, CenterAndVariabilityStrings.medianEqualsValuePatternStringProperty, {
- fill: CAVColors.medianColorProperty,
- tandem: options.tandem.createTandem( 'medianReadoutText' )
- } );
- const iqrReadoutText = new VariabilityReadoutText( iqrReadoutValueProperty, CenterAndVariabilityStrings.iqrEqualsValuePatternStringProperty, {
- fill: CAVColors.iqrColorProperty,
- visibleProperty: model.isShowingIQRProperty,
- tandem: options.tandem.createTandem( 'iqrReadoutText' )
- } );
-
- textReadoutGroup.addChild( medianReadoutText );
- textReadoutGroup.addChild( iqrReadoutText );
- }
+ // if ( providedOptions.parentContext === 'accordion' ) {
+ //
+ // const textReadoutGroup = new VBox( {
+ // x: -110,
+ // y: this.centerY - 20,
+ // align: 'left',
+ // spacing: 10
+ // } );
+ // this.addChild( textReadoutGroup );
+ //
+ // const medianReadoutValueProperty = new DerivedProperty( [ sceneModel.numberOfDataPointsProperty, sceneModel.medianValueProperty ],
+ // numberOfDataPoints => {
+ // return numberOfDataPoints >= 1 ? `${sceneModel.medianValueProperty.value}` : '?';
+ // } );
+ // const iqrReadoutValueProperty = new DerivedProperty( [ sceneModel.iqrValueProperty ], iqrValue => {
+ // return iqrValue ? `${iqrValue}` : '?';
+ // } );
+ //
+ // const medianReadoutText = new VariabilityReadoutText( medianReadoutValueProperty, CenterAndVariabilityStrings.medianEqualsValuePatternStringProperty, {
+ // fill: CAVColors.medianColorProperty,
+ // tandem: options.tandem.createTandem( 'medianReadoutText' )
+ // } );
+ // const iqrReadoutText = new VariabilityReadoutText( iqrReadoutValueProperty, CenterAndVariabilityStrings.iqrEqualsValuePatternStringProperty, {
+ // fill: CAVColors.iqrColorProperty,
+ // visibleProperty: model.isShowingIQRProperty,
+ // tandem: options.tandem.createTandem( 'iqrReadoutText' )
+ // } );
+ //
+ // textReadoutGroup.addChild( medianReadoutText );
+ // textReadoutGroup.addChild( iqrReadoutText );
+ // }
const needAtLeastFiveKicksOffsetY = options.parentContext === 'info' ? 90 : 20;
const needAtLeastFiveKicks = new Text( CenterAndVariabilityStrings.needAtLeastFiveKicksStringProperty, {
Index: main/sun/js/AccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/AccordionBox.ts b/main/sun/js/AccordionBox.ts
--- a/main/sun/js/AccordionBox.ts (revision 8346e6d4343725b09c3556e51b66efc658e2a6a1)
+++ b/main/sun/js/AccordionBox.ts (date 1683400383898)
@@ -198,7 +198,9 @@
options.titleBarOptions = combineOptions<RectangleOptions>( {
fill: null, // {Color|string|null} title bar fill
stroke: null // {Color|string|null} title bar stroke, used only for the expanded title bar
- }, options.titleBarOptions );
+ }, options.titleBarOptions, {
+ fill: null
+ } );
// expandCollapseButtonOptions defaults
options.expandCollapseButtonOptions = combineOptions<ExpandCollapseButtonOptions>( {
@@ -577,7 +579,7 @@
* expand/collapse button.
*/
private getTitleBarShape(): Shape {
- return Shape.roundedRectangleWithRadii( 0, 0, this.getBoxWidth(), this.getCollapsedBoxHeight(), {
+ return Shape.roundedRectangleWithRadii( 0, 0, this.getBoxWidth() - 100, this.getCollapsedBoxHeight(), {
topLeft: this._cornerRadius,
topRight: this._cornerRadius
} );
@@ -624,7 +626,7 @@
public getExpandedBoxHeight(): number {
// content is below button+title
if ( this._showTitleWhenExpanded ) {
- return this.getCollapsedBoxHeight() + this._contentNode.height + this._contentYMargin + this._contentYSpacing;
+ return this._contentNode.height + this._contentYMargin + this._contentYSpacing;
}
// content is next to button
else {
|
This was referenced May 6, 2023
|
The info button can be below the title bar, but should be right aligned with the checkbox icons on the right. We reaffirmed that it is really important for the data points to go up into the title bar. |
|
I wrote in slack:
UPDATE: The related issue is phetsims/sun#803 |
|
In discussion, @jonathanolson and I discussed the design requests for this feature, and @jonathanolson said those features sound reasonable, and he hopes to work on it in the next several days. Thanks! @jonathanolson also commented that the "make a rectangle content node and layout items within that" strategy seems reasonable for this case. |
|
Great results from collaboration with @marlitas. We subtyped the AccordionBoxes, each subtype is adding the correct decorators. We have a better solution for layouts within the accordion box and for aligning the number lines. Still some type errors and TODOs: Subject: [PATCH] Update docs, see https://github.com/phetsims/center-and-variability/issues/189
---
Index: js/mean-and-median/view/MeanAndMedianScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MeanAndMedianScreenView.ts b/js/mean-and-median/view/MeanAndMedianScreenView.ts
--- a/js/mean-and-median/view/MeanAndMedianScreenView.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/mean-and-median/view/MeanAndMedianScreenView.ts (date 1683671713854)
@@ -22,6 +22,7 @@
import PredictionSlider from '../../common/view/PredictionSlider.js';
import Property from '../../../../axon/js/Property.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
+import Vector2 from '../../../../dot/js/Vector2.js';
type MeanAndMedianScreenViewOptions = StrictOmit<CAVScreenViewOptions, 'questionBarOptions'>;
@@ -38,7 +39,9 @@
super( model, options );
- this.setAccordionBoxWithAlignedContent( new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode ) );
+ const meanAndMedianAccordionBox = new MeanAndMedianAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN, this.playAreaNumberLineNode );
+ this.setAccordionBox( meanAndMedianAccordionBox );
+ meanAndMedianAccordionBox.alignWithPlayAreaNumberLineNode( this.playAreaNumberLineNode.globalBounds.x );
const iconGroup = new AlignGroup();
this.setBottomControls( new VerticalCheckboxGroup( [
Index: js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/MADNode.ts b/js/variability/view/MADNode.ts
--- a/js/variability/view/MADNode.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/view/MADNode.ts (date 1683674727454)
@@ -11,8 +11,6 @@
import CAVConstants from '../../common/CAVConstants.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import CAVColors from '../../common/CAVColors.js';
-import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import VariabilityReadoutText from './VariabilityReadoutText.js';
import VariabilitySceneModel from '../model/VariabilitySceneModel.js';
type SelfOptions = {
@@ -30,24 +28,6 @@
...options
} );
- if ( options.parentContext === 'accordion' ) {
- const madReadoutValueProperty = new DerivedProperty( [ sceneModel.meanValueProperty ], meanValue => {
- return meanValue ? `${meanValue}` : '?';
- } );
-
- const madReadoutText = new VariabilityReadoutText( madReadoutValueProperty,
- CenterAndVariabilityStrings.meanEqualsValuePatternStringProperty, {
- fill: CAVColors.meanColorProperty,
- visibleProperty: model.isShowingMADProperty,
- right: this.left,
- y: this.centerY,
- tandem: options.tandem.createTandem( 'rangeReadoutText' )
- } );
-
- this.addChild( madReadoutText );
- }
-
-
const needAtLeastOneKickText = new Text( CenterAndVariabilityStrings.needAtLeastOneKickStringProperty, {
fontSize: 18,
top: 100,
Index: js/mean-and-median/view/MeanAndMedianAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MeanAndMedianAccordionBox.ts b/js/mean-and-median/view/MeanAndMedianAccordionBox.ts
--- a/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (date 1683673502749)
@@ -1,7 +1,7 @@
// Copyright 2023, University of Colorado Boulder
import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
-import { AlignGroup, Node, Text } from '../../../../scenery/js/imports.js';
+import { AlignGroup, Color, Node, Path, Text } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
@@ -11,38 +11,61 @@
import MedianPlotNode from './MedianPlotNode.js';
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
+import CAVConstants from '../../common/CAVConstants.js';
+
+const MARGIN = 12.5;
export default class MeanAndMedianAccordionBox extends CAVAccordionBox {
+ private readonly medianPlotNode: MedianPlotNode;
public constructor( model: MeanAndMedianModel, layoutBounds: Bounds2, tandem: Tandem, top: number, playAreaNumberLineNode: Node ) {
const iconGroup = new AlignGroup();
+ const backgroundShape = CAVConstants.ACCORDION_BOX_CONTENTS_SHAPE_MEAN_AND_OR_MEDIAN;
+ const backgroundNode = new Path( backgroundShape, {
+ clipArea: backgroundShape,
+ fill: new Color( 255, 0, 0, 0.2 )
+ } );
+
+ // There is only one scene in the mean and median screen
+ const sceneModel = model.selectedSceneModelProperty.value;
+
+ // TODO: https://github.com/phetsims/center-and-variability/issues/170 Why is this called the MedianPlotNode?
+ const medianPlotNode = new MedianPlotNode( model, sceneModel, {
+ tandem: tandem.createTandem( 'plotNode' )
+ } ).mutate( {
+ bottom: backgroundNode.height
+ } );
+
const checkboxGroup = new VerticalCheckboxGroup( [
TopRepresentationCheckboxGroup.getMedianCheckboxWithIconItem( iconGroup, model.isShowingTopMedianProperty ),
TopRepresentationCheckboxGroup.getMeanCheckboxWithIconItem( iconGroup, model.isShowingTopMeanProperty )
], {
- tandem: tandem.createTandem( 'accordionCheckboxGroup' )
+ tandem: tandem.createTandem( 'accordionCheckboxGroup' ),
+ right: backgroundNode.width - MARGIN,
+ centerY: backgroundNode.height / 2
} );
- // There is only one scene in the mean and median screen
- const sceneModel = model.selectedSceneModelProperty.value;
+ backgroundNode.addChild( checkboxGroup );
+ backgroundNode.addChild( medianPlotNode );
- const plotNode = new MedianPlotNode( model, sceneModel, { tandem: tandem.createTandem( 'plotNode' ) } );
-
- super( sceneModel.resetEmitter, plotNode,
- new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, {
- font: new PhetFont( 16 ),
- maxWidth: 300
- } ),
- layoutBounds,
- checkboxGroup,
- {
+ super( backgroundNode, {
+ titleNode: new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, {
+ font: new PhetFont( 16 ),
+ maxWidth: 300
+ } ),
leftMargin: 0,
tandem: tandem,
top: top,
centerX: layoutBounds.centerX
}
);
+
+ this.medianPlotNode = medianPlotNode;
+ }
+
+ public alignWithPlayAreaNumberLineNode( x: number ): void {
+ this.medianPlotNode.alignWithPlayAreaNumberLineNode( x );
}
}
Index: js/mean-and-median/model/MeanAndMedianModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/model/MeanAndMedianModel.ts b/js/mean-and-median/model/MeanAndMedianModel.ts
--- a/js/mean-and-median/model/MeanAndMedianModel.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/mean-and-median/model/MeanAndMedianModel.ts (date 1683675223399)
@@ -18,7 +18,7 @@
// constants
const HIGHLIGHT_ANIMATION_TIME_STEP = 0.25; // in seconds
-export default class MeanAndMedianModel extends CAVModel {
+export default class MeanAndMedianModel extends CAVModel<CAVSceneModel> {
// Indicates how far the show median animation has progressed, or null if not animating. Not PhET-iO instrumented since
// it represents a transient value.
Index: js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/VariabilityAccordionBox.ts b/js/variability/view/VariabilityAccordionBox.ts
--- a/js/variability/view/VariabilityAccordionBox.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/view/VariabilityAccordionBox.ts (date 1683674815455)
@@ -1,9 +1,7 @@
// Copyright 2023, University of Colorado Boulder
-import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
-import { AlignGroup, Text } from '../../../../scenery/js/imports.js';
+import { AlignGroup, Color, Path, Text, VBox } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
-import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import Bounds2 from '../../../../dot/js/Bounds2.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import centerAndVariability from '../../centerAndVariability.js';
@@ -11,17 +9,30 @@
import VariabilityPlotNode from './VariabilityPlotNode.js';
import InfoButton from '../../../../scenery-phet/js/buttons/InfoButton.js';
import VariabilityMeasure from '../model/VariabilityMeasure.js';
-import CAVConstants from '../../common/CAVConstants.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
import ToggleNode from '../../../../sun/js/ToggleNode.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
+import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
+import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
+import VariabilityReadoutText from './VariabilityReadoutText.js';
+import CAVColors from '../../common/CAVColors.js';
+import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
+import CAVConstants from '../../common/CAVConstants.js';
+import CAVSceneModel from '../../common/model/CAVSceneModel.js';
export default class VariabilityAccordionBox extends CAVAccordionBox {
+ // private plotNode: Node;
+ private plotToggleNode: ToggleNode<CAVSceneModel | null>;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
+ const backgroundShape = CAVConstants.ACCORDION_BOX_CONTENTS_SHAPE_VARIABILITY;
+ const backgroundNode = new Path( backgroundShape, {
+ clipArea: backgroundShape,
+ fill: new Color( 255, 0, 0, 0.2 )
+ } );
+
const currentProperty = new DerivedProperty( [ model.selectedVariabilityProperty ], selectedVariability =>
selectedVariability === VariabilityMeasure.RANGE ? CenterAndVariabilityStrings.rangeStringProperty :
selectedVariability === VariabilityMeasure.IQR ? CenterAndVariabilityStrings.interquartileRangeIQRStringProperty :
@@ -34,12 +45,15 @@
return {
value: model.sceneModels[ i ],
createNode: ( tandem: Tandem ) => new VariabilityPlotNode( model, model.variabilitySceneModels[ i ], {
- tandem: tandem.createTandem( 'plotNode' + i )
+ tandem: tandem.createTandem( 'plotNode' + i ),
+ bottom: backgroundNode.height
} )
};
} );
- const accordionBoxContents = new ToggleNode( model.selectedSceneModelProperty, contents );
+ const plotToggleNode = new ToggleNode( model.selectedSceneModelProperty, contents, {
+ bottom: backgroundNode.height
+ } );
const infoButton = new InfoButton( {
iconFill: 'cornflowerblue',
@@ -51,14 +65,16 @@
},
// TODO: How to position this properly? Can we use AlignBox? See https://github.com/phetsims/center-and-variability/issues/170
- top: 20,
- right: accordionBoxContents.right - 10
+ top: 0,
+ right: backgroundNode.right
} );
- accordionBoxContents.addChild( infoButton );
+ backgroundNode.addChild( infoButton );
const iconGroup = new AlignGroup();
const checkboxToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
{
+
+ // TODO: Why should these be "group"? see https://github.com/phetsims/center-and-variability/issues/170
createNode: tandem => new VerticalCheckboxGroup( [
TopRepresentationCheckboxGroup.getRangeCheckboxWithIconItem( iconGroup, model.isShowingRangeProperty )
], { tandem: tandem.createTandem( 'rangeAccordionCheckboxGroup' ) } ),
@@ -79,21 +95,151 @@
tandemName: 'madAccordionCheckboxGroup',
value: VariabilityMeasure.MAD
}
- ] );
+ ], {
+ rightCenter: backgroundNode.rightCenter,
+ alignChildren: ToggleNode.LEFT
+ } );
+
+ // Since the title is visible while the accordion box is open, this background will not any area above the bottom of
+ // the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
+ // Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
+ // horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
+ // const fullBackgroundBounds =
+ // backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
+
+ // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
+ // backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
+
+ // plotToggleNode.bottom = backgroundNode.height;
+
+ // Will later be centered by a ManualConstraint to align with the one in the play area
+ // plotToggleNode.left = backgroundNode.left;
+ backgroundNode.addChild( plotToggleNode );
+ backgroundNode.addChild( checkboxToggleNode );
+
+ // const rangeReadoutValueProperty = new DerivedProperty( [ model.variabilitySceneModels[ 0 ].rangeValueProperty ], rangeValue => {
+ // return rangeValue ? `${rangeValue}` : '?';
+ // } );
+
+ const rangeValueProperty = DerivedProperty.deriveAny( [ model.selectedSceneModelProperty, ...model.variabilitySceneModels.map( vsm => vsm.rangeValueProperty ) ], () => {
+ return model.selectedSceneModelProperty.value.rangeValueProperty.value;
+ } );
+
+ const medianValueProperty = DerivedProperty.deriveAny( [ model.selectedSceneModelProperty, ...model.variabilitySceneModels.map( vsm => vsm.medianValueProperty ) ], () => {
+ return model.selectedSceneModelProperty.value.medianValueProperty.value;
+ } );
+
+ const iqrValueProperty = DerivedProperty.deriveAny( [ model.selectedSceneModelProperty, ...model.variabilitySceneModels.map( vsm => vsm.iqrValueProperty ) ], () => {
+ return model.selectedSceneModelProperty.value.iqrValueProperty.value;
+ } );
+
+ const madValueProperty = DerivedProperty.deriveAny( [ model.selectedSceneModelProperty, ...model.variabilitySceneModels.map( vsm => vsm.madValueProperty ) ], () => {
+ return model.selectedSceneModelProperty.value.madValueProperty.value;
+ } );
+
+ const readoutsToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
+ {
+ value: VariabilityMeasure.RANGE,
+ tandemName: 'rangeReadoutToggleNode',
+ createNode: tandem => new VariabilityReadoutText( rangeValueProperty,
+ CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
+ fill: CAVColors.meanColorProperty,
+ visibleProperty: model.isShowingRangeProperty,
+ left: 0,
+ centerY: backgroundNode.height / 2,
+ tandem: tandem.createTandem( 'rangeReadoutText' )
+ } )
+ }, {
+ value: VariabilityMeasure.IQR,
+ tandemName: 'iqrReadoutToggleNode',
+ createNode: tandem => {
+ const textReadoutGroup = new VBox( {
+ // x: -110,
+ // y: this.centerY - 20,
+ align: 'left',
+ spacing: 10
+ } );
+ // this.addChild( textReadoutGroup );
+
+ const medianReadoutValueProperty = new DerivedProperty( [ medianValueProperty ],
+ medianValue => medianValue === null ? '?' : `${medianValue}`
+ );
+ const iqrReadoutValueProperty = new DerivedProperty( [ iqrValueProperty ], iqrValue => {
+ return iqrValue ? `${iqrValue}` : '?';
+ } );
+
+ const medianReadoutText = new VariabilityReadoutText( medianReadoutValueProperty, CenterAndVariabilityStrings.medianEqualsValuePatternStringProperty, {
+ fill: CAVColors.medianColorProperty
+ // tandem: options.tandem.createTandem( 'medianReadoutText' )
+ } );
+ const iqrReadoutText = new VariabilityReadoutText( iqrReadoutValueProperty, CenterAndVariabilityStrings.iqrEqualsValuePatternStringProperty, {
+ fill: CAVColors.iqrColorProperty,
+ visibleProperty: model.isShowingIQRProperty
+ // tandem: options.tandem.createTandem( 'iqrReadoutText' )
+ } );
+
+ textReadoutGroup.addChild( medianReadoutText );
+ textReadoutGroup.addChild( iqrReadoutText );
+
+ return textReadoutGroup;
+ }
+ },
+ {
+ value: VariabilityMeasure.MAD,
+ tandemName: 'madReadoutToggleNode',
+ createNode: tandem => {
+ const madReadoutValueProperty = new DerivedProperty( [ madValueProperty ], madValue => {
+ return madValue ? `${madValue}` : '?';
+ } );
+
+ return new VariabilityReadoutText( madReadoutValueProperty,
+ CenterAndVariabilityStrings.meanEqualsValuePatternStringProperty, {
+ fill: CAVColors.meanColorProperty,
+ visibleProperty: model.isShowingMADProperty,
+ // tandem: options.tandem.createTandem( 'rangeReadoutText' )
+ } );
+ }
+ } ]
+ );
- super( model.resetEmitter, accordionBoxContents,
- new Text( accordionBoxTitleProperty, {
+ backgroundNode.addChild( readoutsToggleNode );
+
+ super( backgroundNode, {
+ tandem: tandem,
+ titleNode: new Text( accordionBoxTitleProperty, {
font: new PhetFont( 16 ),
maxWidth: 300
} ),
- layoutBounds,
- checkboxToggleNode,
- {
- leftMargin: 70,
- tandem: tandem,
- top: top,
- right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
- } );
+ left: 100
+ } );
+
+ // resetEmitter.addListener( () => this.reset() );
+
+ // this.plotToggleNode = plotToggleNode;
+
+ // super( model.resetEmitter, plotToggleNode,
+ // new Text( accordionBoxTitleProperty, {
+ // font: new PhetFont( 16 ),
+ // maxWidth: 300
+ // } ),
+ // layoutBounds,
+ // checkboxToggleNode,
+ // {
+ // leftMargin: 70,
+ // tandem: tandem,
+ // top: top,
+ // right: layoutBounds.right - CAVConstants.SCREEN_VIEW_X_MARGIN
+ // } );
+
+ this.plotToggleNode = plotToggleNode;
+ }
+
+ public alignWithPlayAreaNumberLineNode( x: number ): void {
+
+ // TODO: HACK ALERT, see https://github.com/phetsims/center-and-variability/issues/170
+ this.plotToggleNode.children.forEach( ( plotNode: VariabilityPlotNode ) => {
+ plotNode.alignWithPlayAreaNumberLineNode( x );
+ } );
}
}
Index: js/common/view/CAVAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVAccordionBox.ts b/js/common/view/CAVAccordionBox.ts
--- a/js/common/view/CAVAccordionBox.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/common/view/CAVAccordionBox.ts (date 1683675043555)
@@ -7,23 +7,19 @@
* @author Sam Reid (PhET Interactive Simulations)
*/
-import Bounds2 from '../../../../dot/js/Bounds2.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.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';
import centerAndVariability from '../../centerAndVariability.js';
-import { Shape } from '../../../../kite/js/imports.js';
import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
-import TEmitter from '../../../../axon/js/TEmitter.js';
+import { Node } from '../../../../scenery/js/imports.js';
type SelfOptions = {
leftMargin: number;
};
export type CAVAccordionBoxOptions =
SelfOptions
- & StrictOmit<AccordionBoxOptions, 'titleNode' | 'expandedProperty'>
+ & StrictOmit<AccordionBoxOptions, 'expandedProperty'>
& PickRequired<AccordionBoxOptions, 'tandem'>;
// constants
@@ -32,11 +28,8 @@
export default class CAVAccordionBox extends AccordionBox {
- public readonly plotNode: Node;
-
// NOTE: The positions of the passed-in nodes are modified directly, so they cannot be used in the scenery DAG
- public constructor( resetEmitter: TEmitter, plotNode: Node, titleNode: Node,
- layoutBounds: Bounds2, checkboxGroup: Node, providedOptions: CAVAccordionBoxOptions ) {
+ public constructor( contentNode: Node, providedOptions: CAVAccordionBoxOptions ) {
const options = optionize<CAVAccordionBoxOptions, SelfOptions, AccordionBoxOptions>()( {
titleAlignX: 'left',
@@ -45,7 +38,7 @@
titleYMargin: CONTENT_MARGIN,
buttonXMargin: CONTENT_MARGIN,
buttonYMargin: CONTENT_MARGIN,
- contentXMargin: CONTENT_MARGIN,
+ contentXMargin: 0,
contentYMargin: 0,
contentYSpacing: 0,
contentAlign: 'left',
@@ -54,45 +47,13 @@
},
// TODO: This is currently highlighting a layout issues with AccordionBox, see: https://github.com/phetsims/center-and-variability/issues/170
titleBarOptions: {
- stroke: 'black'
- },
- titleNode: titleNode
+ stroke: 'black',
+ opacity: 0.1
+ }
}, providedOptions );
- const backgroundNode = new Rectangle( {
- rectHeight: 140,
- rectWidth: layoutBounds.width - CAVConstants.SCREEN_VIEW_X_MARGIN * 2 - CONTENT_MARGIN * 2 - options.leftMargin
- } );
-
- // Since the title is visible while the accordion box is open, this background will not any area above the bottom of
- // the expand/collapse button. To vertically-center things, make a new set of bounds that includes the missing space.
- // Values come from the height of the expand/collapse button plus the y margin above and below it. Also add the
- // horizontal content margin that is not part of backgroundNode so these bounds are the full area of the accordion box.
- const fullBackgroundBounds =
- backgroundNode.localBounds.withOffsets( CONTENT_MARGIN, CONTENT_MARGIN * 2 + BUTTON_SIDE_LENGTH, CONTENT_MARGIN, 0 );
-
- // add clip area so dot stacks that are taller than the accordion box are clipped appropriately
- backgroundNode.clipArea = Shape.bounds( fullBackgroundBounds );
-
- // Vertical positioning
- plotNode.centerY = fullBackgroundBounds.centerY;
- if ( plotNode.bottom > fullBackgroundBounds.bottom - 5 ) {
- plotNode.bottom = fullBackgroundBounds.bottom - 5;
- }
- backgroundNode.addChild( plotNode );
-
- const checkboxAlignBox = new AlignBox( checkboxGroup,
- { xAlign: 'right', yAlign: 'center', rightMargin: 20, alignBounds: fullBackgroundBounds } );
-
- backgroundNode.addChild( checkboxAlignBox );
-
- super( backgroundNode, options );
-
- resetEmitter.addListener( () => this.reset() );
-
- this.plotNode = plotNode;
+ super( contentNode, options );
}
-
}
centerAndVariability.register( 'CAVAccordionBox', CAVAccordionBox );
\ No newline at end of file
Index: js/median/view/MedianAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/MedianAccordionBox.ts b/js/median/view/MedianAccordionBox.ts
--- a/js/median/view/MedianAccordionBox.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/median/view/MedianAccordionBox.ts (date 1683670923843)
@@ -2,7 +2,7 @@
import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
import CardNodeContainer from './CardNodeContainer.js';
-import { Text } from '../../../../scenery/js/imports.js';
+import { Path, Text } from '../../../../scenery/js/imports.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import MedianModel from '../model/MedianModel.js';
@@ -11,39 +11,49 @@
import centerAndVariability from '../../centerAndVariability.js';
import TopRepresentationCheckboxGroup from '../../common/view/TopRepresentationCheckboxGroup.js';
import VerticalCheckboxGroup from '../../../../sun/js/VerticalCheckboxGroup.js';
+import CardNode from './CardNode.js';
+import CAVConstants from '../../common/CAVConstants.js';
export default class MedianAccordionBox extends CAVAccordionBox {
public constructor( model: MedianModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
- 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' )
+ const backgroundShape = CAVConstants.ACCORDION_BOX_CONTENTS_SHAPE_MEAN_AND_OR_MEDIAN;
+ const backgroundNode = new Path( backgroundShape, {
+ clipArea: backgroundShape
} );
const checkboxGroup = new VerticalCheckboxGroup( [
TopRepresentationCheckboxGroup.getSortDataCheckboxItem( model.isSortingDataProperty ),
TopRepresentationCheckboxGroup.getMedianCheckboxWithoutIconItem( model.isShowingTopMedianProperty )
], {
- tandem: tandem.createTandem( 'accordionCheckboxGroup' )
+ tandem: tandem.createTandem( 'accordionCheckboxGroup' ),
+ right: backgroundNode.width - 12.5,
+ centerY: backgroundNode.height / 2
+ } );
+
+ 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' ),
+ x: 12.5,
+ y: backgroundNode.height / 2 - CardNode.CARD_DIMENSION / 2 - 10
} );
- super( model.selectedSceneModelProperty.value.resetEmitter, cardNodeContainer,
- new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, {
- font: new PhetFont( 16 ),
- maxWidth: 300
- } ),
- layoutBounds,
- checkboxGroup,
- {
- leftMargin: 0,
- tandem: tandem,
- top: top,
- centerX: layoutBounds.centerX
- } );
+ backgroundNode.addChild( cardNodeContainer );
+ backgroundNode.addChild( checkboxGroup );
+
+ super( backgroundNode, {
+ leftMargin: 0,
+ tandem: tandem,
+ top: top,
+ centerX: layoutBounds.centerX,
+ titleNode: new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, {
+ font: new PhetFont( 16 ),
+ maxWidth: 300
+ } )
+ } );
}
-
-
}
centerAndVariability.register( 'MedianAccordionBox', MedianAccordionBox );
\ No newline at end of file
Index: js/median/view/CardNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/CardNode.ts b/js/median/view/CardNode.ts
--- a/js/median/view/CardNode.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/median/view/CardNode.ts (date 1683670426614)
@@ -39,12 +39,12 @@
public animation: Animation | null = null;
private animationTo: Vector2 | null = null;
- public static readonly CARD_WIDTH = 43;
+ public static readonly CARD_DIMENSION = 43;
public constructor( cardModel: CardModel, position: Vector2, getDragRange: () => Range, providedOptions: CardNodeOptions ) {
const cornerRadius = 10;
- const rectangle = new Rectangle( 0, 0, CardNode.CARD_WIDTH, CardNode.CARD_WIDTH, cornerRadius, cornerRadius, {
+ const rectangle = new Rectangle( 0, 0, CardNode.CARD_DIMENSION, CardNode.CARD_DIMENSION, cornerRadius, cornerRadius, {
stroke: 'black',
lineWidth: 1,
fill: 'white'
Index: js/common/view/CAVPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVPlotNode.ts b/js/common/view/CAVPlotNode.ts
--- a/js/common/view/CAVPlotNode.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/common/view/CAVPlotNode.ts (date 1683672079803)
@@ -22,6 +22,7 @@
import CAVModel from '../model/CAVModel.js';
import MeanAndMedianModel from '../../mean-and-median/model/MeanAndMedianModel.js';
import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
+import Vector2 from '../../../../dot/js/Vector2.js';
type SelfOptions = {
dataPointFill: TColor;
@@ -32,6 +33,7 @@
private readonly dotLayer = new Node();
protected readonly modelViewTransform: ModelViewTransform2;
+ private readonly numberLineNode: NumberLineNode;
public constructor( model: CAVModel, sceneModel: CAVSceneModel, providedOptions?: CAVPlotOptions ) {
@@ -55,7 +57,7 @@
);
this.modelViewTransform = modelViewTransform;
- const numberLineNode = new NumberLineNode(
+ this.numberLineNode = new NumberLineNode(
sceneModel.meanValueProperty,
modelViewTransform,
model instanceof MeanAndMedianModel ? model.isShowingTopMeanProperty : new BooleanProperty( false ),
@@ -66,15 +68,15 @@
tandem: options.tandem.createTandem( 'numberLineNode' ),
y: numberLinePositionY
} );
- backgroundNode.addChild( numberLineNode );
+ backgroundNode.addChild( this.numberLineNode );
const distanceInMetersText = new Text( CenterAndVariabilityStrings.distanceInMetersStringProperty, {
- top: numberLineNode.bottom + 2,
+ top: this.numberLineNode.bottom + 2,
maxWidth: CAVConstants.INFO_DIALOG_MAX_TEXT_WIDTH
} );
backgroundNode.addChild( distanceInMetersText );
- ManualConstraint.create( this, [ numberLineNode.tickMarkSet, distanceInMetersText ], ( tickMarkSetProxy, textProxy ) => {
+ ManualConstraint.create( this, [ this.numberLineNode.tickMarkSet, distanceInMetersText ], ( tickMarkSetProxy, textProxy ) => {
textProxy.centerX = tickMarkSetProxy.centerX;
} );
@@ -105,6 +107,18 @@
public reset(): void {
// No implementation because this node is powered by the model. Reset needed for uniformity with CardNodeContainer.
}
+
+ // Assumes this node has been added to the scene graph, so we can use the scene graph to determine the coordinate transforms
+ public alignWithPlayAreaNumberLineNode( x: number ): void {
+
+ const numberLineGlobalBounds = this.numberLineNode.globalBounds;
+ const globalOffset = x - numberLineGlobalBounds.x;
+
+ // This wouldn't be correct if there were a scale transform, but there isn't.
+ // Also, if either number line had children that went out of bounds, it would throw off
+ // this calculation
+ this.translate( globalOffset, 0 );
+ }
}
centerAndVariability.register( 'CAVPlotNode', CAVPlotNode );
\ No newline at end of file
Index: js/common/view/SceneView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/SceneView.ts b/js/common/view/SceneView.ts
--- a/js/common/view/SceneView.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/common/view/SceneView.ts (date 1683675448243)
@@ -26,13 +26,13 @@
* Renders view elements for a CAVSceneModel. Note that to satisfy the correct z-ordering, elements
* populate the backObjectLayer and frontObjectLayer in the parent.
*/
-export default class SceneView {
+export default class SceneView<T extends CAVSceneModel = CAVSceneModel> {
private readonly updateMedianNode: () => void;
public constructor(
- model: CAVModel,
- sceneModel: CAVSceneModel,
+ model: CAVModel<T>,
+ sceneModel: T,
backObjectLayer: Node,
frontObjectLayer: Node,
modelViewTransform: ModelViewTransform2,
Index: js/common/model/CAVModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVModel.ts b/js/common/model/CAVModel.ts
--- a/js/common/model/CAVModel.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/common/model/CAVModel.ts (date 1683675283502)
@@ -16,7 +16,7 @@
};
export type CAVModelOptions = SelfOptions;
-export default class CAVModel {
+export default class CAVModel<T extends CAVSceneModel = CAVSceneModel> {
public readonly isShowingPlayAreaMedianProperty: BooleanProperty; // Screens 1-3
@@ -25,10 +25,10 @@
public readonly isShowingMedianPredictionProperty: BooleanProperty; // Screens 1-2
public readonly medianPredictionProperty: NumberProperty; // Screens 1-2
- public readonly selectedSceneModelProperty: Property<CAVSceneModel>;
+ public readonly selectedSceneModelProperty: Property<T>;
public readonly soccerBallHasBeenDraggedProperty: Property<boolean>;
- public constructor( public readonly maxKicksProperty: Property<number>, public readonly sceneModels: CAVSceneModel[], options: CAVModelOptions ) {
+ public constructor( public readonly maxKicksProperty: Property<number>, public readonly sceneModels: T[], options: CAVModelOptions ) {
this.isShowingPlayAreaMeanProperty = new BooleanProperty( false, {
tandem: options.instrumentMeanPredictionProperty ? options.tandem.createTandem( 'isShowingPlayAreaMeanProperty' ) : Tandem.OPT_OUT
Index: js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/IQRNode.ts b/js/variability/view/IQRNode.ts
--- a/js/variability/view/IQRNode.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/view/IQRNode.ts (date 1683674953883)
@@ -2,7 +2,7 @@
import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
import MedianBarNode from '../../common/view/MedianBarNode.js';
-import { Circle, Line, Node, ProfileColorProperty, Rectangle, Text, VBox } from '../../../../scenery/js/imports.js';
+import { Circle, Line, Node, ProfileColorProperty, Rectangle, Text } from '../../../../scenery/js/imports.js';
import ArrowNode from '../../../../scenery-phet/js/ArrowNode.js';
import centerAndVariability from '../../centerAndVariability.js';
import VariabilityModel from '../model/VariabilityModel.js';
@@ -10,9 +10,7 @@
import CAVPlotNode, { CAVPlotOptions } from '../../common/view/CAVPlotNode.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import CAVColors from '../../common/CAVColors.js';
-import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
-import VariabilityReadoutText from './VariabilityReadoutText.js';
import VariabilitySceneModel from '../model/VariabilitySceneModel.js';
type SelfOptions = {
@@ -30,38 +28,7 @@
...options
} );
- if ( providedOptions.parentContext === 'accordion' ) {
-
- const textReadoutGroup = new VBox( {
- x: -110,
- y: this.centerY - 20,
- align: 'left',
- spacing: 10
- } );
- this.addChild( textReadoutGroup );
-
- const medianReadoutValueProperty = new DerivedProperty( [ sceneModel.numberOfDataPointsProperty, sceneModel.medianValueProperty ],
- numberOfDataPoints => {
- return numberOfDataPoints >= 1 ? `${sceneModel.medianValueProperty.value}` : '?';
- } );
- const iqrReadoutValueProperty = new DerivedProperty( [ sceneModel.iqrValueProperty ], iqrValue => {
- return iqrValue ? `${iqrValue}` : '?';
- } );
-
- const medianReadoutText = new VariabilityReadoutText( medianReadoutValueProperty, CenterAndVariabilityStrings.medianEqualsValuePatternStringProperty, {
- fill: CAVColors.medianColorProperty,
- tandem: options.tandem.createTandem( 'medianReadoutText' )
- } );
- const iqrReadoutText = new VariabilityReadoutText( iqrReadoutValueProperty, CenterAndVariabilityStrings.iqrEqualsValuePatternStringProperty, {
- fill: CAVColors.iqrColorProperty,
- visibleProperty: model.isShowingIQRProperty,
- tandem: options.tandem.createTandem( 'iqrReadoutText' )
- } );
-
- textReadoutGroup.addChild( medianReadoutText );
- textReadoutGroup.addChild( iqrReadoutText );
- }
-
+ // TODO: get rid of context === accordion, see https://github.com/phetsims/center-and-variability/issues/170
const needAtLeastFiveKicksOffsetY = options.parentContext === 'info' ? 90 : 20;
const needAtLeastFiveKicks = new Text( CenterAndVariabilityStrings.needAtLeastFiveKicksStringProperty, {
fontSize: 18,
Index: js/common/CAVConstants.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/CAVConstants.ts b/js/common/CAVConstants.ts
--- a/js/common/CAVConstants.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/common/CAVConstants.ts (date 1683672629485)
@@ -15,6 +15,7 @@
import PlotType from './model/PlotType.js';
import ScreenView from '../../../joist/js/ScreenView.js';
import Range from '../../../dot/js/Range.js';
+import { Shape } from '../../../kite/js/imports.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 = [
@@ -56,7 +57,10 @@
},
PHYSICAL_RANGE: new Range( 1, 15 ),
- MAX_KICKS_VALUES: [ 15, 20, 25, 30 ]
+ MAX_KICKS_VALUES: [ 15, 20, 25, 30 ],
+
+ ACCORDION_BOX_CONTENTS_SHAPE_MEAN_AND_OR_MEDIAN: Shape.rect( 0, 0, 1000, 140 ),
+ ACCORDION_BOX_CONTENTS_SHAPE_VARIABILITY: Shape.rect( 0, 0, 920, 140 )
};
centerAndVariability.register( 'CAVConstants', CAVConstants );
Index: js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/RangeNode.ts b/js/variability/view/RangeNode.ts
--- a/js/variability/view/RangeNode.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/view/RangeNode.ts (date 1683668941137)
@@ -17,8 +17,6 @@
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import CAVConstants from '../../common/CAVConstants.js';
import CAVColors from '../../common/CAVColors.js';
-import VariabilityReadoutText from './VariabilityReadoutText.js';
-import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import VariabilitySceneModel from '../model/VariabilitySceneModel.js';
type SelfOptions = {
@@ -36,23 +34,6 @@
...options
} );
- if ( options.parentContext === 'accordion' ) {
- const rangeReadoutValueProperty = new DerivedProperty( [ sceneModel.rangeValueProperty ], rangeValue => {
- return rangeValue ? `${rangeValue}` : '?';
- } );
-
- const rangeReadoutText = new VariabilityReadoutText( rangeReadoutValueProperty,
- CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
- fill: CAVColors.meanColorProperty,
- visibleProperty: model.isShowingRangeProperty,
- right: this.left,
- y: this.centerY,
- tandem: options.tandem.createTandem( 'rangeReadoutText' )
- } );
-
- this.addChild( rangeReadoutText );
- }
-
const needAtLeastOneKickText = new Text( CenterAndVariabilityStrings.needAtLeastOneKickStringProperty, {
fontSize: 18,
top: 100,
Index: js/variability/view/VariabilityPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/VariabilityPlotNode.ts b/js/variability/view/VariabilityPlotNode.ts
--- a/js/variability/view/VariabilityPlotNode.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/view/VariabilityPlotNode.ts (date 1683673437174)
@@ -18,15 +18,18 @@
import VariabilityMeasure from '../model/VariabilityMeasure.js';
import ToggleNode from '../../../../sun/js/ToggleNode.js';
import VariabilitySceneModel from '../model/VariabilitySceneModel.js';
+import CAVPlotNode from '../../common/view/CAVPlotNode.js';
export type CAVPlotOptions = NodeOptions & PickRequired<NodeOptions, 'tandem'>;
export default class VariabilityPlotNode extends Node {
+ private toggleNode: ToggleNode<VariabilityMeasure | null>;
public constructor( model: VariabilityModel, sceneModel: VariabilitySceneModel, providedOptions: CAVPlotOptions ) {
super( providedOptions );
const toggleNode = new ToggleNode( model.selectedVariabilityProperty, [ {
+ alignChildren: ToggleNode.NONE,
createNode: tandem => new RangeNode( model, sceneModel, {
parentContext: 'accordion',
tandem: tandem.createTandem( 'rangeNode' )
@@ -52,6 +55,16 @@
} );
this.addChild( toggleNode );
toggleNode.moveToBack();
+
+ this.toggleNode = toggleNode;
+ }
+
+ public alignWithPlayAreaNumberLineNode( x: number ): void {
+
+ // TODO: This could be a lot better https://github.com/phetsims/center-and-variability/issues/170
+ this.toggleNode.children.forEach( ( child: CAVPlotNode ) => {
+ child.alignWithPlayAreaNumberLineNode( x );
+ } );
}
}
Index: js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
--- a/js/common/view/CAVScreenView.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/common/view/CAVScreenView.ts (date 1683672111837)
@@ -13,7 +13,7 @@
import CAVConstants from '../CAVConstants.js';
import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.js';
import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
-import { AlignBox, ManualConstraint, Node } from '../../../../scenery/js/imports.js';
+import { AlignBox, Node } from '../../../../scenery/js/imports.js';
import EraserButton from '../../../../scenery-phet/js/buttons/EraserButton.js';
import QuestionBar, { QuestionBarOptions } from '../../../../scenery-phet/js/QuestionBar.js';
import NumberLineNode from './NumberLineNode.js';
@@ -181,22 +181,6 @@
this.updateAccordionBoxPosition();
}
- /**
- * Sets the accordion box and aligns its content with the play area number line node.
- * Should be used with accordion boxes that also have a number line.
- * The alignment assumes that the NumberLineNode in the play area and in the dot plot have the same characteristics:
- * - Same font
- * -Same offset and scale
- * Given those assumptions, this code moves the dot plot so that its number line matches the play area one.
- */
- protected setAccordionBoxWithAlignedContent( accordionBox: CAVAccordionBox ): void {
- this.setAccordionBox( accordionBox );
- ManualConstraint.create( this, [ this.playAreaNumberLineNode, accordionBox.plotNode ],
- ( lowerNumberLineWrapper, contentsWrapper ) => {
- contentsWrapper.x = lowerNumberLineWrapper.x;
- } );
- }
-
protected setBottomControls( controlNode: Node ): void {
// In order to use the AlignBox we need to know the distance from the top of the screen, to the top of the grass.
Index: js/median/model/MedianModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/model/MedianModel.ts b/js/median/model/MedianModel.ts
--- a/js/median/model/MedianModel.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/median/model/MedianModel.ts (date 1683675223411)
@@ -15,7 +15,7 @@
import Tandem from '../../../../tandem/js/Tandem.js';
import NumberProperty from '../../../../axon/js/NumberProperty.js';
-export default class MedianModel extends CAVModel {
+export default class MedianModel extends CAVModel<CAVSceneModel> {
public readonly cards: CardModel[];
public readonly isSortingDataProperty: BooleanProperty;
Index: js/median/view/CardNodeContainer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/CardNodeContainer.ts b/js/median/view/CardNodeContainer.ts
--- a/js/median/view/CardNodeContainer.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/median/view/CardNodeContainer.ts (date 1683670426607)
@@ -39,7 +39,7 @@
// constants
const CARD_SPACING = 10;
-const getCardPositionX = ( index: number ) => index * ( CardNode.CARD_WIDTH + CARD_SPACING );
+const getCardPositionX = ( index: number ) => index * ( CardNode.CARD_DIMENSION + CARD_SPACING );
type SelfOptions = EmptySelfOptions;
export type CardNodeContainerOptions = SelfOptions & NodeOptions & PickRequired<NodeOptions, 'tandem'>;
Index: js/variability/view/VariabilityScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts
--- a/js/variability/view/VariabilityScreenView.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/view/VariabilityScreenView.ts (date 1683673060970)
@@ -45,7 +45,9 @@
super( model, options );
- this.setAccordionBoxWithAlignedContent( new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN ) );
+ const variabilityAccordionBox = new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN );
+ this.setAccordionBox( variabilityAccordionBox );
+ variabilityAccordionBox.alignWithPlayAreaNumberLineNode( this.playAreaNumberLineNode.globalBounds.x );
ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBox! ],
( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => {
Index: js/variability/model/VariabilityModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/model/VariabilityModel.ts b/js/variability/model/VariabilityModel.ts
--- a/js/variability/model/VariabilityModel.ts (revision dd415aba3a07495c4c75511fa6bd805db8d3768f)
+++ b/js/variability/model/VariabilityModel.ts (date 1683675158381)
@@ -22,7 +22,7 @@
type SelfOptions = EmptySelfOptions;
type VariabilityModelOptions = SelfOptions & CAVModelOptions;
-export default class VariabilityModel extends CAVModel {
+export default class VariabilityModel extends CAVModel<VariabilitySceneModel> {
private readonly initialized: boolean = false;
public readonly selectedVariabilityProperty: Property<VariabilityMeasure>;
|
marlitas
added a commit
that referenced
this issue
May 10, 2023
|
Here's an idea for how we can fix those 2x hack alerts: Subject: [PATCH] Update docs, see https://github.com/phetsims/center-and-variability/issues/189
---
Index: main/center-and-variability/js/variability/view/VariabilityPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts b/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts
--- a/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts (revision 2d1e0e5da12d0f06525f0e4e72c29e270eb9d260)
+++ b/main/center-and-variability/js/variability/view/VariabilityPlotNode.ts (date 1683679571577)
@@ -23,7 +23,7 @@
export type CAVPlotOptions = NodeOptions & PickRequired<NodeOptions, 'tandem'>;
export default class VariabilityPlotNode extends Node {
- private toggleNode: ToggleNode<VariabilityMeasure | null>;
+ private toggleNode: ToggleNode<VariabilityMeasure | null, CAVPlotNode>;
public constructor( model: VariabilityModel, sceneModel: VariabilitySceneModel, providedOptions: CAVPlotOptions ) {
super( providedOptions );
@@ -59,12 +59,7 @@
}
public alignWithPlayAreaNumberLineNode( x: number ): void {
-
- // TODO: This could be a lot better https://github.com/phetsims/center-and-variability/issues/170
- this.toggleNode.children.forEach( child => {
- const plotNode = child as CAVPlotNode;
- plotNode.alignWithPlayAreaNumberLineNode( x );
- } );
+ this.toggleNode.nodes.forEach( plotNode => plotNode.alignWithPlayAreaNumberLineNode( x ) );
}
}
Index: main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts
--- a/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (revision 2d1e0e5da12d0f06525f0e4e72c29e270eb9d260)
+++ b/main/center-and-variability/js/variability/view/VariabilityAccordionBox.ts (date 1683679482312)
@@ -20,12 +20,11 @@
import CAVAccordionBox from '../../common/view/CAVAccordionBox.js';
import CAVConstants from '../../common/CAVConstants.js';
import CAVSceneModel from '../../common/model/CAVSceneModel.js';
-import CAVPlotNode from '../../common/view/CAVPlotNode.js';
export default class VariabilityAccordionBox extends CAVAccordionBox {
// private plotNode: Node;
- private plotToggleNode: ToggleNode<CAVSceneModel | null>;
+ private plotToggleNode: ToggleNode<CAVSceneModel | null, VariabilityPlotNode>;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
@@ -53,7 +52,7 @@
};
} );
- const plotToggleNode = new ToggleNode( model.selectedSceneModelProperty, contents, {
+ const plotToggleNode = new ToggleNode<CAVSceneModel, VariabilityPlotNode>( model.selectedSceneModelProperty, contents, {
bottom: backgroundNode.height
} );
@@ -139,14 +138,14 @@
return model.selectedSceneModelProperty.value.madValueProperty.value;
} );
- const readoutsToggleNode = new ToggleNode( model.selectedVariabilityProperty, [
+ const readoutsToggleNode = new ToggleNode<VariabilityMeasure>( model.selectedVariabilityProperty, [
{
value: VariabilityMeasure.RANGE,
tandemName: 'rangeReadoutToggleNode',
createNode: tandem => {
const rangeReadoutValueProperty = new DerivedProperty( [ rangeValueProperty ],
rangeValue => rangeValue === null ? '?' : `${rangeValue}`
- );
+ );
return new VariabilityReadoutText( rangeReadoutValueProperty,
CenterAndVariabilityStrings.rangeEqualsValuePatternStringProperty, {
@@ -162,12 +161,9 @@
tandemName: 'iqrReadoutToggleNode',
createNode: tandem => {
const textReadoutGroup = new VBox( {
- // x: -110,
- // y: this.centerY - 20,
align: 'left',
spacing: 10
} );
- // this.addChild( textReadoutGroup );
const medianReadoutValueProperty = new DerivedProperty( [ medianValueProperty ],
medianValue => medianValue === null ? '?' : `${medianValue}`
@@ -225,12 +221,7 @@
}
public alignWithPlayAreaNumberLineNode( x: number ): void {
-
- // TODO: HACK ALERT, see https://github.com/phetsims/center-and-variability/issues/170
- this.plotToggleNode.children.forEach( child => {
- const plotNode = child as CAVPlotNode;
- plotNode.alignWithPlayAreaNumberLineNode( x );
- } );
+ this.plotToggleNode.nodes.forEach( plotNode => plotNode.alignWithPlayAreaNumberLineNode( x ) );
}
}
Index: main/sun/js/GroupItemOptions.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/GroupItemOptions.ts b/main/sun/js/GroupItemOptions.ts
--- a/main/sun/js/GroupItemOptions.ts (revision b96fb1c544aa0b54ad9a2e72fa73224e19d9493a)
+++ b/main/sun/js/GroupItemOptions.ts (date 1683679047726)
@@ -8,8 +8,8 @@
import Tandem from '../../tandem/js/Tandem.js';
import { Node } from '../../scenery/js/imports.js';
-type GroupItemOptions = {
- createNode: ( contentTandem: Tandem ) => Node;
+type GroupItemOptions<T extends Node = Node> = {
+ createNode: ( contentTandem: Tandem ) => T;
// If PhET-iO instrumented, tandemName must be supplied to supply the instrumentation. Optional to support
// uninstrumented sims and demos.
@@ -24,7 +24,7 @@
/**
* Get the nodes for the GroupItemOptions
*/
-export function getGroupItemNodes( array: GroupItemOptions[], tandem: Tandem ): Node[] {
+export function getGroupItemNodes<T extends Node = Node>( array: GroupItemOptions<T>[], tandem: Tandem ): T[] {
return array.map( item => {
// @ts-expect-error - runtime check to prevent prior pattern, see https://github.com/phetsims/sun/issues/794
Index: main/sun/js/ToggleNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ToggleNode.ts b/main/sun/js/ToggleNode.ts
--- a/main/sun/js/ToggleNode.ts (revision b96fb1c544aa0b54ad9a2e72fa73224e19d9493a)
+++ b/main/sun/js/ToggleNode.ts (date 1683679769153)
@@ -16,9 +16,9 @@
import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js';
import GroupItemOptions, { getGroupItemNodes } from './GroupItemOptions.js';
-export type ToggleNodeElement<T> = {
+export type ToggleNodeElement<T, N extends Node = Node> = {
value: T; // a value
-} & GroupItemOptions;
+} & GroupItemOptions<N>;
type SelfOptions = {
@@ -28,20 +28,12 @@
export type ToggleNodeOptions = SelfOptions & StrictOmit<NodeOptions, 'children'>;
-export default class ToggleNode<T> extends Node {
+export default class ToggleNode<T, N extends Node = Node> extends Node {
private readonly disposeToggleNode: () => void;
+ public readonly nodes: N[];
- public constructor( valueProperty: TReadOnlyProperty<T>, elements: ToggleNodeElement<T>[], providedOptions?: ToggleNodeOptions ) {
-
- // assert && assert( Array.isArray( elements ), 'elements should be an array' );
- // if ( assert ) {
- // elements.forEach( element => {
- // const keys = _.keys( element );
- // assert && assert( keys.length === 2, 'each element should have two keys' );
- // assert && assert( keys[ 0 ] === 'value' || keys[ 1 ] === 'value', 'element should have a value key' );
- // } );
- // }
+ public constructor( valueProperty: TReadOnlyProperty<T>, elements: ToggleNodeElement<T, N>[], providedOptions?: ToggleNodeOptions ) {
const options = optionize<ToggleNodeOptions, SelfOptions, NodeOptions>()( {
@@ -79,6 +71,8 @@
// tandem: options.tandem.createTandem( 'valueProperty' )
// } );
+ this.nodes = nodes;
+
this.disposeToggleNode = function() {
valueProperty.unlink( valueListener );
nodes.forEach( node => node.dispose() );
|
samreid
added a commit
that referenced
this issue
May 10, 2023
|
I decided to add some comments and TODOs to the previous comment, open a new Sun issue and commit. |
samreid
added a commit
that referenced
this issue
May 10, 2023
samreid
added a commit
that referenced
this issue
May 10, 2023
samreid
added a commit
that referenced
this issue
May 11, 2023
samreid
added a commit
that referenced
this issue
May 17, 2023
matthew-blackman
added a commit
that referenced
this issue
May 17, 2023
samreid
added a commit
that referenced
this issue
May 17, 2023
samreid
added
status:ready-for-review
and removed
dev:help-wanted
|
We still need to align the number lines--it was disrupted recently. ManualConstraint has problems--when the interval tool goes all the way to the left, it shifts the number line: Subject: [PATCH] Update TODO, see https://github.com/phetsims/center-and-variability/issues/170 and https://github.com/phetsims/center-and-variability/issues/213
---
Index: js/mean-and-median/view/MeanAndMedianScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MeanAndMedianScreenView.ts b/js/mean-and-median/view/MeanAndMedianScreenView.ts
--- a/js/mean-and-median/view/MeanAndMedianScreenView.ts (revision 8ced8247f5dacc8acb37af7642811c95961c55ab)
+++ b/js/mean-and-median/view/MeanAndMedianScreenView.ts (date 1684364490582)
@@ -12,7 +12,7 @@
import MeanAndMedianModel from '../model/MeanAndMedianModel.js';
import CAVColors from '../../common/CAVColors.js';
import CenterAndVariabilityStrings from '../../CenterAndVariabilityStrings.js';
-import { AlignGroup } from '../../../../scenery/js/imports.js';
+import { AlignGroup, ManualConstraint } from '../../../../scenery/js/imports.js';
import Range from '../../../../dot/js/Range.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import CAVScreenView, { CAVScreenViewOptions } from '../../common/view/CAVScreenView.js';
@@ -46,7 +46,10 @@
this.playAreaNumberLineNode
);
this.setAccordionBox( meanAndMedianAccordionBox );
- meanAndMedianAccordionBox.alignWithPlayAreaNumberLineNode( this.playAreaNumberLineNode.globalBounds.x );
+
+ ManualConstraint.create( this, [ meanAndMedianAccordionBox.medianPlotNode, this.playAreaNumberLineNode ], ( medianPlotNode, playAreaNumberLineNode ) => {
+ medianPlotNode.left = playAreaNumberLineNode.left;
+ } );
const iconGroup = new AlignGroup();
this.setBottomControls( new VerticalCheckboxGroup( [
Index: js/mean-and-median/view/MeanAndMedianAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MeanAndMedianAccordionBox.ts b/js/mean-and-median/view/MeanAndMedianAccordionBox.ts
--- a/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (revision 8ced8247f5dacc8acb37af7642811c95961c55ab)
+++ b/js/mean-and-median/view/MeanAndMedianAccordionBox.ts (date 1684364490595)
@@ -16,7 +16,7 @@
const MARGIN = 12.5;
export default class MeanAndMedianAccordionBox extends CAVAccordionBox {
- private readonly medianPlotNode: MeanAndMedianPlotNode;
+ public readonly medianPlotNode: MeanAndMedianPlotNode;
public constructor( model: MeanAndMedianModel, layoutBounds: Bounds2, tandem: Tandem, top: number, playAreaNumberLineNode: Node ) {
const iconGroup = new AlignGroup();
@@ -58,10 +58,6 @@
this.medianPlotNode = meanAndMedianPlotNode;
}
-
- public alignWithPlayAreaNumberLineNode( x: number ): void {
- this.medianPlotNode.alignWithPlayAreaNumberLineNode( x );
- }
}
centerAndVariability.register( 'MeanAndMedianAccordionBox', MeanAndMedianAccordionBox );
\ No newline at end of file
Index: js/variability/view/VariabilityPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/VariabilityPlotNode.ts b/js/variability/view/VariabilityPlotNode.ts
--- a/js/variability/view/VariabilityPlotNode.ts (revision 8ced8247f5dacc8acb37af7642811c95961c55ab)
+++ b/js/variability/view/VariabilityPlotNode.ts (date 1684364576862)
@@ -25,7 +25,7 @@
export type CAVPlotOptions = NodeOptions & PickRequired<NodeOptions, 'tandem'>;
export default class VariabilityPlotNode extends Node {
- private toggleNode: ToggleNode<VariabilityMeasure, CAVPlotNode>;
+ public readonly toggleNode: ToggleNode<VariabilityMeasure, CAVPlotNode>;
public constructor( model: VariabilityModel, sceneModel: VariabilitySceneModel, providedOptions: CAVPlotOptions ) {
super( providedOptions );
Index: js/variability/view/VariabilityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/VariabilityAccordionBox.ts b/js/variability/view/VariabilityAccordionBox.ts
--- a/js/variability/view/VariabilityAccordionBox.ts (revision 8ced8247f5dacc8acb37af7642811c95961c55ab)
+++ b/js/variability/view/VariabilityAccordionBox.ts (date 1684364490592)
@@ -25,7 +25,7 @@
export default class VariabilityAccordionBox extends CAVAccordionBox {
- private plotToggleNode: ToggleNode<CAVSceneModel, VariabilityPlotNode>;
+ public readonly plotToggleNode: ToggleNode<CAVSceneModel, VariabilityPlotNode>;
public constructor( model: VariabilityModel, layoutBounds: Bounds2, tandem: Tandem, top: number ) {
Index: js/variability/view/VariabilityScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts
--- a/js/variability/view/VariabilityScreenView.ts (revision 8ced8247f5dacc8acb37af7642811c95961c55ab)
+++ b/js/variability/view/VariabilityScreenView.ts (date 1684364490589)
@@ -79,7 +79,10 @@
const variabilityAccordionBox = new VariabilityAccordionBox( model, this.layoutBounds, options.tandem.createTandem( 'accordionBox' ), this.questionBar.bottom + CAVConstants.SCREEN_VIEW_Y_MARGIN );
this.setAccordionBox( variabilityAccordionBox );
- variabilityAccordionBox.alignWithPlayAreaNumberLineNode( this.playAreaNumberLineNode.globalBounds.x );
+
+ ManualConstraint.create( this, [ variabilityAccordionBox.plotToggleNode, this.playAreaNumberLineNode ], ( plotNode, playAreaNumberLineNode ) => {
+ plotNode.left = playAreaNumberLineNode.left;
+ } );
ManualConstraint.create( this, [ variabilityMeasureRadioButtonGroup, this.accordionBox! ],
( variabilityRadioButtonGroupWrapper, accordionBoxWrapper ) => {
We need to adjust the location of the parent so that the child ends up in the right spot--that's what's making ManualConstraint difficult to use in this case. The imperative approach works well if we run it over and over. Or if we make sure the play area number line + objects bounds/topology are the same as the one in the accordion box, we can align the containers. But soccer balls are wider than data points. |
samreid
added a commit
that referenced
this issue
May 18, 2023
samreid
added a commit
that referenced
this issue
May 18, 2023
samreid
added a commit
that referenced
this issue
May 18, 2023
samreid
added a commit
that referenced
this issue
May 18, 2023
marlitas
added a commit
that referenced
this issue
May 22, 2023
marlitas
added a commit
that referenced
this issue
May 26, 2023
marlitas
added a commit
that referenced
this issue
May 26, 2023
|
I addressed the review comments and TODO here. I also completed review. This issue seems ready to close to me, and any other accordionBox tweaks or bugs we notice can be sourced out to another issue. @samreid if that all sounds right to you, please feel free to close. |
|
I also removed the below TODO, and it's corresponding ManualConstraint. To me it didn't make sense that we would need to update the numberLine's center at any point past start-up. So I tested removing the manualConstraint in both the MeanAndMedianAccordionBox as well as the VariabilityAccordionBox while running |
marlitas
added a commit
that referenced
this issue
May 26, 2023
samreid
added a commit
that referenced
this issue
May 26, 2023
|
@matthew-blackman and I reviewed and committed some improvements. We feel this issue can be closed, but we will open 2 smaller side issues. |
This was referenced May 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are a lot of hacks and workarounds in the accordion box. I tried to sort it out in this version, but the ManualConstraint that aligns the plot nodes is throwing things off:
The text was updated successfully, but these errors were encountered: