AnimatedPanZoomListener should generally keep the focused node in view #1558
Assignees
Comments
|
I discussed this with @terracoda who confirmed that the sim should pan to the focused item when it moves around with hotkeys. But in addition, we discussed that panning should occur any time the focused Node moves. For example in BASE, we should follow the moving balloon when it is released. Here is a demo of that much working: https://bayes.colorado.edu/dev/html/jg-tests/balloons-and-static-electricity_en_phet.html That will impact (and simplify) how this issue is solved because we will keep the focused Node in view always instead of panning in response to specific drag/alt input commands. |
jessegreenberg
changed the title
|
From discussion as "Onion. . ." team had it today, this effects Acid Base Solutions and CAV since they both have alt input, so I will mark as blocking from conversation that JG and KP had over in phetsims/center-and-variability#539 |
|
This has been tricker than I thought, but I am getting close. Here are my notes and a patch for picking up on Monday.
This patch with TODOs is progress toward supporting providing the pan target bounds in KeyboardListener. This is working well for BASE, CaV, and acid-base-solutions so far but needs to be finished and get a bit more thought. Subject: [PATCH] Fix bug with images and other forms
---
Index: scenery/js/listeners/KeyboardListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/listeners/KeyboardListener.ts b/scenery/js/listeners/KeyboardListener.ts
--- a/scenery/js/listeners/KeyboardListener.ts (revision 2b58df06678677d27fd6d9421a92c972d5e3cb69)
+++ b/scenery/js/listeners/KeyboardListener.ts (date 1694817778555)
@@ -45,6 +45,7 @@
import optionize from '../../../phet-core/js/optionize.js';
import { EnglishStringToCodeMap, FocusManager, globalKeyStateTracker, scenery, SceneryEvent, TInputListener } from '../imports.js';
import KeyboardUtils from '../accessibility/KeyboardUtils.js';
+import Bounds2 from '../../../dot/js/Bounds2.js';
// NOTE: The typing for ModifierKey and OneKeyStroke is limited TypeScript, there is a limitation to the number of
// entries in a union type. If that limitation is not acceptable remove this typing. OR maybe TypeScript will
@@ -207,6 +208,13 @@
this._fireOnHoldDelay = options.fireOnHoldDelay;
this._fireOnHoldInterval = options.fireOnHoldInterval;
+ this._createPanTargetBounds = null;
+ this._pointerListener = {
+
+ // TODO: Implementation is required for an attached listener, what shoudl this be?
+ interrupt: () => {}
+ };
+
this._activeKeyGroups = [];
this.keysDown = false;
@@ -246,6 +254,17 @@
if ( this.areKeysDownForListener( keyGroup ) &&
keyGroup.keys.includes( globalKeyStateTracker.getLastKeyDown()! ) ) {
+ // Only on the first keygroup activation should this listener be added, and we cannot attach to
+ // an already-attached pointer
+
+ // TODO: Best place to remove this listener? keyup when there are no more active groups and
+ // the pointerListener is attached?
+ if ( this._activeKeyGroups.length === 0 && !event.pointer.isAttached() ) {
+ if ( this._createPanTargetBounds ) {
+ event.pointer.addInputListener( this._pointerListener, true );
+ }
+ }
+
this._activeKeyGroups.push( keyGroup );
this.keysDown = true;
@@ -387,6 +406,14 @@
this._abort && event.abort();
}
+ public setCreatePanTargetBounds( createGlobalPanBounds: ( () => Bounds2 ) | null ): void {
+ this._createPanTargetBounds = createGlobalPanBounds;
+ this._pointerListener.createPanTargetBounds = createGlobalPanBounds;
+
+ // TODO: If set to null, detach the pointer listener from the pointer. That means this listener
+ // will need to keep a reference to the pointer.
+ }
+
/**
* This is part of the scenery Input API (implementing TInputListener). Handle the keydown event when not
* added to the global key events. Target will be the Node, Display, or Pointer this listener was added to.
Index: scenery/js/overlays/HighlightOverlay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/overlays/HighlightOverlay.ts b/scenery/js/overlays/HighlightOverlay.ts
--- a/scenery/js/overlays/HighlightOverlay.ts (revision 2b58df06678677d27fd6d9421a92c972d5e3cb69)
+++ b/scenery/js/overlays/HighlightOverlay.ts (date 1694813560039)
@@ -12,7 +12,7 @@
import BooleanProperty from '../../../axon/js/BooleanProperty.js';
import { Shape } from '../../../kite/js/imports.js';
import optionize from '../../../phet-core/js/optionize.js';
-import { ActivatedReadingBlockHighlight, Display, Focus, FocusManager, HighlightFromNode, HighlightPath, Node, scenery, TOverlay, TPaint, Trail, TransformTracker } from '../imports.js';
+import { ActivatedReadingBlockHighlight, animatedPanZoomSingleton, Display, Focus, FocusManager, HighlightFromNode, HighlightPath, Node, scenery, TOverlay, TPaint, Trail, TransformTracker } from '../imports.js';
import { InteractiveHighlightingNode } from '../accessibility/voicing/InteractiveHighlighting.js';
import { ReadingBlockNode } from '../accessibility/voicing/ReadingBlock.js';
import TProperty from '../../../axon/js/TProperty.js';
@@ -179,6 +179,8 @@
this.focusRootNode.addChild( this.highlightNode );
this.focusRootNode.addChild( this.readingBlockHighlightNode );
+ window.highlightNode = this.highlightNode;
+
this.pdomFocusHighlightsVisibleProperty = options.pdomFocusHighlightsVisibleProperty;
this.interactiveHighlightsVisibleProperty = options.interactiveHighlightsVisibleProperty;
this.readingBlockHighlightsVisibleProperty = options.readingBlockHighlightsVisibleProperty;
@@ -377,6 +379,21 @@
// handle any changes to the focus highlight while the node has focus
node.focusHighlightChangedEmitter.addListener( this.focusHighlightListener );
+ //
+ // // Want to keep the global bounds of the highlight node in view
+ //
+ //
+ //
+ // // Add a listener to the transform tracker that will pan to keep the focus highlight in view
+ //
+ //
+ //
+ // window.myListener = () => {
+ // animatedPanZoomSingleton.listener.keepBoundsInView( this.highlightNode.globalBounds, false );
+ // };
+ // if ( animatedPanZoomSingleton.initialized ) {
+ // this.transformTracker!.addListener( window.myListener );
+ // }
}
/**
Index: soccer-common/js/view/SoccerBallNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/soccer-common/js/view/SoccerBallNode.ts b/soccer-common/js/view/SoccerBallNode.ts
--- a/soccer-common/js/view/SoccerBallNode.ts (revision 029a4154a7621a9da818652a43dfceef7c0b5600)
+++ b/soccer-common/js/view/SoccerBallNode.ts (date 1694816428409)
@@ -120,13 +120,15 @@
// have enough space to move that far. If we make sure that bounds surrounding the SoccerObjectNode have a width
// of 2 model units the pointer will always have enough space to drag the SoccerObjectNode to a new position.
// See https://github.com/phetsims/center-and-variability/issues/88
- dragListener.createPanTargetBounds = () => {
+ this.createPanTargetBounds = () => {
const modelPosition = soccerBall.positionProperty.value;
const modelBounds = new Bounds2( modelPosition.x - 1, modelPosition.y - 1, modelPosition.x + 1, modelPosition.y + 1 );
const viewBounds = modelViewTransform.modelToViewBounds( modelBounds );
return this.parentToGlobalBounds( viewBounds );
};
+ dragListener.createPanTargetBounds = this.createPanTargetBounds;
+
this.addInputListener( dragListener );
// For PhET-iO, allow clients to shut off interactivity via this Property.
@@ -159,7 +161,7 @@
} );
soccerBall.resetEmitter.addListener( () => {
- this.focusable = false;
+ // this.focusable = false;
this.pickable = false;
this.mouseArea = Shape.rectangle( 0, 0, 0, 0 );
this.touchArea = Shape.rectangle( 0, 0, 0, 0 );
@@ -168,7 +170,7 @@
this.addLinkedElement( soccerBall );
// Not focusable until the ball has been kicked into the play area
- this.focusable = false;
+ // this.focusable = false;
super.addDebugText( soccerBall );
}
Index: scenery/js/listeners/AnimatedPanZoomListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/listeners/AnimatedPanZoomListener.ts b/scenery/js/listeners/AnimatedPanZoomListener.ts
--- a/scenery/js/listeners/AnimatedPanZoomListener.ts (revision 2b58df06678677d27fd6d9421a92c972d5e3cb69)
+++ b/scenery/js/listeners/AnimatedPanZoomListener.ts (date 1694818155625)
@@ -16,7 +16,7 @@
import EventType from '../../../tandem/js/EventType.js';
import isSettingPhetioStateProperty from '../../../tandem/js/isSettingPhetioStateProperty.js';
import PhetioAction from '../../../tandem/js/PhetioAction.js';
-import { EventIO, Focus, FocusManager, globalKeyStateTracker, Intent, KeyboardDragListener, KeyboardUtils, KeyboardZoomUtils, KeyStateTracker, Mouse, MultiListenerPress, Node, PanZoomListener, PanZoomListenerOptions, PDOMPointer, PDOMUtils, Pointer, PressListener, scenery, SceneryEvent, Trail } from '../imports.js';
+import { EventIO, Focus, FocusManager, globalKeyStateTracker, Intent, KeyboardDragListener, KeyboardUtils, KeyboardZoomUtils, KeyStateTracker, Mouse, MultiListenerPress, Node, PanZoomListener, PanZoomListenerOptions, PDOMPointer, PDOMUtils, Pointer, PressListener, scenery, SceneryEvent, Trail, TransformTracker } from '../imports.js';
import optionize, { EmptySelfOptions } from '../../../phet-core/js/optionize.js';
import Tandem from '../../../tandem/js/Tandem.js';
@@ -102,6 +102,8 @@
// to scale the target Node
private trackpadGestureStartScale = 1;
+ private _transformTracker: TransformTracker | null = null;
+
private readonly disposeAnimatedPanZoomListener: () => void;
/**
@@ -192,7 +194,39 @@
globalKeyStateTracker.keydownEmitter.addListener( this.windowKeydown.bind( this ) );
const displayFocusListener = ( focus: Focus | null ) => {
- if ( focus && this.getCurrentScale() > 1 ) {
+ if ( this._transformTracker ) {
+ this._transformTracker.dispose();
+ this._transformTracker = null;
+ }
+
+ if ( focus ) {
+ let trailToTrack = focus.trail;
+ if ( focus.trail.containsNode( this._targetNode ) ) {
+
+ // Track transforms to the focused Node, but exclude the targetNode so that repositions during pan don't
+ // trigger another transform update.
+ const indexOfTarget = focus.trail.nodes.indexOf( this._targetNode );
+ const indexOfLeaf = focus.trail.nodes.length; // end of slice is not included
+ trailToTrack = focus.trail.slice( indexOfTarget, indexOfLeaf );
+ }
+
+ this._transformTracker = new TransformTracker( trailToTrack );
+ this._transformTracker.addListener( () => {
+ if ( this.getCurrentScale() > 1 ) {
+
+ // globalBounds needs full trail to the focused Node (not the subtrail used by TransformTracker)
+
+ // TODO: We need a way to get the panTargetBounds from the PDOMPointer here. If we listen to a
+ // global focus event that could be possible. Or we could look for the PDOMPointer through the
+ // focus.display._input!
+ const globalBounds = focus.trail.localToGlobalBounds( focus.trail.lastNode().localBounds );
+ // const globalBounds = window.highlightNode.globalBounds;
+ console.log( globalBounds );
+ this.keepBoundsInView( globalBounds, true );
+ }
+ } );
+
+ // Pan to the focus trail right away if it is off-screen
this.keepTrailInView( focus.trail );
}
};
@@ -221,6 +255,10 @@
// @ts-expect-error - Event type for this Safari specific event isn't available yet
boundGestureChangeListener && window.removeEventListener( 'gestureChange', boundGestureChangeListener );
+ if ( this._transformTracker ) {
+ this._transformTracker.dispose();
+ }
+
FocusManager.pdomFocusProperty.unlink( displayFocusListener );
};
}
Index: soccer-common/js/view/SoccerSceneView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/soccer-common/js/view/SoccerSceneView.ts b/soccer-common/js/view/SoccerSceneView.ts
--- a/soccer-common/js/view/SoccerSceneView.ts (revision 029a4154a7621a9da818652a43dfceef7c0b5600)
+++ b/soccer-common/js/view/SoccerSceneView.ts (date 1694816817007)
@@ -288,6 +288,20 @@
}
} );
+ focusedSoccerBallProperty.link( focusedBall => {
+ if ( focusedBall ) {
+ keyboardListener.setCreatePanTargetBounds( () => {
+ const modelPosition = focusedBall.positionProperty.value;
+ const modelBounds = new Bounds2( modelPosition.x - 1, modelPosition.y - 1, modelPosition.x + 1, modelPosition.y + 1 );
+ const viewBounds = modelViewTransform.modelToViewBounds( modelBounds );
+ return backLayerSoccerBallLayer.parentToGlobalBounds( viewBounds );
+ } );
+ }
+ else {
+ keyboardListener.setCreatePanTargetBounds( null );
+ }
+ } );
+
// Set the outer group focus region to cover the entire area where soccer balls may land, translate lower so it also includes the number line and labels
this.focusHighlightPath = new HighlightPath( null, {
outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR,
|
|
Coming back to this, I don't think that will fully work either. In the case that a Node has focus and the transform changes in a scripted way there is no input and no Pointer to have globalTargetBounds. I think what we must do is add a field to ParallelDOM, which will be a function to calculate the "pan target bounds". The function will likely use a trail and require there is only one instance of a Node. But that is acceptable because focusable Nodes already cannot have more than one Instance. I will see if I can do a memory profile of the impact of adding another field to ParallelDOM before going too far into it. |
|
@jessegreenberg - Feel free to reach out to @jonathanolson for collaborative discussion as well. |
|
New patch to switch machines (not commitable): Subject: [PATCH] Fix bug with images and other forms
---
Index: scenery/js/overlays/HighlightOverlay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/overlays/HighlightOverlay.ts b/scenery/js/overlays/HighlightOverlay.ts
--- a/scenery/js/overlays/HighlightOverlay.ts (revision 2b58df06678677d27fd6d9421a92c972d5e3cb69)
+++ b/scenery/js/overlays/HighlightOverlay.ts (date 1694813560039)
@@ -12,7 +12,7 @@
import BooleanProperty from '../../../axon/js/BooleanProperty.js';
import { Shape } from '../../../kite/js/imports.js';
import optionize from '../../../phet-core/js/optionize.js';
-import { ActivatedReadingBlockHighlight, Display, Focus, FocusManager, HighlightFromNode, HighlightPath, Node, scenery, TOverlay, TPaint, Trail, TransformTracker } from '../imports.js';
+import { ActivatedReadingBlockHighlight, animatedPanZoomSingleton, Display, Focus, FocusManager, HighlightFromNode, HighlightPath, Node, scenery, TOverlay, TPaint, Trail, TransformTracker } from '../imports.js';
import { InteractiveHighlightingNode } from '../accessibility/voicing/InteractiveHighlighting.js';
import { ReadingBlockNode } from '../accessibility/voicing/ReadingBlock.js';
import TProperty from '../../../axon/js/TProperty.js';
@@ -179,6 +179,8 @@
this.focusRootNode.addChild( this.highlightNode );
this.focusRootNode.addChild( this.readingBlockHighlightNode );
+ window.highlightNode = this.highlightNode;
+
this.pdomFocusHighlightsVisibleProperty = options.pdomFocusHighlightsVisibleProperty;
this.interactiveHighlightsVisibleProperty = options.interactiveHighlightsVisibleProperty;
this.readingBlockHighlightsVisibleProperty = options.readingBlockHighlightsVisibleProperty;
@@ -377,6 +379,21 @@
// handle any changes to the focus highlight while the node has focus
node.focusHighlightChangedEmitter.addListener( this.focusHighlightListener );
+ //
+ // // Want to keep the global bounds of the highlight node in view
+ //
+ //
+ //
+ // // Add a listener to the transform tracker that will pan to keep the focus highlight in view
+ //
+ //
+ //
+ // window.myListener = () => {
+ // animatedPanZoomSingleton.listener.keepBoundsInView( this.highlightNode.globalBounds, false );
+ // };
+ // if ( animatedPanZoomSingleton.initialized ) {
+ // this.transformTracker!.addListener( window.myListener );
+ // }
}
/**
Index: soccer-common/js/view/SoccerBallNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/soccer-common/js/view/SoccerBallNode.ts b/soccer-common/js/view/SoccerBallNode.ts
--- a/soccer-common/js/view/SoccerBallNode.ts (revision 029a4154a7621a9da818652a43dfceef7c0b5600)
+++ b/soccer-common/js/view/SoccerBallNode.ts (date 1695062446693)
@@ -120,12 +120,13 @@
// have enough space to move that far. If we make sure that bounds surrounding the SoccerObjectNode have a width
// of 2 model units the pointer will always have enough space to drag the SoccerObjectNode to a new position.
// See https://github.com/phetsims/center-and-variability/issues/88
- dragListener.createPanTargetBounds = () => {
+ this.createFocusPanTargetBounds = () => {
const modelPosition = soccerBall.positionProperty.value;
const modelBounds = new Bounds2( modelPosition.x - 1, modelPosition.y - 1, modelPosition.x + 1, modelPosition.y + 1 );
const viewBounds = modelViewTransform.modelToViewBounds( modelBounds );
return this.parentToGlobalBounds( viewBounds );
};
+ dragListener.createPanTargetBounds = this.createFocusPanTargetBounds;
this.addInputListener( dragListener );
@@ -159,7 +160,7 @@
} );
soccerBall.resetEmitter.addListener( () => {
- this.focusable = false;
+ // this.focusable = false;
this.pickable = false;
this.mouseArea = Shape.rectangle( 0, 0, 0, 0 );
this.touchArea = Shape.rectangle( 0, 0, 0, 0 );
@@ -168,7 +169,7 @@
this.addLinkedElement( soccerBall );
// Not focusable until the ball has been kicked into the play area
- this.focusable = false;
+ // this.focusable = false;
super.addDebugText( soccerBall );
}
Index: scenery/js/accessibility/pdom/ParallelDOM.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/accessibility/pdom/ParallelDOM.ts b/scenery/js/accessibility/pdom/ParallelDOM.ts
--- a/scenery/js/accessibility/pdom/ParallelDOM.ts (revision 2b58df06678677d27fd6d9421a92c972d5e3cb69)
+++ b/scenery/js/accessibility/pdom/ParallelDOM.ts (date 1695058681313)
@@ -142,6 +142,7 @@
import TinyForwardingProperty from '../../../../axon/js/TinyForwardingProperty.js';
import TProperty from '../../../../axon/js/TProperty.js';
import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js';
+import Bounds2 from '../../../../dot/js/Bounds2.js';
const INPUT_TAG = PDOMUtils.TAGS.INPUT;
const P_TAG = PDOMUtils.TAGS.P;
@@ -229,6 +230,8 @@
'ariaDescribedbyAssociations',
'activeDescendantAssociations',
+ 'createPanTargetBounds',
+
'positionInPDOM',
'pdomTransformSourceNode'
@@ -283,6 +286,9 @@
ariaDescribedbyAssociations?: Association[]; // sets the list of aria-describedby associations between from this node to others (including itself)
activeDescendantAssociations?: Association[]; // sets the list of aria-activedescendant associations between from this node to others (including itself)
+ // Sets the function that creates the pan target bounds for this node
+ createFocusPanTargetBounds?: ( () => Bounds2 ) | null;
+
positionInPDOM?: boolean; // Sets whether the node's DOM elements are positioned in the viewport
pdomTransformSourceNode?: Node | null; // { sets the node that controls primary sibling element positioning in the display, see setPDOMTransformSourceNode()
@@ -503,6 +509,10 @@
// pdomTransformSourceNode cannot use DAG.
private _pdomTransformSourceNode: Node | null;
+ // If this is provided, the AnimatedPanZoomListener will attempt to keep this Node in view as long as it has
+ // focus
+ private _createFocusPanTargetBounds: ( () => Bounds2 ) | null;
+
// Contains information about what pdom displays
// this node is "visible" for, see PDOMDisplaysInfo.js for more information.
// (scenery-internal)
@@ -593,6 +603,7 @@
this._pdomOrder = null;
this._pdomParent = null;
this._pdomTransformSourceNode = null;
+ this._createFocusPanTargetBounds = null;
this._pdomDisplaysInfo = new PDOMDisplaysInfo( this as unknown as Node );
this._pdomInstances = [];
this._positionInPDOM = false;
@@ -2563,6 +2574,23 @@
return this._pdomTransformSourceNode;
}
+ public setCreateFocusPanTargetBounds( createFocusPanTargetBounds: null | ( () => Bounds2 ) ): void {
+ this._createFocusPanTargetBounds = createFocusPanTargetBounds;
+ }
+
+
+ public getCreateFocusPanTargetBounds(): null | ( () => Bounds2 ) {
+ return this._createFocusPanTargetBounds;
+ }
+
+ public set createFocusPanTargetBounds( createFocusPanTargetBounds: null | ( () => Bounds2 ) ) {
+ this.setCreateFocusPanTargetBounds( createFocusPanTargetBounds );
+ }
+
+ public get createFocusPanTargetBounds(): null | ( () => Bounds2 ) {
+ return this.getCreateFocusPanTargetBounds();
+ }
+
/**
* Sets whether the PDOM sibling elements are positioned in the correct place in the viewport. Doing so is a
* requirement for custom gestures on touch based screen readers. However, doing this DOM layout is expensive so
Index: scenery/js/listeners/AnimatedPanZoomListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/listeners/AnimatedPanZoomListener.ts b/scenery/js/listeners/AnimatedPanZoomListener.ts
--- a/scenery/js/listeners/AnimatedPanZoomListener.ts (revision 2b58df06678677d27fd6d9421a92c972d5e3cb69)
+++ b/scenery/js/listeners/AnimatedPanZoomListener.ts (date 1695064177328)
@@ -16,7 +16,7 @@
import EventType from '../../../tandem/js/EventType.js';
import isSettingPhetioStateProperty from '../../../tandem/js/isSettingPhetioStateProperty.js';
import PhetioAction from '../../../tandem/js/PhetioAction.js';
-import { EventIO, Focus, FocusManager, globalKeyStateTracker, Intent, KeyboardDragListener, KeyboardUtils, KeyboardZoomUtils, KeyStateTracker, Mouse, MultiListenerPress, Node, PanZoomListener, PanZoomListenerOptions, PDOMPointer, PDOMUtils, Pointer, PressListener, scenery, SceneryEvent, Trail } from '../imports.js';
+import { EventIO, Focus, FocusManager, globalKeyStateTracker, Intent, KeyboardDragListener, KeyboardUtils, KeyboardZoomUtils, KeyStateTracker, Mouse, MultiListenerPress, Node, PanZoomListener, PanZoomListenerOptions, PDOMPointer, PDOMUtils, Pointer, PressListener, scenery, SceneryEvent, Trail, TransformTracker } from '../imports.js';
import optionize, { EmptySelfOptions } from '../../../phet-core/js/optionize.js';
import Tandem from '../../../tandem/js/Tandem.js';
@@ -102,6 +102,8 @@
// to scale the target Node
private trackpadGestureStartScale = 1;
+ private _transformTracker: TransformTracker | null = null;
+
private readonly disposeAnimatedPanZoomListener: () => void;
/**
@@ -192,7 +194,51 @@
globalKeyStateTracker.keydownEmitter.addListener( this.windowKeydown.bind( this ) );
const displayFocusListener = ( focus: Focus | null ) => {
- if ( focus && this.getCurrentScale() > 1 ) {
+ if ( this._transformTracker ) {
+ this._transformTracker.dispose();
+ this._transformTracker = null;
+ }
+
+ if ( focus ) {
+ const lastNode = focus.trail.lastNode();
+
+ let trailToTrack = focus.trail;
+ if ( focus.trail.containsNode( this._targetNode ) ) {
+
+ // Track transforms to the focused Node, but exclude the targetNode so that repositions during pan don't
+ // trigger another transform update.
+ const indexOfTarget = focus.trail.nodes.indexOf( this._targetNode );
+ const indexOfLeaf = focus.trail.nodes.length; // end of slice is not included
+ trailToTrack = focus.trail.slice( indexOfTarget, indexOfLeaf );
+ }
+
+ this._transformTracker = new TransformTracker( trailToTrack );
+ this._transformTracker.addListener( () => {
+ if ( this.getCurrentScale() > 1 ) {
+
+ // globalBounds needs full trail to the focused Node (not the subtrail used by TransformTracker)
+
+ // TODO: We need a way to get the panTargetBounds from the PDOMPointer here. If we listen to a
+ // global focus event that could be possible. Or we could look for the PDOMPointer through the
+ // focus.display._input!
+ let globalBounds: Bounds2;
+ if ( lastNode.createFocusPanTargetBounds ) {
+
+ // This Node has a custom bounds area that we need to keep in view
+ globalBounds = lastNode.createFocusPanTargetBounds();
+ }
+ else {
+
+ // by default, use the global bounds of the Node
+ globalBounds = focus.trail.localToGlobalBounds( focus.trail.lastNode().localBounds );
+ }
+
+ // const globalBounds = window.highlightNode.globalBounds;
+ this.keepBoundsInView( globalBounds, true );
+ }
+ } );
+
+ // Pan to the focus trail right away if it is off-screen
this.keepTrailInView( focus.trail );
}
};
@@ -221,6 +267,10 @@
// @ts-expect-error - Event type for this Safari specific event isn't available yet
boundGestureChangeListener && window.removeEventListener( 'gestureChange', boundGestureChangeListener );
+ if ( this._transformTracker ) {
+ this._transformTracker.dispose();
+ }
+
FocusManager.pdomFocusProperty.unlink( displayFocusListener );
};
}
@@ -518,19 +568,19 @@
sceneryLog && sceneryLog.InputListener && sceneryLog.pop();
}
}
-
- if ( KeyboardUtils.isMovementKey( domEvent ) ) {
- if ( keyboardDragIntent ) {
-
- // Look for any attached pointers if we are dragging with a keyboard and add them to the list. When dragging
- // stops the Pointer listener is detached and the pointer is removed from the list in `step()`.
- if ( event.pointer.isAttached() ) {
- if ( !this._attachedPointers.includes( event.pointer ) ) {
- this._attachedPointers.push( event.pointer );
- }
- }
- }
- }
+ //
+ // if ( KeyboardUtils.isMovementKey( domEvent ) ) {
+ // if ( keyboardDragIntent ) {
+ //
+ // // Look for any attached pointers if we are dragging with a keyboard and add them to the list. When dragging
+ // // stops the Pointer listener is detached and the pointer is removed from the list in `step()`.
+ // if ( event.pointer.isAttached() ) {
+ // if ( !this._attachedPointers.includes( event.pointer ) ) {
+ // this._attachedPointers.push( event.pointer );
+ // }
+ // }
+ // }
+ // }
}
/**
@@ -820,9 +870,9 @@
* Pan to a provided Node, attempting to place the node in the center of the transformedPanBounds. It may not end
* up exactly in the center since we have to make sure panBounds are completely filled with targetNode content.
*/
- public panToNode( node: Node ): void {
+ public panToNode( node: Node, panToCenter = true ): void {
assert && assert( this._panBounds.isFinite(), 'panBounds should be defined when panning.' );
- this.keepBoundsInView( node.globalBounds, true );
+ this.keepBoundsInView( node.globalBounds, panToCenter );
}
/**
Index: balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js b/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js
--- a/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js (revision 3eee435a586db7dc8c94615ba1fbc13c3defeec4)
+++ b/balloons-and-static-electricity/js/balloons-and-static-electricity/view/BalloonNode.js (date 1695058793310)
@@ -405,6 +405,10 @@
tandem: tandem.createTandem( 'grabDragInteraction' )
} );
+ grabDragTargetNode.createFocusPanTargetBounds = () => {
+ return grabDragTargetNode.globalBounds.shiftedY( -900 );
+ };
+
// jump to the wall on 'J + W'
this.keyboardDragListener.hotkeys = [
{
Index: soccer-common/js/view/SoccerSceneView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/soccer-common/js/view/SoccerSceneView.ts b/soccer-common/js/view/SoccerSceneView.ts
--- a/soccer-common/js/view/SoccerSceneView.ts (revision 029a4154a7621a9da818652a43dfceef7c0b5600)
+++ b/soccer-common/js/view/SoccerSceneView.ts (date 1695064332064)
@@ -7,7 +7,7 @@
* @author Sam Reid (PhET Interactive Simulations)
*/
-import { HighlightFromNode, HighlightPath, InteractiveHighlightingNode, KeyboardListener, Node } from '../../../scenery/js/imports.js';
+import { animatedPanZoomSingleton, HighlightFromNode, HighlightPath, InteractiveHighlightingNode, KeyboardListener, Node } from '../../../scenery/js/imports.js';
import SoccerBallNode from './SoccerBallNode.js';
import { SoccerBallPhase } from '../model/SoccerBallPhase.js';
import SoccerSceneModel from '../model/SoccerSceneModel.js';
@@ -173,6 +173,10 @@
}
hasKeyboardFocusProperty.value = true;
+
+ if ( focusedSoccerBallProperty.value !== null ) {
+ animatedPanZoomSingleton.listener.panToNode( soccerBallMap.get( focusedSoccerBallProperty.value )!, true );
+ }
},
blur: () => {
isSoccerBallKeyboardGrabbedProperty.value = false;
@@ -252,6 +256,8 @@
soccerBall.valueProperty.value = physicalRange.constrainValue( soccerBall.valueProperty.value! + delta );
soccerBall.toneEmitter.emit( soccerBall.valueProperty.value );
}
+
+ animatedPanZoomSingleton.listener.panToNode( soccerBallMap.get( focusedSoccerBallProperty.value )!, false );
}
else if ( keysPressed === 'enter' || keysPressed === 'space' ) {
isSoccerBallKeyboardGrabbedProperty.value = !isSoccerBallKeyboardGrabbedProperty.value;
@@ -288,6 +294,12 @@
}
} );
+ // focusedSoccerBallProperty.link( focusedBall => {
+ // if ( focusedBall ) {
+ // animatedPanZoomSingleton.listener.panToNode( soccerBallMap.get( focusedBall )! );
+ // }
+ // } );
+
// Set the outer group focus region to cover the entire area where soccer balls may land, translate lower so it also includes the number line and labels
this.focusHighlightPath = new HighlightPath( null, {
outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR,
|
jessegreenberg
added a commit
to phetsims/sun
that referenced
this issue
Sep 18, 2023
jessegreenberg
added a commit
that referenced
this issue
Sep 18, 2023
jessegreenberg
added a commit
that referenced
this issue
Sep 19, 2023
|
OK the change using TransformTracker was committed. I decided not to support cases like CaV. I that instance a totally custom focus management implementation (not using scenery's focus) was used and so it doesn't seem beneficial to add new functionality just to support it. Over there, controlling the pan directly with
I reached out to @jonathanolson for a quick synchronous review which we will do soon. |
jessegreenberg
added a commit
to phetsims/sun
that referenced
this issue
Sep 19, 2023
|
@jonathanolson and I met to review the changes and he recommended the following improvements:
|
jessegreenberg
added a commit
that referenced
this issue
Oct 12, 2023
… observable bounds changes for the animatedPanZoomSingleton, see #1558
|
The above commit has this working. This change adds the new Property and replaces setCreateFocusPanTargetBounds, and has the bounds in the Node's local coordinate frame. Ready to close. I tested in BASE by setting a set of bounds on the balloon that were shifted during animation and observed the pan/zoom listener move around the sim as I expected. I used a combination of alt input and mouse input. I ran local aqua and unit tests. Closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Originally reported in phetsims/geometric-optics#471
The AnimatedPanZoomListener should try to keep Nodes that are focused in view when they move with custom key commands. The AnimatedPanZomListener/KeyboardListener needs to do this automatically.
Sometimes, it does this but accidentally for a reason I found in the referenced issue:
The text was updated successfully, but these errors were encountered: