samreid
commented
1 year ago This patch can be applied on the branches for https://github.com/phetsims/sun/issues/814
```diff
Subject: [PATCH] Convert to TypeScript, see https://github.com/phetsims/circuit-construction-kit-common/issues/923
---
Index: main/sun/js/ComboBoxListItemNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/sun/js/ComboBoxListItemNode.ts b/main/sun/js/ComboBoxListItemNode.ts
--- a/main/sun/js/ComboBoxListItemNode.ts (revision ab783751c12a322f46aff1d69b25bc388c4c854a)
+++ b/main/sun/js/ComboBoxListItemNode.ts (date 1673068633280)
@@ -76,7 +76,7 @@
// Together, these options make it possible to reorder the combo box items in studio, and save a customized
// simulation with the new order.
- phetioType: IndexedNodeIO,
+ phetioType: IndexedNodeIO(),
phetioState: true,
visiblePropertyOptions: { phetioFeatured: true }
}, providedOptions );
Index: main/circuit-construction-kit-common/js/view/CircuitElementToolNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/circuit-construction-kit-common/js/view/CircuitElementToolNode.ts b/main/circuit-construction-kit-common/js/view/CircuitElementToolNode.ts
--- a/main/circuit-construction-kit-common/js/view/CircuitElementToolNode.ts (revision f26fb91ba016fbfd4ad60bc1bd425b69c29a4cd6)
+++ b/main/circuit-construction-kit-common/js/view/CircuitElementToolNode.ts (date 1673068608394)
@@ -81,7 +81,7 @@
excludeInvisibleChildrenFromBounds: false,
additionalProperty: new BooleanProperty( true ),
- phetioType: IndexedNodeIO,
+ phetioType: IndexedNodeIO( 2 ),
phetioState: true,
visiblePropertyOptions: {
phetioFeatured: true
Index: main/scenery/js/nodes/IndexedNodeIO.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/nodes/IndexedNodeIO.ts b/main/scenery/js/nodes/IndexedNodeIO.ts
--- a/main/scenery/js/nodes/IndexedNodeIO.ts (revision 66facb241ce8824bf004232e6ecb4f1535cf7555)
+++ b/main/scenery/js/nodes/IndexedNodeIO.ts (date 1673069184181)
@@ -25,94 +25,117 @@
// The next index at which a callback will appear in the map. This always increments and we do reuse old indices
let index = 0;
-const IndexedNodeIO = new IOType( 'IndexedNodeIO', {
- valueType: Node,
- documentation: 'Node that can be moved forward/back by index, which specifies z-order and/or layout order',
- supertype: Node.NodeIO,
- toStateObject: node => {
- const stateObject: { index: number | null } = { index: null };
- if ( node.parents[ 0 ] ) {
- assert && assert( node.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
- stateObject.index = node.parents[ 0 ].indexOfChild( node );
- }
- return stateObject;
- },
- applyState: ( node, stateObject ) => {
- const nodeParent = node.parents[ 0 ];
+function getAncestor( node: Node, containerDepth: number ): Node {
+ for ( let i = 0; i < containerDepth; i++ ) {
+ node = node.parents[ 0 ];
+ }
+ return node;
+}
+
+function getContainer( node: Node, containerDepth: number ): Node {
+ return getAncestor( node, containerDepth );
+}
+
+function getContainerChild( node: Node, containerDepth: number ): Node {
+ return getAncestor( node, containerDepth - 1 );
+}
+
+function assertChildHasOneParent( node: Node, containerDepth: number ): void {
+ assert && assert( getContainerChild( node, containerDepth ).parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
+}
+
+function IndexedNodeIO( containerDepth = 1 ): IOType {
+ return new IOType( 'IndexedNodeIO', {
+ valueType: Node,
+ documentation: 'Node that can be moved forward/back by index, which specifies z-order and/or layout order',
+ supertype: Node.NodeIO,
+ toStateObject: node => {
+ const stateObject: { index: number | null } = { index: null };
+
+ // Only work if the node is attached to the scene graph
+ if ( node.parents[ 0 ] ) {
+ assertChildHasOneParent( node, containerDepth );
+ stateObject.index = getContainer( node, containerDepth ).indexOfChild( getContainerChild( node, containerDepth ) );
+ }
+ return stateObject;
+ },
+ applyState: ( node, stateObject ) => {
- if ( nodeParent && stateObject.index ) {
- assert && assert( node.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
+ // Only work if the node is attached to the scene graph
+ if ( node.parents[ 0 ] && stateObject.index ) {
+ assertChildHasOneParent( node, containerDepth );
- // Swap the child at the destination index with current position of this Node, that way the operation is atomic.
- // This implementation assumes that all children are instrumented IndexedNodeIO instances and can have state set
- // on them to "fix them" after this operation. Without this implementation, using Node.moveChildToIndex could blow
- // away another IndexedNode state set. See https://github.com/phetsims/ph-scale/issues/227
- const children = nodeParent.children;
- const currentIndex = nodeParent.indexOfChild( node );
- children[ currentIndex ] = children[ stateObject.index ];
- children[ stateObject.index ] = node;
- nodeParent.setChildren( children );
- }
- },
- stateSchema: {
- index: NullableIO( NumberIO )
- },
- methods: {
- linkIndex: {
- returnType: NumberIO,
- parameterTypes: [ FunctionIO( VoidIO, [ NumberIO ] ) ],
- documentation: 'Following the PropertyIO.link pattern, subscribe for notifications when the index in the parent ' +
- 'changes, and receive a callback with the current value. The return value is a numeric ID for use ' +
- 'with clearLinkIndex.',
- implementation: function( this: Node, listener ) {
+ // Swap the child at the destination index with current position of this Node, that way the operation is atomic.
+ // This implementation assumes that all children are instrumented IndexedNodeIO instances and can have state set
+ // on them to "fix them" after this operation. Without this implementation, using Node.moveChildToIndex could blow
+ // away another IndexedNode state set. See https://github.com/phetsims/ph-scale/issues/227
+ const children = getContainer( node, containerDepth ).children;
+ const currentIndex = getContainer( node, containerDepth ).indexOfChild( getContainerChild( node, containerDepth ) );
+ children[ currentIndex ] = children[ stateObject.index ];
+ children[ stateObject.index ] = getContainerChild( node, containerDepth );
+ getContainer( node, containerDepth ).setChildren( children );
+ }
+ },
+ stateSchema: {
+ index: NullableIO( NumberIO )
+ },
+ methods: {
+ linkIndex: {
+ returnType: NumberIO,
+ parameterTypes: [ FunctionIO( VoidIO, [ NumberIO ] ) ],
+ documentation: 'Following the PropertyIO.link pattern, subscribe for notifications when the index in the parent ' +
+ 'changes, and receive a callback with the current value. The return value is a numeric ID for use ' +
+ 'with clearLinkIndex.',
+ implementation: function( this: Node, listener ) {
- // The callback which signifies the current index
- const callback = () => {
- assert && assert( this.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
- const index = this.parents[ 0 ].indexOfChild( this );
- listener( index );
- };
+ // The callback which signifies the current index
+ const callback = () => {
+ assertChildHasOneParent( this, containerDepth );
+ const index = getContainer( this, containerDepth ).indexOfChild( getContainerChild( this, containerDepth ) );
+ listener( index );
+ };
- assert && assert( this.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
- this.parents[ 0 ].childrenChangedEmitter.addListener( callback );
- callback();
+ assertChildHasOneParent( this, containerDepth );
+ getContainer( this, containerDepth ).childrenChangedEmitter.addListener( callback );
+ callback();
- const myIndex = index;
- map[ myIndex ] = callback;
- index++;
- return myIndex;
- }
- },
- clearLinkIndex: {
- returnType: VoidIO,
- parameterTypes: [ NumberIO ],
- documentation: 'Unlink a listener that has been added using linkIndex, by its numerical ID (like setTimeout/clearTimeout)',
- implementation: function( this: Node, index ) {
- const method = map[ index ];
- assert && assert( this.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
- this.parents[ 0 ].childrenChangedEmitter.removeListener( method );
- delete map[ index ];
- }
- },
- moveForward: {
- returnType: VoidIO,
- parameterTypes: [],
- implementation: function( this: Node ) {
- return this.moveForward();
- },
- documentation: 'Move this node one index forward in each of its parents. If the node is already at the front, this is a no-op.'
- },
+ const myIndex = index;
+ map[ myIndex ] = callback;
+ index++;
+ return myIndex;
+ }
+ },
+ clearLinkIndex: {
+ returnType: VoidIO,
+ parameterTypes: [ NumberIO ],
+ documentation: 'Unlink a listener that has been added using linkIndex, by its numerical ID (like setTimeout/clearTimeout)',
+ implementation: function( this: Node, index ) {
+ const method = map[ index ];
+ assertChildHasOneParent( this, containerDepth );
+ getContainer( this, containerDepth ).childrenChangedEmitter.removeListener( method );
+ delete map[ index ];
+ }
+ },
+ moveForward: {
+ returnType: VoidIO,
+ parameterTypes: [],
+ implementation: function( this: Node ) {
+ return getContainerChild( this, containerDepth ).moveForward();
+ },
+ documentation: 'Move this node one index forward in each of its parents. If the node is already at the front, this is a no-op.'
+ },
- moveBackward: {
- returnType: VoidIO,
- parameterTypes: [],
- implementation: function( this: Node ) {
- return this.moveBackward();
- },
- documentation: 'Move this node one index backward in each of its parents. If the node is already at the back, this is a no-op.'
- }
- }
-} );
+ moveBackward: {
+ returnType: VoidIO,
+ parameterTypes: [],
+ implementation: function( this: Node ) {
+ return getContainerChild( this, containerDepth ).moveBackward();
+ },
+ documentation: 'Move this node one index backward in each of its parents. If the node is already at the back, this is a no-op.'
+ }
+ }
+ } );
+}
scenery.register( 'IndexedNodeIO', IndexedNodeIO );
export default IndexedNodeIO;
\ No newline at end of file
Index: main/geometric-optics/js/common/view/tools/GOToolNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/geometric-optics/js/common/view/tools/GOToolNode.ts b/main/geometric-optics/js/common/view/tools/GOToolNode.ts
--- a/main/geometric-optics/js/common/view/tools/GOToolNode.ts (revision 9051cf3e44517216c3036437eab78010a593ae96)
+++ b/main/geometric-optics/js/common/view/tools/GOToolNode.ts (date 1673068633269)
@@ -59,7 +59,7 @@
phetioInputEnabledPropertyInstrumented: true,
// Make z-ordering of tools stateful, see https://github.com/phetsims/geometric-optics/issues/431
- phetioType: IndexedNodeIO,
+ phetioType: IndexedNodeIO(),
phetioState: true
}, providedOptions );
```
It gets the depth right, but does not handle the separators
From https://github.com/phetsims/circuit-construction-kit-common/issues/630 and for the DC milestone,
This is in part so you can make sure to reposition the wires as desired.
This was broken when Carousel used an extra layer of nesting for positioning the nodes. @matthew-blackman and I think we can correct it by generalizing IndexedNodeIO.