From e6a8bbf589cf258c32dd6b8c88f5e860961db337 Mon Sep 17 00:00:00 2001 From: pixelzoom Date: Mon, 8 Apr 2024 18:56:09 -0600 Subject: [PATCH] fix leaks related to NumberPicker and EquationNode, https://github.com/phetsims/graphing-lines/issues/153 --- js/common/view/EquationNode.ts | 10 ++++++++-- js/linegame/view/GraphTheLineNode.ts | 14 ++++++++------ js/linegame/view/MakeTheEquationNode.ts | 12 +++++++----- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/js/common/view/EquationNode.ts b/js/common/view/EquationNode.ts index 62322cb1..76f44cf4 100644 --- a/js/common/view/EquationNode.ts +++ b/js/common/view/EquationNode.ts @@ -115,8 +115,7 @@ export default class EquationNode extends Node { const minRiseNode = new SlopePicker( new Property( riseRangeProperty.value.min ), new Property( runRangeProperty.value.max ), riseRangeProperty, pickerOptions ); - const maxRunNode = new SlopePicker( - new Property( runRangeProperty.value.max ), + const maxRunNode = new SlopePicker( new Property( runRangeProperty.value.max ), new Property( riseRangeProperty.value.max ), runRangeProperty, pickerOptions ); const minRunNode = new SlopePicker( new Property( runRangeProperty.value.min ), @@ -125,6 +124,13 @@ export default class EquationNode extends Node { // Compute the max width const maxRiseWidth = Math.max( maxRiseNode.width, minRiseNode.width ); const maxRunWidth = Math.max( maxRunNode.width, minRunNode.width ); + + // Clean up the NumberPicker instances, because they link to a LocalizedStringProperty for PDOM. + maxRiseNode.dispose(); + minRiseNode.dispose(); + maxRunNode.dispose(); + minRunNode.dispose(); + return Math.max( maxRiseWidth, maxRunWidth ); } } diff --git a/js/linegame/view/GraphTheLineNode.ts b/js/linegame/view/GraphTheLineNode.ts index ceabdbf3..ec547523 100644 --- a/js/linegame/view/GraphTheLineNode.ts +++ b/js/linegame/view/GraphTheLineNode.ts @@ -50,11 +50,12 @@ export default class GraphTheLineNode extends ChallengeNode { } ); // Answer - const answerBoxNode = new EquationBoxNode( GraphingLinesStrings.lineToGraphStringProperty, challenge.answer.color, boxSize, - ChallengeNode.createEquationNode( new Property( challenge.answer ), challenge.equationForm, { - fontSize: LineGameConstants.STATIC_EQUATION_FONT_SIZE, - slopeUndefinedVisible: false - } ) ); + const answerEquationNode = ChallengeNode.createEquationNode( new Property( challenge.answer ), challenge.equationForm, { + fontSize: LineGameConstants.STATIC_EQUATION_FONT_SIZE, + slopeUndefinedVisible: false + } ); + const answerBoxNode = new EquationBoxNode( GraphingLinesStrings.lineToGraphStringProperty, challenge.answer.color, + boxSize, answerEquationNode ); const guessLineProperty = new Property( Line.Y_EQUALS_X_LINE ); // start with any non-null line const guessEquationNode = ChallengeNode.createEquationNode( guessLineProperty, challenge.equationForm, { @@ -165,12 +166,13 @@ export default class GraphTheLineNode extends ChallengeNode { this.disposeGraphTheLineNode = () => { titleText.dispose(); + answerEquationNode.dispose(); + guessEquationNode.dispose(); answerBoxNode.dispose(); guessBoxNode.dispose(); this.notALineText.dispose(); challenge.guessProperty.unlink( guessObserver ); model.playStateProperty.unlink( playStateObserver ); - guessEquationNode.dispose(); this.graphNode.dispose(); }; } diff --git a/js/linegame/view/MakeTheEquationNode.ts b/js/linegame/view/MakeTheEquationNode.ts index 5eec07dc..cf13a6e2 100644 --- a/js/linegame/view/MakeTheEquationNode.ts +++ b/js/linegame/view/MakeTheEquationNode.ts @@ -46,10 +46,11 @@ export default class MakeTheEquationNode extends ChallengeNode { } ); // Answer - const answerBoxNode = new EquationBoxNode( GraphingLinesStrings.aCorrectEquationStringProperty, challenge.answer.color, boxSize, - ChallengeNode.createEquationNode( new Property( challenge.answer ), challenge.equationForm, { - fontSize: LineGameConstants.STATIC_EQUATION_FONT_SIZE - } ) ); + const answerEquationNode = ChallengeNode.createEquationNode( new Property( challenge.answer ), challenge.equationForm, { + fontSize: LineGameConstants.STATIC_EQUATION_FONT_SIZE + } ); + const answerBoxNode = new EquationBoxNode( GraphingLinesStrings.aCorrectEquationStringProperty, challenge.answer.color, + boxSize, answerEquationNode ); answerBoxNode.visible = false; // Guess @@ -147,9 +148,10 @@ export default class MakeTheEquationNode extends ChallengeNode { titleText.dispose(); answerBoxNode.dispose(); guessBoxNode.dispose(); + answerEquationNode.dispose(); + guessEquationNode.dispose(); challenge.guessProperty.unlink( guessObserver ); model.playStateProperty.unlink( playStateObserver ); - guessEquationNode.dispose(); graphNode.dispose(); }; }