Move TandemDragHandler to Tandem about tandem HOT 19 CLOSED

As noted in #8 this may require us to make all sims depend on Tandem, since some sun components use TandemDragHandler.

from tandem.

I'd rather not have to move this twice, so I'm marking this for discussion at next phet-io meeting.

from tandem.

If there's consensus to add TANDEM dependency to all sims... Then move to tandem/js/input/TandemDragHandler, to match scenery/js/input/SimpleDragHandler.

from tandem.

@jonathanolson says as a general principle it is OK for sun to rely on tandem.

from tandem.

Consensus: Add tandem dependency to all sims and move TandemDragHandler to tandem

Also call it TandemSimpleDragHandler for consistency.

from tandem.

@jonathanolson said an uncoventional option would be to "monkey-patch" change the original objects (but could not change the original constructor).

from tandem.

New Consensus: get rid of all the decorator subclasses, see if performance is still OK (esp. for TandemDragHandler), get rid of TTandem names, etc.

from tandem.

And move SimpleDragHandler to scenery-phet.

from tandem.

@jonathanolson also points out that we don't have to make this change immediately, since it won't directly break wrapper clients (if a name changes from TTandemDragHandler to TSimpleDragHandler).

from tandem.

Why do we want to get rid of decorator types? Seems like they nicely isolate the PhET-io related features, without adding complexity to the main types.

from tandem.

Why do we want to get rid of decorator types? Seems like they nicely isolate the PhET-iO related features, without adding complexity to the main types.

I agree this is great separation of concerns, the main issues are:

• in the extreme case we would have a new Tandem* subclass for each original common code class. This means TandemHSlider, etc.
• All of the base (non-tandem types) would have to be flexible enough to adding tandem behavior by subclassing.
• Knowing which class to instantiate. For instance, scenery-phet's NumberControl.js creates ArrowButton instances. Would TandemNumberControl create TandemArrowButton instances? If so, how?
• Where in an inheritance hierarchy to put the Tandem part, for instance BooleanRectangularStickyToggleButton? Does TandemBooleanRectangularStickyToggleButton extend TandemRectangularStickyToggleButton etc?

Personally I like the way that Tandem has been separated from the base classes in TandemEmitter, TandemDragHandler and TandemText. It would be great to address the above issues and continue with (expand?) this paradigm. @jonathanolson please chime in if I've forgotten any of the reasons we were thinking of getting rid of these decorator subclasses.

from tandem.

@jonathanolson please chime in if I've forgotten any of the reasons we were thinking of getting rid of these decorator subclasses.

I think that covers what you said earlier.

from tandem.

We need to discuss this again when @pixelzoom is back.

from tandem.

Also pointing out, I wasn't trying to recommend whether we should have the extra types. Just noting potential ways that we COULD get rid of them.

from tandem.

Meeting notes:

It seems like many places like HSlider it make more sense to inline the tandem code. TandemDragHandler made sense because we didn't want tandem in scenery and because it resulted in a lot of code that made sense to consolidate and doesn't cause tricky inheritance issues (yet).

from tandem.

I moved TandemDragHandler to tandem, and updated all config files to point to tandem. I tested that all sims are launching, closing.

from tandem.

Fixed lint errors in joist, scenery-phet and sun, due to a dangling comma where TANDEM is the last entry in config.js:

TANDEM: '../../tandem/js',

from tandem.

Thanks @pixelzoom, I'll run grunt-all to make sure no others were skipped.

from tandem.

Lint-all checks out, closing.

from tandem.