phetsims / center-and-variability

"Center and Variability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 2 forks source link

Data points in accordion box should autoscale so that all data is visible #187

Closed matthew-blackman closed 1 year ago

matthew-blackman commented 1 year ago

If the user has a tall stack of soccer balls, we want to autoscale the size of the data points in the accordion box so that all of the data is visible.

catherinecarter commented 1 year ago

Here's a mockup of all 30 balls stacked in the same location with the accordion box remaining the same size and the soccer balls auto-scaling in the y-direction only.

Image

And one where the accordion box also resizes. Note the soccer balls still have to shrink even when the accordion box is expanded.

Image

@kathy-phet said on slack:

I actually prefer not making the accordion box bigger.

The auto-scale seems like a good solution so you can see all of the data points together in the abstract representation in the accordion box. Since users will have to manually stack the balls in the same location, the auto-scale will not be jarring as they will shrink one ball at a time.

samreid commented 1 year ago

I tried to make the modelViewTransform a Property. Is this the right approach?

```diff Subject: [PATCH] Remove unnecessary type annotations, see https://github.com/phetsims/center-and-variability/issues/187 --- Index: js/mean-and-median/view/MeanAndMedianPlotNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/mean-and-median/view/MeanAndMedianPlotNode.ts b/js/mean-and-median/view/MeanAndMedianPlotNode.ts --- a/js/mean-and-median/view/MeanAndMedianPlotNode.ts (revision f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/mean-and-median/view/MeanAndMedianPlotNode.ts (date 1683923931660) @@ -39,10 +39,10 @@ this.addChild( this.medianBarNode ); - const modelViewTransform = this.modelViewTransform; - const updateMedianBarNode = () => { + const modelViewTransform = this.modelViewTransformProperty.value; + const sortedDots = _.sortBy( sceneModel.getActiveSoccerBalls().filter( soccerBall => soccerBall.valueProperty.value !== null ), object => object.valueProperty.value ); const leftmostSoccerBall = sortedDots[ 0 ]; @@ -81,6 +81,7 @@ if ( model instanceof MeanAndMedianModel ) { model.isMedianAnimationCompleteProperty.link( updateMedianBarNode ); } + this.modelViewTransformProperty.link( updateMedianBarNode ); } } Index: js/common/view/SoccerBallNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/SoccerBallNode.ts b/js/common/view/SoccerBallNode.ts --- a/js/common/view/SoccerBallNode.ts (revision f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/view/SoccerBallNode.ts (date 1683923931665) @@ -15,6 +15,7 @@ import ballDark_png from '../../../images/ballDark_png.js'; import ball_png from '../../../images/ball_png.js'; import Vector2 from '../../../../dot/js/Vector2.js'; +import Property from '../../../../axon/js/Property.js'; export default class SoccerBallNode extends CAVObjectNode { @@ -25,7 +26,7 @@ // Use the y dimension, since it determines how the soccer balls stack. But maintain the same aspect ratio as the image const viewRadius = Math.abs( modelViewTransform.modelToViewDeltaY( CAVObjectType.SOCCER_BALL.radius ) ); - super( soccerBall, isPlayAreaMedianVisibleProperty, modelViewTransform, CAVObjectType.SOCCER_BALL.radius, options ); + super( soccerBall, isPlayAreaMedianVisibleProperty, new Property( modelViewTransform ), CAVObjectType.SOCCER_BALL.radius, options ); // The dark soccer ball is used for when a ball has input disabled. const soccerBallNode = new Image( ball_png ); 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 f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/view/CAVPlotNode.ts (date 1683923829081) @@ -25,6 +25,8 @@ import VariabilityModel from '../../variability/model/VariabilityModel.js'; import VariabilityMeasure from '../../variability/model/VariabilityMeasure.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import Property from '../../../../axon/js/Property.js'; +import TProperty from '../../../../axon/js/TProperty.js'; type SelfOptions = { dataPointFill: TColor; @@ -34,7 +36,7 @@ export default class CAVPlotNode extends Node { private readonly dotLayer = new Node(); - public readonly modelViewTransform: ModelViewTransform2; + public readonly modelViewTransformProperty: TProperty; private readonly numberLineNode: NumberLineNode; public constructor( model: CAVModel, sceneModel: CAVSceneModel, providedOptions?: CAVPlotOptions ) { @@ -53,15 +55,22 @@ // Coordinates here are somewhat unusual, since x dimension is based off of meters, and y dimension is based off of // number of objects. - const modelViewTransform = ModelViewTransform2.createRectangleInvertedYMapping( - new Bounds2( CAVConstants.PHYSICAL_RANGE.min, 0, CAVConstants.PHYSICAL_RANGE.max, 1 ), - new Bounds2( 0, numberLinePositionY - dataPointHeight, CAVConstants.CHART_VIEW_WIDTH, numberLinePositionY ) - ); - this.modelViewTransform = modelViewTransform; + const createModelViewTransform = ( tallestStackHeight: number ) => { + return ModelViewTransform2.createRectangleInvertedYMapping( + new Bounds2( CAVConstants.PHYSICAL_RANGE.min, 0, CAVConstants.PHYSICAL_RANGE.max, 10 ), + new Bounds2( 0, numberLinePositionY - dataPointHeight * tallestStackHeight, CAVConstants.CHART_VIEW_WIDTH, numberLinePositionY ) + ); + }; + + this.modelViewTransformProperty = new Property( createModelViewTransform( 10 ) ); + + sceneModel.stackChangedEmitter.addListener( () => { + this.modelViewTransformProperty.value = createModelViewTransform( Math.min( sceneModel.getTallestStackHeight(), 10 ) ); + } ); this.numberLineNode = new NumberLineNode( sceneModel.meanValueProperty, - modelViewTransform, + this.modelViewTransformProperty, model instanceof MeanAndMedianModel ? model.isTopMeanVisibleProperty : // TODO: The design calls for a different "look" for the mean. But why? See https://github.com/phetsims/center-and-variability/issues/180 @@ -98,7 +107,7 @@ // Create the data points for that scene scene.soccerBalls.forEach( ( soccerBall, index ) => { - const dotNode = new DataPointNode( soccerBall, ( model instanceof MeanAndMedianModel ) ? model.isTopMedianVisibleProperty : new BooleanProperty( false ), modelViewTransform, { + const dotNode = new DataPointNode( soccerBall, ( model instanceof MeanAndMedianModel ) ? model.isTopMedianVisibleProperty : new BooleanProperty( false ), this.modelViewTransformProperty, { tandem: options.tandem.createTandem( 'scene' + sceneIndex ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), fill: options.dataPointFill } ); 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 f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/view/CAVScreenView.ts (date 1683923851285) @@ -157,7 +157,7 @@ new DynamicProperty( model.selectedSceneModelProperty, { derive: 'meanValueProperty' } ), - modelViewTransform, + new Property( modelViewTransform ), model.isPlayAreaMeanVisibleProperty, new DynamicProperty( model.selectedSceneModelProperty, { derive: 'dataRangeProperty' Index: js/common/view/NumberLineNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/NumberLineNode.ts b/js/common/view/NumberLineNode.ts --- a/js/common/view/NumberLineNode.ts (revision f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/view/NumberLineNode.ts (date 1683923585921) @@ -39,7 +39,7 @@ public constructor( meanValueProperty: TReadOnlyProperty, - modelViewTransform: ModelViewTransform2, + modelViewTransformProperty: TReadOnlyProperty, isMeanIndicatorVisibleProperty: TReadOnlyProperty, rangeProperty: TReadOnlyProperty, providedOptions?: NumberLineNodeOptions @@ -90,8 +90,8 @@ stroke: CAVColors.meanColorProperty, lineWidth: 3.2 } ); - Multilink.multilink( [ rangeProperty, isMeanIndicatorVisibleProperty ], - ( range, isMeanIndicatorVisible ) => { + Multilink.multilink( [ rangeProperty, isMeanIndicatorVisibleProperty, modelViewTransformProperty ], + ( range, isMeanIndicatorVisible, modelViewTransform ) => { if ( range !== null ) { // Do not show any area or text above the data point if the range is 0 @@ -108,8 +108,8 @@ const meanIndicatorNode = NumberLineNode.createMeanIndicatorNode( options.includeMeanStroke, false ); this.addChild( meanIndicatorNode ); - Multilink.multilink( [ meanValueProperty, isMeanIndicatorVisibleProperty ], - ( meanValue, isMeanIndicatorVisible ) => { + Multilink.multilink( [ meanValueProperty, isMeanIndicatorVisibleProperty, modelViewTransformProperty ], + ( meanValue, isMeanIndicatorVisible, modelViewTransform ) => { if ( meanValue !== null ) { meanIndicatorNode.centerTop = new Vector2( modelViewTransform.modelToViewX( meanValue ), 0 ); } Index: js/common/view/DataPointNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/DataPointNode.ts b/js/common/view/DataPointNode.ts --- a/js/common/view/DataPointNode.ts (revision f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/view/DataPointNode.ts (date 1683923650863) @@ -16,12 +16,12 @@ export default class DataPointNode extends CAVObjectNode { public constructor( soccerBall: SoccerBall, isPlayAreaMedianVisibleProperty: TReadOnlyProperty, - modelViewTransform: ModelViewTransform2, + modelViewTransformProperty: TReadOnlyProperty, options: CAVObjectNodeOptions & { fill: TColor } ) { - super( soccerBall, isPlayAreaMedianVisibleProperty, modelViewTransform, CAVObjectType.DATA_POINT.radius, options ); + super( soccerBall, isPlayAreaMedianVisibleProperty, modelViewTransformProperty, CAVObjectType.DATA_POINT.radius, options ); - const viewRadius = modelViewTransform.modelToViewDeltaX( CAVObjectType.DATA_POINT.radius ); + const viewRadius = modelViewTransformProperty.value.modelToViewDeltaX( CAVObjectType.DATA_POINT.radius ); const circle = new Circle( viewRadius, { fill: options.fill, Index: js/common/model/CAVSceneModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/model/CAVSceneModel.ts b/js/common/model/CAVSceneModel.ts --- a/js/common/model/CAVSceneModel.ts (revision f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/model/CAVSceneModel.ts (date 1683923815758) @@ -349,6 +349,14 @@ this.stackChangedEmitter.emit( soccerBallStack ); } + public getTallestStackHeight(): number { + const heights = _.range( + CAVConstants.PHYSICAL_RANGE.min, + CAVConstants.PHYSICAL_RANGE.max + 1 + ).map( location => this.getStackAtLocation( location ).length ); + return _.max( heights )!; + } + /** * Clears out the data */ Index: js/common/view/CAVObjectNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts --- a/js/common/view/CAVObjectNode.ts (revision f8c7a4f276e41a54258688bf960264a41d5ea659) +++ b/js/common/view/CAVObjectNode.ts (date 1683923682053) @@ -32,7 +32,7 @@ public constructor( public readonly soccerBall: SoccerBall, isPlayAreaMedianVisibleProperty: TReadOnlyProperty, - modelViewTransform: ModelViewTransform2, + modelViewTransformProperty: TReadOnlyProperty, modelRadius: number, providedOptions?: CAVObjectNodeOptions ) { @@ -41,7 +41,7 @@ }, providedOptions ); super( options ); - const viewRadius = modelViewTransform.modelToViewDeltaX( modelRadius ); + const viewRadius = modelViewTransformProperty.value.modelToViewDeltaX( modelRadius ); // Visibilty controlled by subclass logic. Also this whole node is moved to front when the medianHighlight is shown // so it will appear in front (unless the user drags another object on top of it). @@ -51,7 +51,7 @@ this.addChild( this.medianHighlight ); soccerBall.positionProperty.link( position => { - this.translation = modelViewTransform.modelToViewPosition( position ); + this.translation = modelViewTransformProperty.value.modelToViewPosition( position ); } ); // The initial ready-to-kick ball is full opacity. The rest of the balls waiting to be kicked are lower opacity so ```
marlitas commented 1 year ago

