Deprecate old listeners?

[zepumph]

While working on phetsims/projectile-motion#177 I found myself doing a code review on some of the older listeners. For example DownUpListener. I saw only a few usages, and none of them were instrumented for PhET-iO. I'm pretty sure that's because we prefer PressListener, but it would be nice if that could be marked in old scener listeners.

I know @jonathanolson had some work still to do in the new listeners, but aren't they functional enough to at least point people to them first? Here is the list that I thought of as the "old" listeners:

• DownUpListener
• SimpleDragHandler
• ButtonListener
• SCENERY_PHET/FireOnHoldInputListener?

Perhaps people already feel this way, in which case we should just add @deprecated doc so it isn't tribal knowledge.

[jonathanolson]

Sounds fine to me to mark them deprecated.

[zepumph]

@jonathanolson is this an accurate and complete list?

[pixelzoom]

Sounds fine to me to mark them as deprecated. But also finish up the work on their replacements. Specifically:

• Close issues related to creation of the replacement listeners. For example create DragListener as a replacement for SimpleDragHandler #131 (create DragListener as a replacement for SimpleDragHandler) is still open.

• Code review the new listeners. A quick look at classes MultiListener and PanZoomListener is all it takes to know that they will fail code review. If they are experimental and not part of the new listeners package, document them as such.

[jonathanolson]

@jonathanolson is this an accurate and complete list?

It is accurate, but I'm not sure what would be needed for a complete list.

I definitely want to finish up a few things for the new listeners.

[zepumph]

Today @samreid and I found that there were only two base classes of old listeners, SimpleDragHandler and DownUpListener, and they were both already marked as deprecated.

[jonathanolson]

Code review the new listeners. A quick look at classes MultiListener and PanZoomListener is all it takes to know that they will fail code review.

Created #1027 to track these specifically. They ARE being used in the pan/zoom work.

[jonathanolson]

SCENERY_PHET/FireOnHoldInputListener?

@zepumph or @pixelzoom Is there a replacement for FireOnHoldInputListener?

[jonathanolson]

I've marked the listeners I'm aware of that I feel should be deprecated. I have issues to track the main work, but I feel confident now that the "new" listeners should be used over the deprecated alternatives.

I think FireOnHoldInputListener is the main remaining question, otherwise this issue can be closed after that is figured out.

[pixelzoom]

Is there a replacement for FireOnHoldInputListener?

Not that I'm aware of. You might look to see how PushButtonModel is handling the "fire on hold" feature.

[jonathanolson]

The PushButtonModel setup looks like it could be integrated into FireListener. @pixelzoom, any objections to that, or also potentially moving CallbackTimer into axon itself (since it has no sun dependencies)?

[pixelzoom]

Previous comment sounds like a good plan, no objections.

[jonathanolson]

Added fire-on-hold to FireListener (and tested various configurations in the scenery playground). Marked FireOnHoldInputListener as deprecated.

@pixelzoom are you available to review, or close if desired?

[pixelzoom]

I verified that fire-on-hold was added to FireListener. But the other half of this (as I understood from #1004 (comment)) was that the implementation was moving from PushButtonModel to FireListener. PushButtonModel is not using FireListener, it still has its own copy of this functionality, which may diverge. So recommended to make PushButtonModel use FireListener, or create a sun issue to do so.

[pixelzoom]

In #1078, I identified uses of deprecated input listeners in common code, to be replaced.

So assuming that all deprecated input listeners are actually annotated with @deprecated... @jonathanolson can this issue be closed?

[jonathanolson]

Correct! Closing.