From 371ad838eb03a9d00cb07ab01961c808fc73d979 Mon Sep 17 00:00:00 2001 From: Michael Kauzmann Date: Tue, 4 Apr 2023 12:45:17 -0600 Subject: [PATCH] fix Disposable, removing disposer and options and using disposeEmitter, https://github.com/phetsims/axon/issues/433 --- js/keyboard/TextKeyNode.ts | 10 +++---- .../help/ComboBoxKeyboardHelpSection.ts | 22 ++++++++++------ .../help/GrabReleaseKeyboardHelpSection.ts | 26 ++++++++++++------- js/keyboard/help/KeyboardHelpIconFactory.ts | 2 +- js/keyboard/help/KeyboardHelpSectionRow.ts | 19 ++++++++------ 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/js/keyboard/TextKeyNode.ts b/js/keyboard/TextKeyNode.ts index 520f1380..4272a90b 100644 --- a/js/keyboard/TextKeyNode.ts +++ b/js/keyboard/TextKeyNode.ts @@ -7,7 +7,6 @@ */ import LinkableProperty from '../../../axon/js/LinkableProperty.js'; -import TinyEmitter from '../../../axon/js/TinyEmitter.js'; import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; import optionize from '../../../phet-core/js/optionize.js'; import platform from '../../../phet-core/js/platform.js'; @@ -38,21 +37,18 @@ export default class TextKeyNode extends KeyNode { textMaxWidth: 55, // Long keys like Space, Enter, Tab, Shift are all smaller than this. // by default, key should tightly surround the text, with a bit more horizontal space - xPadding: 11, - - disposeEmitter: new TinyEmitter() - + xPadding: 11 }, providedOptions ); // use RichText because some keys (like page up/page down/caps lock) might span multiple lines const text = new RichText( string, { font: options.font, fill: options.fill, - maxWidth: options.textMaxWidth, - disposer: options.disposeEmitter + maxWidth: options.textMaxWidth } ); super( text, options ); + this.disposeEmitter.addListener( () => text.dispose() ); } /** diff --git a/js/keyboard/help/ComboBoxKeyboardHelpSection.ts b/js/keyboard/help/ComboBoxKeyboardHelpSection.ts index 369fd481..0251b8ee 100644 --- a/js/keyboard/help/ComboBoxKeyboardHelpSection.ts +++ b/js/keyboard/help/ComboBoxKeyboardHelpSection.ts @@ -8,7 +8,6 @@ import PatternStringProperty from '../../../../axon/js/PatternStringProperty.js'; import StringProperty from '../../../../axon/js/StringProperty.js'; -import TinyEmitter from '../../../../axon/js/TinyEmitter.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import optionize from '../../../../phet-core/js/optionize.js'; import sceneryPhet from '../../sceneryPhet.js'; @@ -47,8 +46,7 @@ export default class ComboBoxKeyboardHelpSection extends KeyboardHelpSection { a11yContentTagName: 'ol', // ordered list vBoxOptions: { spacing: 8 // A bit tighter so that it looks like one set of instructions - }, - disposeEmitter: new TinyEmitter() + } }, providedOptions ); // options may be string or TReadOnlyProperty, so ensure that we have a TReadOnlyProperty. @@ -67,14 +65,14 @@ export default class ComboBoxKeyboardHelpSection extends KeyboardHelpSection { providedStringProperty, { thingPlural: thingAsLowerCasePluralStringProperty, thingSingular: thingAsLowerCaseSingularStringProperty - }, { disposer: options.disposeEmitter } ); + } ); ourPatternStringsToDispose.push( patternStringProperty ); return patternStringProperty; }; - const spaceKeyNode = TextKeyNode.space( { disposer: options.disposeEmitter } ); - const enterKeyNode = TextKeyNode.enter( { disposer: options.disposeEmitter } ); - const spaceOrEnterIcon = KeyboardHelpIconFactory.iconOrIcon( spaceKeyNode, enterKeyNode, { disposer: options.disposeEmitter } ); + const spaceKeyNode = TextKeyNode.space(); + const enterKeyNode = TextKeyNode.enter(); + const spaceOrEnterIcon = KeyboardHelpIconFactory.iconOrIcon( spaceKeyNode, enterKeyNode ); const popUpList = KeyboardHelpSectionRow.labelWithIcon( createPatternStringProperty( SceneryPhetStrings.keyboardHelpDialog.comboBox.popUpListPatternStringProperty ), @@ -94,7 +92,7 @@ export default class ComboBoxKeyboardHelpSection extends KeyboardHelpSection { labelInnerContent: createPatternStringProperty( SceneryPhetStrings.a11y.keyboardHelpDialog.comboBox.chooseNewPatternDescriptionStringProperty ) } ); - const escapeKeyNode = TextKeyNode.esc( { disposer: options.disposeEmitter } ); + const escapeKeyNode = TextKeyNode.esc(); const closeWithoutChanging = KeyboardHelpSectionRow.labelWithIcon( SceneryPhetStrings.keyboardHelpDialog.comboBox.closeWithoutChangingStringProperty, escapeKeyNode, { @@ -103,6 +101,14 @@ export default class ComboBoxKeyboardHelpSection extends KeyboardHelpSection { // order the rows of content super( options.headingString, [ popUpList, moveThrough, chooseNew, closeWithoutChanging ], options ); + + this.disposeEmitter.addListener( () => { + escapeKeyNode.dispose(); + spaceOrEnterIcon.dispose(); + enterKeyNode.dispose(); + spaceKeyNode.dispose(); + ourPatternStringsToDispose.forEach( pattern => pattern.dispose() ); + } ); } } diff --git a/js/keyboard/help/GrabReleaseKeyboardHelpSection.ts b/js/keyboard/help/GrabReleaseKeyboardHelpSection.ts index 135dd60c..02b0caa3 100644 --- a/js/keyboard/help/GrabReleaseKeyboardHelpSection.ts +++ b/js/keyboard/help/GrabReleaseKeyboardHelpSection.ts @@ -7,7 +7,6 @@ */ import PatternStringProperty from '../../../../axon/js/PatternStringProperty.js'; -import TinyEmitter from '../../../../axon/js/TinyEmitter.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import { combineOptions } from '../../../../phet-core/js/optionize.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; @@ -33,29 +32,27 @@ export default class GrabReleaseKeyboardHelpSection extends KeyboardHelpSection const options = combineOptions( { // There is only a single paragraph for this section, no list needed in the PDOM - a11yContentTagName: null, - - disposeEmitter: new TinyEmitter() + a11yContentTagName: null }, providedOptions ); // the visible heading string const headingStringProperty = new PatternStringProperty( SceneryPhetStrings.keyboardHelpDialog.grabOrReleaseHeadingPatternStringProperty, { thing: thingAsTitle - }, { disposer: options.disposeEmitter } ); + } ); // the visible label string const labelStringProperty = new PatternStringProperty( SceneryPhetStrings.keyboardHelpDialog.grabOrReleaseLabelPatternStringProperty, { thing: thingAsLowerCase - }, { disposer: options.disposeEmitter } ); + } ); // the string for the PDOM const descriptionStringProperty = new PatternStringProperty( SceneryPhetStrings.a11y.keyboardHelpDialog.grabOrReleaseDescriptionPatternStringProperty, { thing: thingAsLowerCase - }, { disposer: options.disposeEmitter } ); + } ); - const spaceKeyNode = TextKeyNode.space( { disposer: options.disposeEmitter } ); - const enterKeyNode = TextKeyNode.enter( { disposer: options.disposeEmitter } ); - const icons = KeyboardHelpIconFactory.iconOrIcon( spaceKeyNode, enterKeyNode, { disposer: options.disposeEmitter } ); + const spaceKeyNode = TextKeyNode.space(); + const enterKeyNode = TextKeyNode.enter(); + const icons = KeyboardHelpIconFactory.iconOrIcon( spaceKeyNode, enterKeyNode ); const labelWithContentRow = KeyboardHelpSectionRow.labelWithIcon( labelStringProperty, icons, { labelInnerContent: descriptionStringProperty, iconOptions: { @@ -64,6 +61,15 @@ export default class GrabReleaseKeyboardHelpSection extends KeyboardHelpSection } ); super( headingStringProperty, [ labelWithContentRow ], options ); + + this.disposeEmitter.addListener( () => { + icons.dispose(); + enterKeyNode.dispose(); + spaceKeyNode.dispose(); + descriptionStringProperty.dispose(); + labelStringProperty.dispose(); + headingStringProperty.dispose(); + } ); } } diff --git a/js/keyboard/help/KeyboardHelpIconFactory.ts b/js/keyboard/help/KeyboardHelpIconFactory.ts index 8378d956..3e366a2f 100644 --- a/js/keyboard/help/KeyboardHelpIconFactory.ts +++ b/js/keyboard/help/KeyboardHelpIconFactory.ts @@ -70,7 +70,7 @@ export default class KeyboardHelpIconFactory { const icon = KeyboardHelpIconFactory.iconRow( [ new Node( { children: [ leftIcon ] } ), orText, new Node( { children: [ rightIcon ] } ) ], options ); - orText.disposer = icon; + icon.disposeEmitter.addListener( () => orText.dispose() ); return icon; } diff --git a/js/keyboard/help/KeyboardHelpSectionRow.ts b/js/keyboard/help/KeyboardHelpSectionRow.ts index b31453bb..ddd45b1f 100644 --- a/js/keyboard/help/KeyboardHelpSectionRow.ts +++ b/js/keyboard/help/KeyboardHelpSectionRow.ts @@ -13,7 +13,7 @@ import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import optionize, { combineOptions } from '../../../../phet-core/js/optionize.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import { AlignBoxOptions, AlignGroup, HBox, Node, PDOMValueType, RichText, RichTextOptions, Text, VBox, VBoxOptions } from '../../../../scenery/js/imports.js'; -import Disposable, { DisposableOptions } from '../../../../axon/js/Disposable.js'; +import Disposable from '../../../../axon/js/Disposable.js'; import { VoicingResponse } from '../../../../utterance-queue/js/ResponsePacket.js'; import PhetFont from '../../PhetFont.js'; import sceneryPhet from '../../sceneryPhet.js'; @@ -68,7 +68,7 @@ type SelfOptions = { // for every KeyboardHelpSectionRow in the KeyboardHelpSection is read. readingBlockContent?: VoicingResponse | null; }; -type KeyboardHelpSectionRowOptions = SelfOptions & DisposableOptions; +type KeyboardHelpSectionRowOptions = SelfOptions; class KeyboardHelpSectionRow extends Disposable { @@ -90,11 +90,11 @@ class KeyboardHelpSectionRow extends Disposable { public static readonly LABEL_FONT = LABEL_FONT; public constructor( text: Text | RichText, label: Node, icon: Node, providedOptions?: KeyboardHelpSectionRowOptions ) { - const options = optionize()( { + const options = optionize()( { readingBlockContent: null }, providedOptions ); - super( options ); + super(); this.text = text; this.label = label; @@ -135,8 +135,10 @@ class KeyboardHelpSectionRow extends Disposable { readingBlockContent: options.readingBlockContent || options.labelInnerContent } ); - labelText.disposer = keyboardHelpSectionRow; - labelIconGroup.disposer = keyboardHelpSectionRow; + keyboardHelpSectionRow.disposeEmitter.addListener( () => { + labelIconGroup.dispose(); + labelText.dispose(); + } ); return keyboardHelpSectionRow; } @@ -248,15 +250,15 @@ class KeyboardHelpSectionRow extends Disposable { labelFirstIconGroup.createBox( new Node( { children: [ icons[ 0 ] ] } ) ); // create the box to restrain bounds, but a reference isn't necessary const labelBox = labelFirstIconGroup.createBox( labelText ); - // for each of the icons (excluding the last one, add a vertically aligned 'or' text to the right const iconsWithOrText = []; + + // for each of the icons (excluding the last one, add a vertically aligned 'or' text to the right for ( let i = 0; i < icons.length - 1; i++ ) { const orText = new Text( SceneryPhetStrings.keyboardHelpDialog.orStringProperty, { font: LABEL_FONT, maxWidth: OR_TEXT_MAX_WIDTH } ); - // place orText with the icon in an HBox const hBox = new HBox( { children: [ new Node( { children: [ icons[ i ] ] } ), orText ], @@ -282,6 +284,7 @@ class KeyboardHelpSectionRow extends Disposable { const keyboardHelpSectionRow = new KeyboardHelpSectionRow( labelText, labelWithHeightBox, iconsBox, { readingBlockContent: options.readingBlockContent || options.labelInnerContent } ); + keyboardHelpSectionRow.disposeEmitter.addListener( () => { labelFirstIconGroup.dispose(); labelText.dispose();