Add the Interval Tool/Variability prediction tool

[samreid]

We still need to add the Interval Tool/ Variability prediction tool. Like the one we made an icon for in #182

[marlitas]

My own unsolicited comment is that this is very cool and fun :-)

[matthew-blackman]

Tagged as 'ready-for-review' and assigning @catherinecarter and @amanda-phet for design review.

[samreid]

Notably, the design doc shows different handles than was implemented for this rough draft. We reused the same code as in screens 1-2 (since this required no effort and is something that would have been needed anyways) and it came out looking like sphere + arrow, as in the previous screens. Is that OK or should we change it?

Also, do we want to make it so you can click in the yellow area to drag/translate the entire region?

[samreid]

Also, I think there needs to be transparency for overlapping cases like this, right?

[catherinecarter]

I'm on the fence about the draggable arrows at the bottom. Still thinking on that one (e.g., it doesn't match the icon). I would love to see some transparency on the colored rectangles, yes. Transparency would help the user know there's overlap, and while the Interval Tool rectangle and the spread rectangles are different heights, it would be great to see a slight color change when there's overlap between the two rectangles.

[samreid]

The rangeFillProperty and intervalToolFillProperty color and opacity can be adjusted using the Color Editor utility. Will that be sufficient to adjust the colors + transparency, or will you need to work with a developer to adjust z-ordering or other aspects?

[catherinecarter]

• centerAndVariability.intervalToolIconShadedSphereMainColor #FDF581
• centerAndVariability.intervalToolIconShadedSphereHighlightColor #FDFDF2
• centerAndVariability.rangeFillProperty Opacity = 0.70
• centerAndVariability.quartileColor Opacity = 0.55
• centerAndVariability.madRectangleColorProeprty Opacity = 0.70 (The slider wasn't changing the opacity, so let's try this and see how it looks.)

[catherinecarter]

Got the opacity working, thanks to the dev's help :)
For
centerAndVariability.madRectangleColorProperty

• let's change the color to #CBA3E6 and opacity 0.50. Thanks!

[catherinecarter]

The explanation is that the idea in the comment #194 (comment) was just in a patch in the comment (to see what it looks like and see if it is a good general direction) and not in a commit. I cleaned it up, made it work with the arrow handles on the previous screens and committed it.

Thanks for explaining that. And thanks for committing so I can see it. I'll take a look!

[samreid]

In the commit, I also added the stroke color to the color profile it is named playAreaIntervalToolHandleLineStrokeProperty and I adjusted it to where it is clearly visible but also doesn't occlude the number labels too much. Please feel free to adjust and give new color/opacity for that.

Also, I see an outstanding question from above:

Also, do we want to make it so you can click in the yellow area to drag/translate the entire region?

This refers to the play area only, and it wouldn't do anything if you click on it in the dot/line plot. Want to try it?

[samreid]

In today's meeting we agreed it would be nice to drag the interval in both the play area and the dot/line plot.

[samreid]

Here's a patch idea for the listener which changes the interface of DragListener:

Subject: [PATCH] Allow dragging the interval tool rectangle to translate it, see https://github.com/phetsims/center-and-variability/issues/194
---
Index: main/scenery/js/listeners/DragListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/listeners/DragListener.ts b/main/scenery/js/listeners/DragListener.ts
--- a/main/scenery/js/listeners/DragListener.ts	(revision bb2ea3a8c08144267b1ec8c4755c1e96d3825080)
+++ b/main/scenery/js/listeners/DragListener.ts	(date 1684185897539)
@@ -88,12 +88,17 @@
 type MapPosition = ( point: Vector2 ) => Vector2;
 type OffsetPosition<Listener extends DragListener> = ( point: Vector2, listener: Listener ) => Vector2;
 
