Skip to content

Commit 8987f4f

Browse files
committed
Add dispose for unlink playObjectTypeProperty, see phetsims/number-play#22
1 parent 2ce84d6 commit 8987f4f

File tree

3 files changed

+26
-7
lines changed

3 files changed

+26
-7
lines changed

js/common/view/BasePictorialNode.js

+20-3
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ class BasePictorialNode extends Node {
2929
// Translate everything by our offset
3030
this.translation = baseNumber.offset;
3131

32+
// @private {EnumerationProperty.<PlayObjectType>}
33+
this.playObjectTypeProperty = playObjectTypeProperty;
34+
3235
// saves the case from when value is 0
3336
value = Math.max( baseNumber.numberValue, value );
3437

3538
let backgroundNode;
36-
3739
const objectWidth = CountingCommonConstants.PLAY_OBJECT_SIZE.width;
3840
const objectHeight = CountingCommonConstants.PLAY_OBJECT_SIZE.width;
3941
const stackOffset = 10;
@@ -60,6 +62,7 @@ class BasePictorialNode extends Node {
6062
}
6163

6264
// add and position the object images
65+
const objectImages = [];
6366
for ( let i = 0; i < value; i++ ) {
6467
const offset = ( sideMargin + i * stackOffset );
6568
const objectImage = new Image( CountingCommonConstants.PLAY_OBJECT_TYPE_TO_IMAGE[ playObjectTypeProperty.value ], {
@@ -69,10 +72,16 @@ class BasePictorialNode extends Node {
6972
y: offset
7073
} );
7174
this.addChild( objectImage );
72-
playObjectTypeProperty.link( playObjectType => {
75+
objectImages.push( objectImage );
76+
}
77+
78+
// @private
79+
this.playObjectTypeListener = playObjectType => {
80+
objectImages.forEach( objectImage => {
7381
objectImage.image = CountingCommonConstants.PLAY_OBJECT_TYPE_TO_IMAGE[ playObjectType ];
7482
} );
75-
}
83+
};
84+
this.playObjectTypeProperty.link( this.playObjectTypeListener );
7685

7786
// TODO: these should be elminated with future designs, see https://github.com/phetsims/number-play/issues/19
7887
// add the grippy lines if this number is on the top layer
@@ -92,6 +101,14 @@ class BasePictorialNode extends Node {
92101
this.addChild( grippyLines );
93102
}
94103
}
104+
105+
/**
106+
* @public
107+
*/
108+
dispose() {
109+
this.playObjectTypeProperty.unlink( this.playObjectTypeListener );
110+
super.dispose();
111+
}
95112
}
96113

97114
countingCommon.register( 'BasePictorialNode', BasePictorialNode );

js/common/view/CountingCommonView.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,8 @@ class CountingCommonView extends ScreenView {
127127
const paperNumberNode = this.findPaperNumberNode( paperNumber );
128128

129129
delete this.paperNumberNodeMap[ paperNumberNode.paperNumber.id ];
130-
this.paperNumberLayerNode.removeChild( paperNumberNode );
131-
paperNumberNode.detachListeners();
132-
133130
this.closestDragListener.removeDraggableItem( paperNumberNode );
131+
paperNumberNode.dispose();
134132
}
135133

136134
/**

js/common/view/PaperNumberNode.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -265,10 +265,14 @@ class PaperNumberNode extends Node {
265265
* Removes listeners from the model. Should be called when removed from the scene graph.
266266
* @public
267267
*/
268-
detachListeners() {
268+
dispose() {
269269
this.paperNumber.positionProperty.unlink( this.translationListener );
270270
this.paperNumber.numberValueProperty.unlink( this.updateNumberListener );
271271
this.paperNumber.userControlledProperty.unlink( this.userControlledListener );
272+
273+
// remove any listeners on the children before detaching them
274+
this.numberImageContainer.children.forEach( child => child.dispose() );
275+
super.dispose();
272276
}
273277

274278
/**

0 commit comments

Comments
 (0)