search

phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Incorrect type for Sim modalNodeStack #825

Closed pixelzoom closed 2 years ago

pixelzoom commented 2 years ago

Encountered when trying to port BarrierRectangle.js to TS.

In Sim.js, @samreid ported modalNodeStack like this:

  private modalNodeStack = createObservableArray<Node>();

Node is an incorrect type here. Looking at BarrierRectangle, the only place that takes modalNodeStack as an input:

53  modalNodeStack.get( modalNodeStack.length - 1 ).hide();

So it doesn't need to be a Node at all, just something that implements hide: () => void. Creating a type just for that seems a little odd. It would be nice it that were Popupable, but it's not. Maybe something like this in BarrierRectangle?

export type BarrierRectangleHideable = {
  hide: () => void;
};

Suggestions on how to proceed?

pixelzoom commented 2 years ago

Hmm... Some evidence in Sim.ts that modalNodeStack should be PopupableNode, which does implement hide:

  public hidePopup( popup: PopupableNode, isModal: boolean ): void {
    assert && assert( popup && this.modalNodeStack.includes( popup ) );
pixelzoom commented 2 years ago

The above commit works, and allowed me to proceed with converting BarrierRectangle. Closing.