Here's a start at scaling the data points through a scaleProperty. It's a bit of a rough implementation, I think the logic can be cleaned up quite a bit, and it is also only handling when the data points need to scale down, doesn't handle scaling up quite yet. I think it would be good to pair with @samreid for next steps to see if we like this better than the MVT route.

```diff Subject: [PATCH] Scale accordion box, see: https://github.com/phetsims/center-and-variability/issues/187 --- Index: js/common/view/CAVObjectNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts --- a/js/common/view/CAVObjectNode.ts (revision 7437bdadf5c5e295a9b52de1c4453e3e827726bf) +++ b/js/common/view/CAVObjectNode.ts (date 1684797931744) @@ -18,10 +18,15 @@ import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import Multilink from '../../../../axon/js/Multilink.js'; -import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; +import optionize from '../../../../phet-core/js/optionize.js'; +import Vector2 from '../../../../dot/js/Vector2.js'; + +type SelfOptions = { + translationStrategy?: ( position: Vector2 ) => void; +}; export type CAVObjectNodeOptions = - + SelfOptions // Take all options from NodeOptions, but do not allow passing through inputEnabledProperty since it requires special handling in multilink & StrictOmit & PickRequired; @@ -36,8 +41,11 @@ modelRadius: number, providedOptions?: CAVObjectNodeOptions ) { - const options = optionize()( { - cursor: 'pointer' + const options = optionize()( { + cursor: 'pointer', + translationStrategy: ( position: Vector2 ) => { + this.translation = modelViewTransform.modelToViewPosition( position ); + } }, providedOptions ); super( options ); @@ -51,7 +59,7 @@ this.addChild( this.medianHighlight ); soccerBall.positionProperty.link( position => { - this.translation = modelViewTransform.modelToViewPosition( position ); + options.translationStrategy( position ); } ); // The initial ready-to-kick ball is full opacity. The rest of the balls waiting to be kicked are lower opacity so Index: js/common/view/DataPointNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/DataPointNode.ts b/js/common/view/DataPointNode.ts --- a/js/common/view/DataPointNode.ts (revision 7437bdadf5c5e295a9b52de1c4453e3e827726bf) +++ b/js/common/view/DataPointNode.ts (date 1684797859106) @@ -12,25 +12,53 @@ import CAVConstants from '../CAVConstants.js'; import PlotType from '../model/PlotType.js'; import Multilink from '../../../../axon/js/Multilink.js'; +import Property from '../../../../axon/js/Property.js'; +import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; + +type DataPointNodeOptions = CAVObjectNodeOptions & { fill: TColor }; export default class DataPointNode extends CAVObjectNode { + private readonly translationStrategy: ( position: Vector2 ) => void; + private readonly scaleCountProperty: Property; + public constructor( soccerBall: SoccerBall, isPlayAreaMedianVisibleProperty: TReadOnlyProperty, - modelViewTransform: ModelViewTransform2, - options: CAVObjectNodeOptions & { fill: TColor } ) { + modelViewTransform: ModelViewTransform2, scaleProperty: Property, scaleCountProperty: Property, + providedOptions: DataPointNodeOptions ) { + + const translationProperty = new Property( new Vector2( 1, 0 ) ); + const translationStrategy = ( position: Vector2 ) => { + // console.log( 'position: ' + position ); + const scale = scaleCountProperty.value === 0 ? scaleProperty.value : scaleProperty.value ** scaleCountProperty.value; + console.log( scaleCountProperty.value ); + console.log( scale ); + const scaledPosition = new Vector2( position.x, position.y * scale ); + // console.log( 'scaledPosition: ' + scaledPosition ); + translationProperty.value = modelViewTransform.modelToViewPosition( scaledPosition ); + }; + + + super( soccerBall, isPlayAreaMedianVisibleProperty, modelViewTransform, CAVObjectType.DATA_POINT.radius, optionize()( { + translationStrategy: translationStrategy + }, providedOptions ) ); - super( soccerBall, isPlayAreaMedianVisibleProperty, modelViewTransform, CAVObjectType.DATA_POINT.radius, options ); + translationProperty.link( translation => { + this.translation = translation; + } ); + + this.translationStrategy = translationStrategy; + this.scaleCountProperty = scaleCountProperty; const viewRadius = modelViewTransform.modelToViewDeltaX( CAVObjectType.DATA_POINT.radius ); const circle = new Circle( viewRadius, { - fill: options.fill, + fill: providedOptions.fill, center: Vector2.ZERO } ); const cross = new Path( timesSolidShape, { // Leave some spacing between the stacked 'x' marks - fill: options.fill, + fill: providedOptions.fill, maxWidth: viewRadius * 2 * 0.8, center: Vector2.ZERO } ); @@ -68,6 +96,11 @@ } } ); } + + public triggerScaling( scale: number ): void { + this.translationStrategy( this.soccerBall.positionProperty.value ); + this.scale( scale ); + } } centerAndVariability.register( 'DataPointNode', DataPointNode ); \ No newline at end of file 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 7437bdadf5c5e295a9b52de1c4453e3e827726bf) +++ b/js/common/view/CAVPlotNode.ts (date 1684797931751) @@ -24,6 +24,7 @@ import VariabilityModel from '../../variability/model/VariabilityModel.js'; import VariabilityMeasure from '../../variability/model/VariabilityMeasure.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; type SelfOptions = { dataPointFill: TColor; @@ -38,6 +39,7 @@ private readonly dotLayer = new Node(); public readonly modelViewTransform: ModelViewTransform2; private readonly numberLineNode: NumberLineNode; + private scaleCount = 0; public constructor( model: CAVModel, sceneModel: CAVSceneModel, providedOptions?: CAVPlotNodeOptions ) { @@ -98,19 +100,40 @@ excludeInvisibleChildrenFromBounds: true } ); + const scaleProperty = new NumberProperty( 1 ); + const scaleCountProperty = new NumberProperty( 0 ); + + dataPointLayer.boundsProperty.link( bounds => { + if ( bounds.height > 110 ) { + scaleProperty.value = 0.8; + scaleCountProperty.value += scaleProperty.value >= 1 ? -1 : 1; + this.triggerScaling( scaleProperty.value, dataPointLayer.children ); + console.log( 'scale changed: ' + scaleProperty.value ); + } + // else if ( bounds.height < 90 && this.scaleCount !== 0 ) { + // scaleProperty.value = 1.25; + // this.triggerScaling( scaleProperty.value, dataPointLayer.children ); + // console.log( 'scale changed: ' + scaleProperty.value ); + // + // } + } ); + // Create the data points for that scene scene.soccerBalls.forEach( ( soccerBall, index ) => { - const dotNode = new DataPointNode( soccerBall, ( model instanceof MeanAndMedianModel ) ? model.isTopMedianVisibleProperty : new BooleanProperty( false ), modelViewTransform, { - tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), - fill: options.dataPointFill - } ); + const dotNode = new DataPointNode( soccerBall, ( model instanceof MeanAndMedianModel ) ? model.isTopMedianVisibleProperty : new BooleanProperty( false ), + modelViewTransform, scaleProperty, scaleCountProperty, { + tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), + fill: options.dataPointFill + } ); dataPointLayer.addChild( dotNode ); } ); this.dotLayer.addChild( dataPointLayer ); } ); + + } public reset(): void { @@ -128,6 +151,12 @@ // this calculation this.translate( globalOffset, 0 ); } + + private triggerScaling( scale: number, dataPointLayerChildren: DataPointNode[] ): void { + dataPointLayerChildren.forEach( child => { + child.triggerScaling( scale ); + } ); + } } centerAndVariability.register( 'CAVPlotNode', CAVPlotNode ); \ No newline at end of file ```
samreid commented 1 year ago

Patch:

```diff Subject: [PATCH] Add playback speed option, see https://github.com/phetsims/center-and-variability/issues/217 --- 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 06f85bc4e8e7cf3668e976a5cf721acc7f865759) +++ b/js/common/view/CAVPlotNode.ts (date 1684960898949) @@ -24,6 +24,7 @@ import VariabilityModel from '../../variability/model/VariabilityModel.js'; import VariabilityMeasure from '../../variability/model/VariabilityMeasure.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; type SelfOptions = { dataPointFill: TColor; @@ -38,6 +39,7 @@ private readonly dotLayer = new Node(); public readonly modelViewTransform: ModelViewTransform2; private readonly numberLineNode: NumberLineNode; + private scaleCount = 0; public constructor( model: CAVModel, sceneModel: CAVSceneModel, providedOptions?: CAVPlotNodeOptions ) { @@ -98,19 +100,55 @@ excludeInvisibleChildrenFromBounds: true } ); + const scaleProperty = new NumberProperty( 1, { + reentrant: true + } ); + // const scaleCountProperty = new NumberProperty( 0 ); + + let depth = 0; + // dataPointLayer.boundsProperty.link( bounds => { + // + // if ( depth >= 2 ) { + // return; + // } + // + // scaleProperty.value = Math.max( 110 / bounds.height, 1 ); + // + // depth++; + // this.triggerScaling( scaleProperty.value, dataPointLayer.children ); + // depth--; + // } ); + + sceneModel.stackChangedEmitter.addListener( () => { + const sortedSoccerBalls = _.sortBy( scene.getTopSoccerBalls(), soccerBall => soccerBall.positionProperty.value.y ); + if ( sortedSoccerBalls.length > 0 ) { + const highest = sortedSoccerBalls[ 0 ]; // TODO or maybe [length-1] + + const lowest = sortedSoccerBalls[ sortedSoccerBalls.length - 1 ]; + + const maxYValue = modelViewTransform.modelToViewY( highest.positionProperty.value.y ); + scaleProperty.value = Math.max( 110 / maxYValue, 1 ); + + this.triggerScaling( scaleProperty.value, dataPointLayer.children ); + } + } ); + // Create the data points for that scene scene.soccerBalls.forEach( ( soccerBall, index ) => { - const dotNode = new DataPointNode( soccerBall, modelViewTransform, { - tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), - fill: options.dataPointFill - } ); + const dotNode = new DataPointNode( soccerBall, + modelViewTransform, scaleProperty, { + tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), + fill: options.dataPointFill + } ); dataPointLayer.addChild( dotNode ); } ); this.dotLayer.addChild( dataPointLayer ); } ); + + } public reset(): void { @@ -128,6 +166,12 @@ // this calculation this.translate( globalOffset, 0 ); } + + private triggerScaling( scale: number, dataPointLayerChildren: DataPointNode[] ): void { + dataPointLayerChildren.forEach( child => { + child.triggerScaling( scale ); + } ); + } } centerAndVariability.register( 'CAVPlotNode', CAVPlotNode ); \ No newline at end of file Index: js/common/view/DataPointNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/DataPointNode.ts b/js/common/view/DataPointNode.ts --- a/js/common/view/DataPointNode.ts (revision 06f85bc4e8e7cf3668e976a5cf721acc7f865759) +++ b/js/common/view/DataPointNode.ts (date 1684960439563) @@ -12,16 +12,39 @@ import PlotType from '../model/PlotType.js'; import Multilink from '../../../../axon/js/Multilink.js'; import CAVColors from '../CAVColors.js'; +import Property from '../../../../axon/js/Property.js'; +import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; + +type DataPointNodeOptions = CAVObjectNodeOptions & { fill: TColor }; export default class DataPointNode extends CAVObjectNode { protected readonly medianHighlight: Circle; + private readonly translationStrategy: ( position: Vector2 ) => void; + // private readonly scaleCountProperty: Property; + public constructor( soccerBall: SoccerBall, - modelViewTransform: ModelViewTransform2, - options: CAVObjectNodeOptions & { fill: TColor } ) { + modelViewTransform: ModelViewTransform2, scaleProperty: Property, + providedOptions: DataPointNodeOptions ) { - super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, options ); + const translationProperty = new Property( new Vector2( 1, 0 ),{ + reentrant: true + } ); + const translationStrategy = ( position: Vector2 ) => { + // console.log( 'position: ' + position ); + // const scale = scaleCountProperty.value === 0 ? scaleProperty.value : scaleProperty.value ** scaleCountProperty.value; + // console.log( scaleCountProperty.value ); + // console.log( scale ); + const scale = scaleProperty.value; + const scaledPosition = new Vector2( position.x, position.y * scale ); + // console.log( 'scaledPosition: ' + scaledPosition ); + translationProperty.value = modelViewTransform.modelToViewPosition( scaledPosition ); + }; + + super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, optionize()( { + translationStrategy: translationStrategy + }, providedOptions ) ); const viewRadius = modelViewTransform.modelToViewDeltaX( CAVObjectType.DATA_POINT.radius ); @@ -32,13 +55,13 @@ this.addChild( this.medianHighlight ); const circle = new Circle( viewRadius, { - fill: options.fill, + fill: providedOptions.fill, center: Vector2.ZERO } ); const cross = new Path( timesSolidShape, { // Leave some spacing between the stacked 'x' marks - fill: options.fill, + fill: providedOptions.fill, maxWidth: viewRadius * 2 * 0.8, center: Vector2.ZERO } ); @@ -62,6 +85,18 @@ Multilink.multilink( [ soccerBall.isActiveProperty, soccerBall.valueProperty ], ( isActive, value ) => { this.visible = isActive && value !== null; } ); + + translationProperty.link( translation => { + this.translation = translation; + } ); + + this.translationStrategy = translationStrategy; + // this.scaleCountProperty = scaleCountProperty; + } + + public triggerScaling( scale: number ): void { + this.translationStrategy( this.soccerBall.positionProperty.value ); + this.setScaleMagnitude( scale ); } } Index: js/common/view/CAVObjectNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts --- a/js/common/view/CAVObjectNode.ts (revision 06f85bc4e8e7cf3668e976a5cf721acc7f865759) +++ b/js/common/view/CAVObjectNode.ts (date 1684959552686) @@ -16,12 +16,15 @@ import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import Multilink from '../../../../axon/js/Multilink.js'; -import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; +import optionize from '../../../../phet-core/js/optionize.js'; +import Vector2 from '../../../../dot/js/Vector2.js'; -type SelfOptions = EmptySelfOptions; +type SelfOptions = { + translationStrategy?: ( position: Vector2 ) => void; +}; export type CAVObjectNodeOptions = - + SelfOptions // Take all options from NodeOptions, but do not allow passing through inputEnabledProperty since it requires special handling in multilink & StrictOmit & PickRequired; @@ -33,12 +36,15 @@ providedOptions?: CAVObjectNodeOptions ) { const options = optionize()( { - cursor: 'pointer' + cursor: 'pointer', + translationStrategy: ( position: Vector2 ) => { + this.translation = modelViewTransform.modelToViewPosition( position ); + } }, providedOptions ); super( options ); soccerBall.positionProperty.link( position => { - this.translation = modelViewTransform.modelToViewPosition( position ); + options.translationStrategy( position ); } ); // The initial ready-to-kick ball is full opacity. The rest of the balls waiting to be kicked are lower opacity so ```
matthew-blackman commented 1 year ago

