phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Interval tool keyboard should support home/end and page up/page down #208

Closed matthew-blackman closed 5 months ago

matthew-blackman commented 5 months ago

@catherinecarter noticed that the interval tool does not support large increment jumps and skipping to the min/max. Let's add support for modifier keys and more advanced keyboard use of the interval tool.

samreid commented 5 months ago

This feature should also be described in the keyboard help dialog.

samreid commented 5 months ago

Collaboration patch:

```diff Subject: [PATCH] Minor cleanup, see https://github.com/phetsims/chipper/issues/1421 --- Index: projectile-data-lab/js/measures/view/IntervalToolNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/projectile-data-lab/js/measures/view/IntervalToolNode.ts b/projectile-data-lab/js/measures/view/IntervalToolNode.ts --- a/projectile-data-lab/js/measures/view/IntervalToolNode.ts (revision 43b434a22a01d0215665abd9ad7230581bdc889f) +++ b/projectile-data-lab/js/measures/view/IntervalToolNode.ts (date 1709674842593) @@ -8,7 +8,7 @@ */ import projectileDataLab from '../../projectileDataLab.js'; -import { DragListener, DragListenerOptions, KeyboardDragListener, KeyboardDragListenerOptions, Line, Node, NodeOptions, PressedDragListener, VBox } from '../../../../scenery/js/imports.js'; +import { DragListener, DragListenerOptions, Line, Node, NodeOptions, PressedDragListener, VBox } from '../../../../scenery/js/imports.js'; import IntervalTool from '../model/IntervalTool.js'; import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js'; import ShadedSphereNode from '../../../../scenery-phet/js/ShadedSphereNode.js'; @@ -34,6 +34,8 @@ import nullSoundPlayer from '../../../../tambo/js/shared-sound-players/nullSoundPlayer.js'; import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import NumberProperty from '../../../../axon/js/NumberProperty.js'; +import AccessibleSlider, { AccessibleSliderOptions } from '../../../../sun/js/accessibility/AccessibleSlider.js'; +import Bounds2 from '../../../../dot/js/Bounds2.js'; const edgeFilter = new BiquadFilterNode( phetAudioContext, { type: 'lowpass', @@ -105,6 +107,57 @@ this.addChild( edge1Line ); this.addChild( edge2Line ); + + // /** + // * Here, we create adapters to work with the 2D DragListeners (even though we really only need the horizontal values). + // */ + // const centerProperty = { + // get value(): Vector2 { + // return new Vector2( intervalTool.center, 0 ); + // }, + // set value( v: Vector2 ) { + // intervalTool.center = v.x; + // } + // }; + // + // const edge1Property = { + // get value(): Vector2 { + // return new Vector2( intervalTool.edge1, 0 ); + // }, + // set value( v: Vector2 ) { + // intervalTool.edge1 = v.x; + // } + // }; + // + // const edge2Property = { + // get value(): Vector2 { + // return new Vector2( intervalTool.edge2, 0 ); + // }, + // set value( v: Vector2 ) { + // intervalTool.edge2 = v.x; + // } + // }; + + // These are a downstream Properties (only updated in the update function) that is used for sonification only. Please see the documentation + // in IntervalTool.ts about atomicity. + const edge1XProperty = new NumberProperty( intervalTool.edge1 ); + const edge2XProperty = new NumberProperty( intervalTool.edge2 ); + const centerXProperty = new NumberProperty( intervalTool.center, { + reentrant: true + } ); + + edge1XProperty.link( edge1 => { + intervalTool.edge1 = edge1; + } ); + + edge2XProperty.link( edge2X => { + intervalTool.edge2 = edge2X; + } ); + + centerXProperty.link( centerX => { + intervalTool.center = centerX; + } ); + const sphereOptions = { mainColor: PDLColors.intervalToolSphereFillProperty, focusable: true, @@ -112,8 +165,22 @@ translation: Vector2.ZERO }; - const edge1Sphere = new ShadedSphereNode( 20, sphereOptions ); - const edge2Sphere = new ShadedSphereNode( 20, sphereOptions ); + class DraggableShadedSphereNode extends AccessibleSlider( Node, 0 ) { + public constructor( options: AccessibleSliderOptions ) { + super( options ); + + this.addChild( new ShadedSphereNode( 20, sphereOptions ) ); + } + } + + const edge1Sphere = new DraggableShadedSphereNode( { + valueProperty: edge1XProperty, + enabledRangeProperty: new Property( new Range( 0, PDLConstants.MAX_FIELD_DISTANCE ) ) + } ); + const edge2Sphere = new DraggableShadedSphereNode( { + valueProperty: edge2XProperty, + enabledRangeProperty: new Property( new Range( 0, PDLConstants.MAX_FIELD_DISTANCE ) ) + } ); const height = 71; edge1Sphere.touchArea = edge1Sphere.localBounds.dilatedY( height ).shiftY( height ); @@ -189,49 +256,36 @@ percentReadout.touchArea = percentReadout.localBounds.dilatedXY( TEXT_PANEL_BOUNDS_DILATION, TEXT_PANEL_BOUNDS_DILATION ); } ); - const readoutVBox = new VBox( { - focusable: true, - tagName: 'div', - children: [ intervalReadout, percentReadout ], - spacing: 4 + class ReadoutVBox extends AccessibleSlider( Node, 0 ) { + public constructor( options: AccessibleSliderOptions ) { + super( options ); + this.addChild( new VBox( { + focusable: true, + tagName: 'div', + children: [ intervalReadout, percentReadout ], + spacing: 4 + } ) ); + } + } + + const centerRangeProperty = new Property( new Range( 0, PDLConstants.MAX_FIELD_DISTANCE ) ); + intervalTool.changedEmitter.addListener( () => { + const width = Math.abs( intervalTool.edge2 - intervalTool.edge1 ); + const min = Math.min( intervalTool.edge1, intervalTool.edge2 ); + const max = Math.max( intervalTool.edge1, intervalTool.edge2 ); + + const leftEdge = Math.max( 0, min + width / 2 ); + const rightEdge = Math.min( PDLConstants.MAX_FIELD_DISTANCE, max - width / 2 ); + + centerRangeProperty.value = new Range( leftEdge, rightEdge ); + } ); + + const readoutVBox = new ReadoutVBox( { + valueProperty: centerXProperty, + enabledRangeProperty: new Property( new Range( 0, PDLConstants.MAX_FIELD_DISTANCE ) ) } ); this.addChild( readoutVBox ); - /** - * Here, we create adapters to work with the 2D DragListeners (even though we really only need the horizontal values). - */ - const centerProperty = { - get value(): Vector2 { - return new Vector2( intervalTool.center, 0 ); - }, - set value( v: Vector2 ) { - intervalTool.center = v.x; - } - }; - - const edge1Property = { - get value(): Vector2 { - return new Vector2( intervalTool.edge1, 0 ); - }, - set value( v: Vector2 ) { - intervalTool.edge1 = v.x; - } - }; - - const edge2Property = { - get value(): Vector2 { - return new Vector2( intervalTool.edge2, 0 ); - }, - set value( v: Vector2 ) { - intervalTool.edge2 = v.x; - } - }; - - // These are a downstream Properties (only updated in the update function) that is used for sonification only. Please see the documentation - // in IntervalTool.ts about atomicity. - const edge1XProperty = new NumberProperty( intervalTool.edge1 ); - const edge2XProperty = new NumberProperty( intervalTool.edge2 ); - const centerXProperty = new NumberProperty( intervalTool.center ); const update = () => { const viewEdge1X = modelViewTransform.modelToViewX( intervalTool.edge1 ); @@ -262,6 +316,7 @@ readoutVBox.top = ARROW_Y - intervalReadout.height / 2; // Update the downstream Properties which are only used for sonification. + // Only update the Property values once everything is fully in-sync. edge1XProperty.value = intervalTool.edge1; edge2XProperty.value = intervalTool.edge2; centerXProperty.value = intervalTool.center; @@ -286,8 +341,9 @@ transform: modelViewTransform }; - const dragListenerOptions = { - useInputListenerCursor: true + const dragListenerOptions: DragListenerOptions = { + useInputListenerCursor: true, + dragBoundsProperty: new Property( new Bounds2( 0, 0, PDLConstants.MAX_FIELD_DISTANCE, 0 ) ) }; const translateDragListenerOptions = { @@ -303,17 +359,16 @@ readoutVBox.addInputListener( new DragListener( combineOptions>( { applyOffset: true, useParentOffset: true, - positionProperty: centerProperty, + positionProperty: centerXProperty, tandem: providedOptions.tandem.createTandem( 'centerDragListener' ) }, translateDragListenerOptions, dragListenerOptions ) ) ); edge1Sphere.addInputListener( new DragListener( combineOptions>( { - positionProperty: edge1Property, + positionProperty: edge1XProperty, tandem: providedOptions.tandem.createTandem( 'edge1DragListener' ), drag: moveToFront( edge1Sphere ) }, listenerOptions, dragListenerOptions ) ) ); - // Play a ratcheting sound as either edge is dragged. The sound is played when passing thresholds on the field, // but the sound played is a function of the width of the interval. const edgePlaybackRateMapper = ( value: number ) => Utils.linear( 0, 100, 3, 1, value ); @@ -328,12 +383,12 @@ maxSoundPlayer: nullSoundPlayer } ); - const createEdgeSonificationListener = ( otherEdgeProperty: { value: Vector2 } ) => { + const createEdgeSonificationListener = ( otherEdgeProperty: { value: number } ) => { return ( newValue: number, oldValue: number ) => { if ( !this.isCenterDraggingProperty.value ) { - const newWidth = otherEdgeProperty.value.x - newValue; - const oldWidth = otherEdgeProperty.value.x - oldValue; + const newWidth = otherEdgeProperty.value - newValue; + const oldWidth = otherEdgeProperty.value - oldValue; edgeValueChangeSoundPlayer.playSoundIfThresholdReached( Math.abs( newWidth ), Math.abs( oldWidth ) ); } @@ -348,8 +403,8 @@ }; }; - edge1XProperty.lazyLink( createEdgeSonificationListener( edge2Property ) ); - edge2XProperty.lazyLink( createEdgeSonificationListener( edge1Property ) ); + edge1XProperty.lazyLink( createEdgeSonificationListener( edge2XProperty ) ); + edge2XProperty.lazyLink( createEdgeSonificationListener( edge1XProperty ) ); // Play a sound when the interval tool is being translated, and its center crosses a threshold value. // The sound played is a function of the horizontal position of the center position. @@ -372,33 +427,11 @@ } ); edge2Sphere.addInputListener( new DragListener( combineOptions>( { - positionProperty: edge2Property, + positionProperty: edge2XProperty, tandem: providedOptions.tandem.createTandem( 'edge2DragListener' ), drag: moveToFront( edge2Sphere ) }, listenerOptions, dragListenerOptions ) ) ); - const keyboardDragListenerOptions = { - dragSpeed: 300, // drag speed, in view coordinates per second - shiftDragSpeed: 20 // slower drag speed - }; - - readoutVBox.addInputListener( new KeyboardDragListener( combineOptions( { - positionProperty: centerProperty, - tandem: providedOptions.tandem.createTandem( 'centerKeyboardDragListener' ) - }, translateDragListenerOptions, keyboardDragListenerOptions ) ) ); - - edge1Sphere.addInputListener( new KeyboardDragListener( combineOptions( { - positionProperty: edge1Property, - tandem: providedOptions.tandem.createTandem( 'edge1KeyboardDragListener' ), - drag: moveToFront( edge1Sphere ) - }, listenerOptions, keyboardDragListenerOptions ) ) ); - - edge2Sphere.addInputListener( new KeyboardDragListener( combineOptions( { - positionProperty: edge2Property, - tandem: providedOptions.tandem.createTandem( 'edge2KeyboardDragListener' ), - drag: moveToFront( edge2Sphere ) - }, listenerOptions, keyboardDragListenerOptions ) ) ); - this.pdomOrder = [ edge1Sphere, edge2Sphere, Index: scenery/js/listeners/DragListener.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/listeners/DragListener.ts b/scenery/js/listeners/DragListener.ts --- a/scenery/js/listeners/DragListener.ts (revision 434b1b3ab08aac7a538b3a82ec833f6fbc72417a) +++ b/scenery/js/listeners/DragListener.ts (date 1709674968733) @@ -89,11 +89,14 @@ type OffsetPosition = ( point: Vector2, listener: Listener ) => Vector2; type SelfOptions = { + + dimensionality: '2d' | '1d'; // ???????? + // 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?: Pick, 'value'> | null; + positionProperty?: Pick | TProperty, 'value'> | 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. @@ -340,7 +343,11 @@ const parentPoint = this.globalToParentPoint( this._localPoint.set( point ) ); if ( this._useParentOffset ) { - this.modelToParentPoint( this._parentOffset.set( this._positionProperty!.value ) ).subtract( parentPoint ); + let myValue = this._positionProperty!.value; + if ( typeof myValue === 'number' ) { + myValue = new Vector2( myValue, 0 ); + } + this.modelToParentPoint( this._parentOffset.set( myValue ) ).subtract( parentPoint ); } // Set the local point @@ -684,7 +691,13 @@ } if ( this._positionProperty ) { - this._positionProperty.value = this._modelPoint.copy(); // Include an extra reference so that it will change. + if ( this._positionProperty.value instanceof Vector2 ) { + this._positionProperty.value = this._modelPoint.copy(); // Include an extra reference so that it will change. + } + else { + this._positionProperty.value = this._modelPoint.x; + } + } sceneryLog && sceneryLog.InputListener && sceneryLog.pop(); ```
matthew-blackman commented 5 months ago

@samreid and I updated the interval tool to implement AccessibleSlider, and also added PhET-IO instrumentation of the edges. The work we've put into the IntervalTool and IntervalToolNode has really paid off. Nice work! Let's update the keyboard help dialog before closing this issue.

samreid commented 5 months ago

I followed the pattern in Center and Variability and added keyboard help for the measures screen interval tool and its edges. It doesn't look very good in my opinion but it's unclear how best to improve it. It look like this at the moment:

image

Let's check in with @matthew-blackman

matthew-blackman commented 5 months ago

@samreid and I reviewed this with @catherinecarter and everything is working great. Nice job! Closing.