phetsims / sun

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

Review pattern for ContentAppearanceStrategy #752

Open pixelzoom opened 2 years ago

pixelzoom commented 2 years ago

In https://github.com/phetsims/sun/issues/751, @samreid said:

  • [ ] This problem predates TS but I thought I'd mention it while in the area. new options.contentAppearanceStrategy( looks odd. Can we use a factory pattern instead of constructor pattern? This pattern looks a little unusual to me (or maybe I'm just not used to it)
    // TButtonAppearanceStrategy.ts
    type TButtonAppearanceStrategy = {
    dispose?: () => void;
    new( content: PaintableNode,
    interactionStateProperty: IProperty<ButtonInteractionState | RadioButtonInteractionState>,
    baseColorProperty: IProperty<Color>,
    options?: any ): any;
    }

That issue is not related to TypeScript conversion, so has been moved here.

Assigned to @jbphet (author) and @samreid to discuss.

samreid commented 2 years ago

@jbphet can you please take the lead and reach out to me as appropriate?