Skip to content

Commit

Permalink
Address ui interaction corner cases for tracks and control points, see …
Browse files Browse the repository at this point in the history 
…#385
  • Loading branch information
@samreid
samreid committed Jan 15, 2025
1 parent 7ef69b6 commit c39746d
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 45 deletions.
21 changes: 15 additions & 6 deletions js/common/model/ControlPoint.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import Vector2Property from '../../../../dot/js/Vector2Property.js';
import optionize from '../../../../phet-core/js/optionize.js';
import PhetioObject, { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import BooleanIO from '../../../../tandem/js/types/BooleanIO.js';
import IOType from '../../../../tandem/js/types/IOType.js';
import NumberIO from '../../../../tandem/js/types/NumberIO.js';
import NullableIO from '../../../../tandem/js/types/NullableIO.js';
import energySkatePark from '../../energySkatePark.js';
import EnergySkateParkModel from './EnergySkateParkModel.js';

Expand Down Expand Up @@ -117,15 +118,23 @@ export default class ControlPoint extends PhetioObject {
valueType: ControlPoint,
documentation: 'A control point that can manipulate the track.',
toStateObject: controlPoint => {
return { x: controlPoint.positionProperty.value.x, y: controlPoint.positionProperty.value.y };
return {
visible: controlPoint.visible,
interactive: controlPoint.interactive,
limitBounds: controlPoint.limitBounds
};
},
stateSchema: {
x: NumberIO,
y: NumberIO
visible: BooleanIO,
interactive: BooleanIO,
limitBounds: NullableIO( Bounds2.Bounds2IO )
},
stateObjectToCreateElementArguments: stateObject => {
assert && assert( typeof stateObject.x === 'number' );
return [ stateObject.x, stateObject.y ];
return [ 0, 0, {
visible: stateObject.visible,
interactive: stateObject.interactive,
limitBounds: stateObject.limitBounds
} ];
}
} );
}
Expand Down
4 changes: 0 additions & 4 deletions js/common/model/EnergySkateParkModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,6 @@ export default class EnergySkateParkModel extends PhetioObject {
this.controlPointGroup.disposeElement( controlPointToDelete );
const newTrack = this.trackGroup.createNextElement( points, Track.FULLY_INTERACTIVE_OPTIONS );
newTrack.physicalProperty.value = true;
newTrack.droppedProperty.value = true;

// smooth out the new track, see #177
const smoothingPoint = controlPointIndex >= newTrack.controlPoints.length ? newTrack.controlPoints.length - 1 : controlPointIndex;
Expand Down Expand Up @@ -1607,10 +1606,8 @@ export default class EnergySkateParkModel extends PhetioObject {

const newTrack1 = this.trackGroup.createNextElement( points1, Track.FULLY_INTERACTIVE_OPTIONS );
newTrack1.physicalProperty.value = true;
newTrack1.droppedProperty.value = true;
const newTrack2 = this.trackGroup.createNextElement( points2, Track.FULLY_INTERACTIVE_OPTIONS );
newTrack2.physicalProperty.value = true;
newTrack2.droppedProperty.value = true;

track.removeEmitter.emit();
this.removeAndDisposeTrack( track );
Expand Down Expand Up @@ -1701,7 +1698,6 @@ export default class EnergySkateParkModel extends PhetioObject {

const newTrack = this.trackGroup.createNextElement( points, Track.FULLY_INTERACTIVE_OPTIONS );
newTrack.physicalProperty.value = true;
newTrack.droppedProperty.value = true;

a.disposeControlPoints();
a.removeEmitter.emit();
Expand Down
26 changes: 7 additions & 19 deletions js/common/model/Track.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ export default class Track extends PhetioObject {

// Flag to indicate whether the user has dragged the track out of the toolbox. If dragging from the toolbox,
// then dragging translates the entire track instead of just a point.
public readonly droppedProperty: BooleanProperty;
public readonly controlPoints: ControlPoint[];
public readonly controlPointDraggingProperty: TReadOnlyProperty<boolean>;
public readonly parametricPosition: Float64Array<ArrayBuffer>;
Expand Down Expand Up @@ -185,20 +184,9 @@ export default class Track extends PhetioObject {
phetioState: options.phetioState // Participate in state only if parent track is too
} );

this.leftThePanelProperty = new BooleanProperty( false, {
tandem: tandem.createTandem( 'leftThePanelProperty' ),
phetioState: options.phetioState // Participate in state only if parent track is too
} );
this.leftThePanelProperty = new BooleanProperty( false );

this.draggingProperty = new BooleanProperty( false, {
tandem: tandem.createTandem( 'draggingProperty' ),
phetioState: options.phetioState // Participate in state only if parent track is too
} );

this.droppedProperty = new BooleanProperty( false, {
tandem: tandem.createTandem( 'droppedProperty' ),
phetioState: options.phetioState // Participate in state only if parent track is too
} );
this.draggingProperty = new BooleanProperty( false );

const trackChangedListener = () => { model.trackChangedEmitter.emit(); };
this.physicalProperty.link( trackChangedListener );
Expand Down Expand Up @@ -235,9 +223,9 @@ export default class Track extends PhetioObject {
};
phetioStateSetEmitter.addListener( stateListener );

// when available bounds change, make sure that control points are within - must be disposed
// when available bounds change, make sure that control points are within bounds - must be disposed
const boundsListener = ( bounds: Bounds2 ) => {
if ( this.droppedProperty.get() ) {
if ( this.physicalProperty.value && this.splittable ) {
this.containControlPointsInAvailableBounds( bounds );
}
};
Expand All @@ -248,7 +236,6 @@ export default class Track extends PhetioObject {
this.physicalProperty.dispose();
this.leftThePanelProperty.dispose();
this.draggingProperty.dispose();
this.droppedProperty.dispose();
this.controlPointDraggingProperty.dispose();

this.model.availableModelBoundsProperty.unlink( boundsListener );
Expand Down Expand Up @@ -291,7 +278,6 @@ export default class Track extends PhetioObject {
this.physicalProperty.reset();
this.leftThePanelProperty.reset();
this.draggingProperty.reset();
this.droppedProperty.reset();
for ( let i = 0; i < this.controlPoints.length; i++ ) {
this.controlPoints[ i ].reset();
}
Expand Down Expand Up @@ -1054,7 +1040,9 @@ export default class Track extends PhetioObject {

return [ controlPoints, {
draggable: stateObject.draggable,
configurable: stateObject.configurable
configurable: stateObject.configurable,
splittable: stateObject.splittable,
attachable: stateObject.attachable
} ];
},
stateSchema: {
Expand Down
15 changes: 11 additions & 4 deletions js/common/view/ControlPointNode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export default class ControlPointNode extends Circle {
trackNode.moveToFront();

// If control point dragged out of the control panel, translate the entire track, see #130
if ( !track.physicalProperty.value || ( !track.droppedProperty.value && track.draggable ) ) {
if ( !track.physicalProperty.value ) {

// save drag handler so we can end the drag if we need to after forwarding
if ( track.dragSource === null ) {
Expand All @@ -120,12 +120,19 @@ export default class ControlPointNode extends Circle {
if ( !model.containsTrack( track ) ) { return; }

// If control point dragged out of the control panel, translate the entire track, see #130
if ( !track.physicalProperty.value || ( !track.droppedProperty.value && track.draggable ) ) {
if ( !track.physicalProperty.value ) {

// Only drag a track if nothing else was dragging the track (which caused a flicker), see #282
if ( track.dragSource === dragListener ) {

trackDragHandler && trackDragHandler.trackDragged( event );
if ( trackDragHandler ) {
if ( trackDragHandler.startOffset === null ) {
trackDragHandler.handleDragStart( event, this.globalToParentPoint( event.pointer.point ) );
}

trackDragHandler.trackDragged( event, this.globalToParentPoint( event.pointer.point ) );
}

}
return;
}
Expand Down Expand Up @@ -196,7 +203,7 @@ export default class ControlPointNode extends Circle {
model.userControlledPropertySet.trackControlledProperty.set( false );

// If control point dragged out of the control panel, translate the entire track, see #130
if ( !track.physicalProperty.value || ( !track.droppedProperty.value && track.draggable ) ) {
if ( !track.physicalProperty.value ) {

// Only drop a track if nothing else was dragging the track (which caused a flicker), see #282
if ( track.dragSource === dragListener ) {
Expand Down
23 changes: 11 additions & 12 deletions js/common/view/TrackDragHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default class TrackDragHandler extends DragListener {
private readonly model: EnergySkateParkModel;
private readonly modelViewTransform: ModelViewTransform2;
private readonly availableBoundsProperty: Property<Bounds2>;
private readonly startOffset: Vector2 | null;
public readonly startOffset: Vector2 | null;

/**
* @param trackNode the track node that this listener will drag
Expand Down Expand Up @@ -54,15 +54,15 @@ export default class TrackDragHandler extends DragListener {
}

/**
* Start of a drag interaction from an event.
* Start of a drag interaction from an event. TODO: clean up overrideParentPoint usages? See https://github.com/phetsims/energy-skate-park/issues/385
*/
public handleDragStart( event: SceneryEvent ): void {
public handleDragStart( event: SceneryEvent, overrideParentPoint: Vector2 | null = null ): void {

// Move the track to the front when it starts dragging, see #296
// The track is in a layer of tracks (without other nodes) so moving it to the front will work perfectly
this.trackNode.moveToFront();

this.calculateStartOffset( event );
this.calculateStartOffset( event, overrideParentPoint );
}

/**
Expand All @@ -81,7 +81,7 @@ export default class TrackDragHandler extends DragListener {
this.trackDragEnded( event );
}

public trackDragged( event: SceneryEvent<PressListenerDOMEvent> ): void {
public trackDragged( event: SceneryEvent<PressListenerDOMEvent>, overrideParentPoint: Vector2 | null = null ): void {
let snapTargetChanged = false;
const model = this.model;
const track = this.track;
Expand All @@ -91,8 +91,9 @@ export default class TrackDragHandler extends DragListener {

track.draggingProperty.value = true;

const myPt = overrideParentPoint || this.globalToParentPoint( event.pointer.point );
// @ts-expect-error
const parentPoint = this.globalToParentPoint( event.pointer.point ).minus( this.startOffset );
const parentPoint = myPt.minus( this.startOffset );
const position = this.modelViewTransform.viewToModelPosition( parentPoint );

// If the user moved it out of the toolbox above y=0, then make it physically interactive
Expand Down Expand Up @@ -214,10 +215,6 @@ export default class TrackDragHandler extends DragListener {
const track = this.track;
const model = this.model;

// If dropped in the play area, signify that it has been dropped--this will make it so that dragging the control points
// reshapes the track instead of translating it
track.droppedProperty.value = true;

// Check whether the model contains a track so that input listeners for detached elements can't create bugs, see #230
if ( !model.containsTrack( track ) ) { return; }

Expand Down Expand Up @@ -248,11 +245,13 @@ export default class TrackDragHandler extends DragListener {
/**
* Determine the offset point at the start of a drag so that the track translates with the mouse without jumping.
*/
private calculateStartOffset( event: SceneryEvent ): void {
private calculateStartOffset( event: SceneryEvent, overrideParentPoint: Vector2 | null = null ): void {
const startingPosition = this.modelViewTransform.modelToViewPosition( this.track.position );

const pt = overrideParentPoint || event.currentTarget!.globalToParentPoint( event.pointer.point );

// @ts-expect-error
this.startOffset = event.currentTarget.globalToParentPoint( event.pointer.point ).minus( startingPosition );
this.startOffset = pt.minus( startingPosition );
}
}

Expand Down

0 comments on commit c39746d

Please sign in to comment.