Updated patch:

Subject: [PATCH] Patch --- 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 06f85bc4e8e7cf3668e976a5cf721acc7f865759) +++ b/js/common/view/CAVPlotNode.ts (date 1684962425511) @@ -24,6 +24,7 @@ import VariabilityModel from '../../variability/model/VariabilityModel.js'; import VariabilityMeasure from '../../variability/model/VariabilityMeasure.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; type SelfOptions = { dataPointFill: TColor; @@ -38,6 +39,7 @@ private readonly dotLayer = new Node(); public readonly modelViewTransform: ModelViewTransform2; private readonly numberLineNode: NumberLineNode; + private scaleCount = 0; public constructor( model: CAVModel, sceneModel: CAVSceneModel, providedOptions?: CAVPlotNodeOptions ) { @@ -98,19 +100,57 @@ excludeInvisibleChildrenFromBounds: true } ); + const scaleProperty = new NumberProperty( 1, { + reentrant: true + } ); + // const scaleCountProperty = new NumberProperty( 0 ); + + let depth = 0; + // dataPointLayer.boundsProperty.link( bounds => { + // + // if ( depth >= 2 ) { + // return; + // } + // + // scaleProperty.value = Math.max( 110 / bounds.height, 1 ); + // + // depth++; + // this.triggerScaling( scaleProperty.value, dataPointLayer.children ); + // depth--; + // } ); + + sceneModel.stackChangedEmitter.addListener( () => { + const sortedSoccerBalls = _.sortBy( scene.getSortedLandedObjects(), soccerBall => soccerBall.positionProperty.value.y ); + if ( sortedSoccerBalls.length > 0 ) { + const highest = sortedSoccerBalls[ 0 ]; + const lowest = sortedSoccerBalls[ sortedSoccerBalls.length - 1 ]; + const minYValue = modelViewTransform.modelToViewY( lowest.positionProperty.value.y ); + const maxYValue = modelViewTransform.modelToViewY( highest.positionProperty.value.y ); + const deltaY = Math.abs( maxYValue - minYValue ); + const minDeltaYForScaleDown = 81; + const scaleFactor = minDeltaYForScaleDown / deltaY; + scaleProperty.value = Math.min( scaleFactor, 1 ); + + this.triggerScaling( scaleProperty.value, dataPointLayer.children ); + } + } ); + // Create the data points for that scene scene.soccerBalls.forEach( ( soccerBall, index ) => { - const dotNode = new DataPointNode( soccerBall, modelViewTransform, { - tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), - fill: options.dataPointFill - } ); + const dotNode = new DataPointNode( soccerBall, + modelViewTransform, scaleProperty, { + tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), + fill: options.dataPointFill + } ); dataPointLayer.addChild( dotNode ); } ); this.dotLayer.addChild( dataPointLayer ); } ); + + } public reset(): void { @@ -128,6 +168,12 @@ // this calculation this.translate( globalOffset, 0 ); } + + private triggerScaling( scale: number, dataPointLayerChildren: DataPointNode[] ): void { + dataPointLayerChildren.forEach( child => { + child.triggerScaling( scale ); + } ); + } } centerAndVariability.register( 'CAVPlotNode', CAVPlotNode ); \ No newline at end of file Index: js/common/view/CAVObjectNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts --- a/js/common/view/CAVObjectNode.ts (revision 06f85bc4e8e7cf3668e976a5cf721acc7f865759) +++ b/js/common/view/CAVObjectNode.ts (date 1684961270468) @@ -16,12 +16,15 @@ import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import Multilink from '../../../../axon/js/Multilink.js'; -import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; +import optionize from '../../../../phet-core/js/optionize.js'; +import Vector2 from '../../../../dot/js/Vector2.js'; -type SelfOptions = EmptySelfOptions; +type SelfOptions = { + translationStrategy?: ( position: Vector2 ) => void; +}; export type CAVObjectNodeOptions = - + SelfOptions // Take all options from NodeOptions, but do not allow passing through inputEnabledProperty since it requires special handling in multilink & StrictOmit & PickRequired; @@ -33,12 +36,15 @@ providedOptions?: CAVObjectNodeOptions ) { const options = optionize()( { - cursor: 'pointer' + cursor: 'pointer', + translationStrategy: ( position: Vector2 ) => { + this.translation = modelViewTransform.modelToViewPosition( position ); + } }, providedOptions ); super( options ); soccerBall.positionProperty.link( position => { - this.translation = modelViewTransform.modelToViewPosition( position ); + options.translationStrategy( position ); } ); // The initial ready-to-kick ball is full opacity. The rest of the balls waiting to be kicked are lower opacity so Index: js/common/view/DataPointNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/DataPointNode.ts b/js/common/view/DataPointNode.ts --- a/js/common/view/DataPointNode.ts (revision 06f85bc4e8e7cf3668e976a5cf721acc7f865759) +++ b/js/common/view/DataPointNode.ts (date 1684961270481) @@ -12,16 +12,39 @@ import PlotType from '../model/PlotType.js'; import Multilink from '../../../../axon/js/Multilink.js'; import CAVColors from '../CAVColors.js'; +import Property from '../../../../axon/js/Property.js'; +import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; + +type DataPointNodeOptions = CAVObjectNodeOptions & { fill: TColor }; export default class DataPointNode extends CAVObjectNode { protected readonly medianHighlight: Circle; + private readonly translationStrategy: ( position: Vector2 ) => void; + // private readonly scaleCountProperty: Property; + public constructor( soccerBall: SoccerBall, - modelViewTransform: ModelViewTransform2, - options: CAVObjectNodeOptions & { fill: TColor } ) { + modelViewTransform: ModelViewTransform2, scaleProperty: Property, + providedOptions: DataPointNodeOptions ) { - super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, options ); + const translationProperty = new Property( new Vector2( 1, 0 ),{ + reentrant: true + } ); + const translationStrategy = ( position: Vector2 ) => { + // console.log( 'position: ' + position ); + // const scale = scaleCountProperty.value === 0 ? scaleProperty.value : scaleProperty.value ** scaleCountProperty.value; + // console.log( scaleCountProperty.value ); + // console.log( scale ); + const scale = scaleProperty.value; + const scaledPosition = new Vector2( position.x, position.y * scale ); + // console.log( 'scaledPosition: ' + scaledPosition ); + translationProperty.value = modelViewTransform.modelToViewPosition( scaledPosition ); + }; + + super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, optionize()( { + translationStrategy: translationStrategy + }, providedOptions ) ); const viewRadius = modelViewTransform.modelToViewDeltaX( CAVObjectType.DATA_POINT.radius ); @@ -32,13 +55,13 @@ this.addChild( this.medianHighlight ); const circle = new Circle( viewRadius, { - fill: options.fill, + fill: providedOptions.fill, center: Vector2.ZERO } ); const cross = new Path( timesSolidShape, { // Leave some spacing between the stacked 'x' marks - fill: options.fill, + fill: providedOptions.fill, maxWidth: viewRadius * 2 * 0.8, center: Vector2.ZERO } ); @@ -62,6 +85,18 @@ Multilink.multilink( [ soccerBall.isActiveProperty, soccerBall.valueProperty ], ( isActive, value ) => { this.visible = isActive && value !== null; } ); + + translationProperty.link( translation => { + this.translation = translation; + } ); + + this.translationStrategy = translationStrategy; + // this.scaleCountProperty = scaleCountProperty; + } + + public triggerScaling( scale: number ): void { + this.translationStrategy( this.soccerBall.positionProperty.value ); + this.setScaleMagnitude( scale ); } }
matthew-blackman commented 1 year ago

