State wrapper error in AmplitudeKeypadDialog

[pixelzoom]

When fuzz testing the State wrapper with phet-io-wrappers/state/?sim=fourier-making-waves&phetioDebug&fuzz, this error occurs:

AmplitudeKeypadDialog.js? [sm]:202 Uncaught TypeError: this.closeCallback is not a function
    at AmplitudeKeypadDialog.hide (AmplitudeKeypadDialog.js? [sm]:202:10)
    at fire (BarrierRectangle.js? [sm]:47:57)
    at TinyEmitter.emit (TinyEmitter.ts:93:9)
    at Emitter.emit (Emitter.ts:65:22)
    at FireListener.fire (FireListener.ts:100:23)
    at FireListener.ts:148:14
    at FireListener.onRelease (PressListener.ts:750:17)
    at PhetioAction.execute (PhetioAction.ts:123:17)
    at FireListener.release (PressListener.ts:508:25)
    at FireListener.release (FireListener.ts:141:11)

AmplitudeKeypadDialog is this dialog, opened by pressing on one of the amplitude values shown above the amplitude sliders.

AmplitudeKeypadDialog has field this.closeCallback. It's set when show is called, then called (and cleared) when hide is called. So what I guess is happening is that the dialog is somehow being opened by the State wrapper without calling show.

AmplitudeKeypadDialog has another field, this.enterCallback, that I suspect would have a similar problem. It's set when show is called, then called (and cleared) when the Enter button is pressed.

[pixelzoom]

A few things...

• PhET-iO is probably restoring Dialog isShowingProperty, and that's not doing the same thing as the user opening the dialog.

• Dialog has option hideCallback, which should probably be used instead of AmplitudeKeypadDialog's this.closeCallback.

• AmplitudeKeypadDialog overrides show and changes its signature. That seems problematic. I.e.:

  show( order, enterCallback, closeCallback ) {
    // ... do stuff specific to AmplitudeKeypadDialog
    super.show();
  }

• One instance of AmplitudeKeypadDialog is currently (re)used for all of the amplitudes per Screen, which is why closeCallback and enterCallback are needed. It would be easier to avoid this problem by creating one AmplitudeKeypadDialog instance per amplitude. But then we'd have 22 instances of AmplitudeKeypadDialog, instead of 2.

[pixelzoom]

It would be easier to avoid this problem by creating one AmplitudeKeypadDialog instance per amplitude.

I investigated this approach, and it looks promising. But it was not entirely straightforward, and I ran into problems with emphasized harmonics not behaving properly, followed by a crash. So I bailed for now, and will defer until we resume work on this sim.

[pixelzoom]

When this sim was converted to TypeScript in https://github.com/phetsims/fourier-making-waves/commit/600727cafa89370ef97ef8a664e033aeaa1aaaa3, the following change was made:

  public override hide(): void {
    super.hide();

    this.interruptSubtreeInput();
-   this.closeCallback();
+   this.closeCallback && this.closeCallback();

This fixed the crash during fuzzing. But the list of emphasized harmonics (managed via enterCallback and closeCallback) is not getting updated because state is just setting isShowingProperty.

So I'll leave this issue open, to be investigated when work resumes on a PhET-iO version of this sim.