phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

TContentAppearanceStrategy and TButtonAppearanceStrategy should be parameterized. #755

Open pixelzoom opened 2 years ago

pixelzoom commented 2 years ago

TContentAppearanceStrategy was created by @zepumph and is presumably a general types. But it's second param is specific to RadioButtonInteractionState. This seems wrong.

type TContentAppearanceStrategy = {
  dispose?: () => void;
  new( content: Node,
       interactionStateProperty: IProperty<RadioButtonInteractionState>,
       options: any ): any;
};
pixelzoom commented 2 years ago

TButtonAppearanceStrategy has the same problem:

type TButtonAppearanceStrategy = {
  dispose?: () => void;
  new( content: PaintableNode,
       interactionStateProperty: IProperty<ButtonInteractionState | RadioButtonInteractionState>,
       baseColorProperty: IProperty<Color>,
       options?: any ): any;
}

This is not type-safe. These type definitions should be parameterized with the IProperty type, e.g:

type TButtonAppearanceStrategy<T> = {
  dispose?: () => void;
  new( content: PaintableNode,
       interactionStateProperty: IProperty<T>,
       baseColorProperty: IProperty<Color>,
       options?: any ): any;
}