Screen recording of sim with above patch:

https://github.com/phetsims/center-and-variability/assets/108756567/d839ea53-c47d-4b87-a5fc-56246c385b2a

marlitas commented 1 year ago

@chrisklus and I chatted through a couple of different scenarios with this autoscale, and we think it should be brought to designers... there's some behavior that seems potentially confusing or undesirable.

  1. When a data point animates to the top, and the points scale down, the height of the stack stays the same, giving the illusion that the amount of data points in the stack didn't change at all.

https://github.com/phetsims/center-and-variability/assets/42476338/0fdf8b81-f6e7-4ce4-9f7f-548f82b97765

  1. With dragging we have a similar effect as above except it's even sneakier.

https://github.com/phetsims/center-and-variability/assets/42476338/5b75b683-7dd9-4751-90c3-b686d5fd67d8

  1. If we change the data points to autoscale before the animation completes (or as soon as a ball is picked up), how would that affect multi-touch? We would have to make space for however many balls a user can pick up ahead of time, and then re-scale up once a user drops the balls in their final positions.

A common solution for all of these is to scale when we know a ball could potentially end up in a tall stack. This would make it clearer to the user that something is being added to the stack.

marlitas commented 1 year ago

This patch is not much different than Matt's but just cleaned up some vestigial code.