+type TPropertyInterface = {
+  get value(): Vector2;
+  set value( value: Vector2 );
+};
+
 type SelfOptions<Listener extends DragListener> = {
   // If provided, it will be synchronized with the drag position in the model coordinate
   // frame (applying any provided transforms as needed). Typically, DURING a drag this Property should not be
   // modified externally (as the next drag event will probably undo the change), but it's completely fine to modify
   // this Property at any other time.
-  positionProperty?: TProperty<Vector2> | null;
+  positionProperty?: TPropertyInterface | null;
 
   // Called as start( event: {SceneryEvent}, listener: {DragListener} ) when the drag is started.
   // This is preferred over passing press(), as the drag start hasn't been fully processed at that point.
Index: main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts b/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts
--- a/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts	(revision 46bdb97a8f6cc0624fb208487f21ec2a64c08823)
+++ b/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts	(date 1684188282388)
@@ -8,6 +8,9 @@
 import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
+import Vector2 from '../../../../dot/js/Vector2.js';
+import Bounds2 from '../../../../dot/js/Bounds2.js';
+import Property from '../../../../axon/js/Property.js';
 
 export default class IntervalToolPlayAreaNode extends Node {
   public constructor( intervalToolValue1Property: NumberProperty, intervalToolValue2Property: NumberProperty, modelViewTransform: ModelViewTransform2,
@@ -45,18 +48,29 @@
         rightEdge.setLine( viewX2, rectBottom, viewX2, rectTop );
       } );
 
+    const dragBoundsProperty = new Property( new Bounds2( -100, -1000, 100, 1000 ) );
+
+    const positionPropertyInterface = {
+      get value() {
+        return new Vector2( ( intervalToolValue1Property.value + intervalToolValue2Property.value ) / 2, 0 );
+      },
+      set value( v: Vector2 ) {
+        const dist = intervalToolValue1Property.value - intervalToolValue2Property.value;
+        intervalToolValue1Property.value = v.x - dist / 2;
+        intervalToolValue2Property.value = v.x + dist / 2;
+
+        dragBoundsProperty.value = new Bounds2(
+          intervalToolValue1Property.range.min + dist / 2 + 0.1, -100,
+          intervalToolValue1Property.range.max - dist / 2 - 0.1, 100
+        );
+      }
+    };
+
     const dragListener = new DragListener( {
+      dragBoundsProperty: dragBoundsProperty,
+      useParentOffset: true,
       transform: modelViewTransform,
-      drag: ( event, dragListener ) => {
-
-        const modelDeltaX = dragListener.modelDelta.x;
-
-        if ( intervalToolValue1Property.range.contains( intervalToolValue1Property.value + modelDeltaX ) &&
-             intervalToolValue2Property.range.contains( intervalToolValue2Property.value + modelDeltaX ) ) {
-          intervalToolValue1Property.value = intervalToolValue1Property.range.constrainValue( intervalToolValue1Property.value + dragListener.modelDelta.x );
-          intervalToolValue2Property.value = intervalToolValue2Property.range.constrainValue( intervalToolValue2Property.value + dragListener.modelDelta.x );
-        }
-      },
+      positionProperty: positionPropertyInterface,
       tandem: providedOptions.tandem.createTandem( 'dragListener' )
     } );
     this.addInputListener( dragListener );

[samreid]

This is working much better, but multitouch is buggy if you drag the left handle and the rectangle at the same time:

Subject: [PATCH] Allow dragging the interval tool rectangle to translate it, see https://github.com/phetsims/center-and-variability/issues/194
---
Index: main/scenery/js/listeners/DragListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/listeners/DragListener.ts b/main/scenery/js/listeners/DragListener.ts
--- a/main/scenery/js/listeners/DragListener.ts	(revision bb2ea3a8c08144267b1ec8c4755c1e96d3825080)
+++ b/main/scenery/js/listeners/DragListener.ts	(date 1684185897539)
@@ -88,12 +88,17 @@
 type MapPosition = ( point: Vector2 ) => Vector2;
 type OffsetPosition<Listener extends DragListener> = ( point: Vector2, listener: Listener ) => Vector2;
 
+type TPropertyInterface = {
+  get value(): Vector2;
+  set value( value: Vector2 );
+};
+
 type SelfOptions<Listener extends DragListener> = {
   // If provided, it will be synchronized with the drag position in the model coordinate
   // frame (applying any provided transforms as needed). Typically, DURING a drag this Property should not be
   // modified externally (as the next drag event will probably undo the change), but it's completely fine to modify
   // this Property at any other time.
-  positionProperty?: TProperty<Vector2> | null;
+  positionProperty?: TPropertyInterface | null;
 
   // Called as start( event: {SceneryEvent}, listener: {DragListener} ) when the drag is started.
   // This is preferred over passing press(), as the drag start hasn't been fully processed at that point.
Index: main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts b/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts
--- a/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts	(revision 46bdb97a8f6cc0624fb208487f21ec2a64c08823)
+++ b/main/center-and-variability/js/variability/view/IntervalToolPlayAreaNode.ts	(date 1684188404532)
@@ -8,6 +8,9 @@
 import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
+import Vector2 from '../../../../dot/js/Vector2.js';
+import Bounds2 from '../../../../dot/js/Bounds2.js';
+import Property from '../../../../axon/js/Property.js';
 
 export default class IntervalToolPlayAreaNode extends Node {
   public constructor( intervalToolValue1Property: NumberProperty, intervalToolValue2Property: NumberProperty, modelViewTransform: ModelViewTransform2,
@@ -45,18 +48,29 @@
         rightEdge.setLine( viewX2, rectBottom, viewX2, rectTop );
       } );
 
+    const dragBoundsProperty = new Property( new Bounds2( -100, -1000, 100, 1000 ) );
+
+    const positionPropertyInterface = {
+      get value() {
+        return new Vector2( intervalToolValue1Property.value, 0 );
+      },
+      set value( v: Vector2 ) {
+        const dist = intervalToolValue2Property.value - intervalToolValue1Property.value;
+        intervalToolValue1Property.value = v.x;
+        intervalToolValue2Property.value = v.x + dist;
+
+        dragBoundsProperty.value = new Bounds2(
+          intervalToolValue1Property.range.min, -100,
+          intervalToolValue1Property.range.max - dist, 100
+        );
+      }
+    };
+
     const dragListener = new DragListener( {
+      dragBoundsProperty: dragBoundsProperty,
+      useParentOffset: true,
       transform: modelViewTransform,
-      drag: ( event, dragListener ) => {
-
-        const modelDeltaX = dragListener.modelDelta.x;
-
-        if ( intervalToolValue1Property.range.contains( intervalToolValue1Property.value + modelDeltaX ) &&
-             intervalToolValue2Property.range.contains( intervalToolValue2Property.value + modelDeltaX ) ) {
-          intervalToolValue1Property.value = intervalToolValue1Property.range.constrainValue( intervalToolValue1Property.value + dragListener.modelDelta.x );
-          intervalToolValue2Property.value = intervalToolValue2Property.range.constrainValue( intervalToolValue2Property.value + dragListener.modelDelta.x );
-        }
-      },
+      positionProperty: positionPropertyInterface,
       tandem: providedOptions.tandem.createTandem( 'dragListener' )
     } );
     this.addInputListener( dragListener );

[samreid]

@marlitas and I agreed we would like to exclude multitouch.

[marlitas]

Marking this for design, so that we can check in on multi-touch and see if we can move forward without it.

[samreid]

Let's close this one and open a new multitouch-specific issue. Thanks!

[phet-dev]

Reopening because there is a TODO marked for this issue.

[samreid]

Today @catherinecarter agreed it is ok to exclude multitouch for translation. It's still nice to have multitouch on the handles though.

[marlitas]

Multi-touch work is being done in #225. Closing.