Skip to content

Commit

Permalink
fix Disposable, removing disposer and options and using disposeEmitter,
Browse files Browse the repository at this point in the history
  • Loading branch information
zepumph committed Apr 4, 2023
1 parent 4912a31 commit 371ad83
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 34 deletions.
10 changes: 3 additions & 7 deletions js/keyboard/TextKeyNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() );
}

/**
Expand Down
22 changes: 14 additions & 8 deletions js/keyboard/help/ComboBoxKeyboardHelpSection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string>, so ensure that we have a TReadOnlyProperty<string>.
Expand All @@ -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 ),
Expand All @@ -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, {
Expand All @@ -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() );
} );
}
}

Expand Down
26 changes: 16 additions & 10 deletions js/keyboard/help/GrabReleaseKeyboardHelpSection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,29 +32,27 @@ export default class GrabReleaseKeyboardHelpSection extends KeyboardHelpSection
const options = combineOptions<KeyboardHelpSectionOptions>( {

// 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: {
Expand All @@ -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();
} );
}
}

Expand Down
2 changes: 1 addition & 1 deletion js/keyboard/help/KeyboardHelpIconFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
19 changes: 11 additions & 8 deletions js/keyboard/help/KeyboardHelpSectionRow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {

Expand All @@ -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<KeyboardHelpSectionRowOptions, SelfOptions, DisposableOptions>()( {
const options = optionize<KeyboardHelpSectionRowOptions, SelfOptions>()( {
readingBlockContent: null
}, providedOptions );

super( options );
super();

this.text = text;
this.label = label;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 ],
Expand All @@ -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();
Expand Down

0 comments on commit 371ad83

Please sign in to comment.