```diff Subject: [PATCH] autoscale --- Index: js/common/view/CAVObjectNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts --- a/js/common/view/CAVObjectNode.ts (revision d20dbe0aeaf70c7ba1e9e71981fda3de2aa1778f) +++ b/js/common/view/CAVObjectNode.ts (date 1684966397512) @@ -16,12 +16,15 @@ import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import Multilink from '../../../../axon/js/Multilink.js'; -import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; +import optionize from '../../../../phet-core/js/optionize.js'; +import Vector2 from '../../../../dot/js/Vector2.js'; -type SelfOptions = EmptySelfOptions; +type SelfOptions = { + translationStrategy?: ( position: Vector2 ) => void; +}; export type CAVObjectNodeOptions = - + SelfOptions // Take all options from NodeOptions, but do not allow passing through inputEnabledProperty since it requires special handling in multilink & StrictOmit & PickRequired; @@ -33,12 +36,15 @@ providedOptions?: CAVObjectNodeOptions ) { const options = optionize()( { - cursor: 'pointer' + cursor: 'pointer', + translationStrategy: ( position: Vector2 ) => { + this.translation = modelViewTransform.modelToViewPosition( position ); + } }, providedOptions ); super( options ); soccerBall.positionProperty.link( position => { - this.translation = modelViewTransform.modelToViewPosition( position ); + options.translationStrategy( position ); } ); // The initial ready-to-kick ball is full opacity. The rest of the balls waiting to be kicked are lower opacity so Index: js/common/view/DataPointNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/DataPointNode.ts b/js/common/view/DataPointNode.ts --- a/js/common/view/DataPointNode.ts (revision d20dbe0aeaf70c7ba1e9e71981fda3de2aa1778f) +++ b/js/common/view/DataPointNode.ts (date 1684967898517) @@ -12,16 +12,36 @@ import PlotType from '../model/PlotType.js'; import Multilink from '../../../../axon/js/Multilink.js'; import CAVColors from '../CAVColors.js'; +import Property from '../../../../axon/js/Property.js'; +import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; + +type DataPointNodeOptions = CAVObjectNodeOptions & { fill: TColor }; export default class DataPointNode extends CAVObjectNode { protected readonly medianHighlight: Circle; public constructor( soccerBall: SoccerBall, - modelViewTransform: ModelViewTransform2, - options: CAVObjectNodeOptions & { fill: TColor } ) { + modelViewTransform: ModelViewTransform2, scaleProperty: Property, + providedOptions: DataPointNodeOptions ) { + + const translationProperty = new Property( new Vector2( 1, 0 ), { + reentrant: true + } ); + const translationStrategy = ( position: Vector2 ) => { + const scale = scaleProperty.value; + const scaledPosition = new Vector2( position.x, position.y * scale ); + translationProperty.value = modelViewTransform.modelToViewPosition( scaledPosition ); + }; - super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, options ); + super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, optionize()( { + translationStrategy: translationStrategy + }, providedOptions ) ); + + scaleProperty.lazyLink( scale => { + this.setScaleMagnitude( scale ); + translationStrategy( this.soccerBall.positionProperty.value ); + } ); const viewRadius = modelViewTransform.modelToViewDeltaX( CAVObjectType.DATA_POINT.radius ); @@ -32,13 +52,13 @@ this.addChild( this.medianHighlight ); const circle = new Circle( viewRadius, { - fill: options.fill, + fill: providedOptions.fill, center: Vector2.ZERO } ); const cross = new Path( timesSolidShape, { // Leave some spacing between the stacked 'x' marks - fill: options.fill, + fill: providedOptions.fill, maxWidth: viewRadius * 2 * 0.8, center: Vector2.ZERO } ); @@ -62,6 +82,10 @@ Multilink.multilink( [ soccerBall.isActiveProperty, soccerBall.valueProperty ], ( isActive, value ) => { this.visible = isActive && value !== null; } ); + + translationProperty.link( translation => { + this.translation = translation; + } ); } } 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 d20dbe0aeaf70c7ba1e9e71981fda3de2aa1778f) +++ b/js/common/view/CAVPlotNode.ts (date 1684968911453) @@ -24,6 +24,7 @@ import VariabilityModel from '../../variability/model/VariabilityModel.js'; import VariabilityMeasure from '../../variability/model/VariabilityMeasure.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; type SelfOptions = { dataPointFill: TColor; @@ -98,13 +99,32 @@ excludeInvisibleChildrenFromBounds: true } ); + const scaleProperty = new NumberProperty( 1, { + // reentrant: true + } ); + + sceneModel.stackChangedEmitter.addListener( () => { + const sortedSoccerBalls = _.sortBy( scene.getSortedLandedObjects(), soccerBall => soccerBall.positionProperty.value.y ); + if ( sortedSoccerBalls.length > 0 ) { + const highest = sortedSoccerBalls[ 0 ]; + const lowest = sortedSoccerBalls[ sortedSoccerBalls.length - 1 ]; + const minYValue = modelViewTransform.modelToViewY( lowest.positionProperty.value.y ); + const maxYValue = modelViewTransform.modelToViewY( highest.positionProperty.value.y ); + const deltaY = Math.abs( maxYValue - minYValue ); + const minDeltaYForScaleDown = 90; + const scaleFactor = minDeltaYForScaleDown / deltaY; + scaleProperty.value = Math.min( scaleFactor, 1 ); + } + } ); + // Create the data points for that scene scene.soccerBalls.forEach( ( soccerBall, index ) => { - const dotNode = new DataPointNode( soccerBall, modelViewTransform, { - tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), - fill: options.dataPointFill - } ); + const dotNode = new DataPointNode( soccerBall, + modelViewTransform, scaleProperty, { + tandem: options.tandem.createTandem( `scene${sceneIndex + 1}` ).createTandem( 'dataPointNodes' ).createTandem( 'dataPointNode' + index ), + fill: options.dataPointFill + } ); dataPointLayer.addChild( dotNode ); } ); ```
marlitas commented 1 year ago

Met with @catherinecarter and we decided that we do not want to autoscale due to the complexity that arises in dev implementation as well as user interactions. We decided that instead we will scale the data points down as maxKicks goes up.

For example:

These percent scales are just examples to illustrate the decision and will probably not be the actual numbers used in the end

marlitas commented 1 year ago

The data points are now scaling based on the maxKicks preferences. @catherinecarter sending your way to interact with and see if this solution works for you to handle the visualization of more data points. Changes are pushed up to master.

catherinecarter commented 1 year ago

I like how there's a 1/2 an 'x' and 1/2 a dot at the top to indicate the markers are going off the screen and there are more up there.

The 70% is pretty small. Since there's a half object to indicate going off the screen, I'm thinking they don't need to be quite that small when the max kicks is set to 30. Let's try 100%, 95%, 90% and 85% and see how it looks. Thanks, @marlitas!

marlitas commented 1 year ago

Awesome! I made them a bit bigger in the above commit. Just for reference the scaling you were actually looking at was:

Now they are:

We tweaked the scales to line up so that only half of a datapoint is obscured. Do those sizes look better?

catherinecarter commented 1 year ago

They look better, but I think the points on the max = 30 are still a bit small. It looks like a pretty big difference (noticeable) between 25 and 30. Thanks for the actual references percentages - when I was looking at max = 15 and 20, I was having trouble noticing any difference at all, so I'm glad it wasn't me! Haha!

Let's bring up the percent on the max = 25 and 30 so the increments are smaller between them and hence, less noticeable. I'm becoming less and less worried about it now that I have in my mind the info box will show all the data. Maybe rather than leaping by 9% and 11%, we leap by 4% and 7% or something?

marlitas commented 1 year ago

Okay, here it is with a smaller difference between the 25 & 30 data points. I wasn't able to have maxKicks = 25 at 94% because it didn't line up with cutting off the "x".

MaxKicks = 15, DataPointsScale = 98% MaxKicks = 20, DataPointsScale = 98% MaxKicks = 25, DataPointsScale = 91% MaxKicks = 30, DataPointsScale = 83%

marlitas commented 1 year ago

From 6/6/23 meeting, the scaling is good to go! Closing.