Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to handle converting to new listener patterns? #1022

Closed
zepumph opened this issue Nov 23, 2019 · 5 comments
Closed

How to handle converting to new listener patterns? #1022

zepumph opened this issue Nov 23, 2019 · 5 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Nov 23, 2019

In a few recent sim's I have been converting old, deprecated listeners to use the newer PressListener hierarchy.

For example phetsims/projectile-motion#209, phetsims/projectile-motion#183, and phetsims/gravity-force-lab#191 have all been issues to upgrade to new listeners.

My main reasons for this work have been for PhET-iO. It is, in general, much less costly to bring sims up to code standards before publication, rather than trying to support multiple listener apis in the public PhET-iO api. Especially when we know that a collection of listeners are deprecated.

As a result of this work, bugs have been showing themselves in the new listeners. I created #1014 and #1021 during these conversions.

From this, I wanted to discuss with the team what the priority should be. Do we think that I should just stop trying to convert? To me it feels good to be finding these bugs now, but I often don't know how they should be prioritized when I can't solve them.

The priority to convert all listeners seems to be mostly long-term. I don't see us right now going through and converting everything.

Marking for developer meeting to gauge what the team is feeling.

@zepumph
Copy link
Member Author

zepumph commented Dec 5, 2019

See #1004 for a list of the old, deprecated listeners.

@Denz1994
Copy link
Contributor

Denz1994 commented Dec 5, 2019

12/5/19 dev meeting notes:

This process would motivate updating dragListeners and ironing out bugs that are present. @jonathanolson will up the priority on updating the related listeners in #1004 and will recruit @zepumph if needed.

@pixelzoom
Copy link
Contributor

On the topic of wrapping up the new listeners.... #131, "create DragListener as a replacement for SimpleDragHandler" is still open, and was created on Sep 10, 2013 (6+ years ago).

@jonathanolson
Copy link
Contributor

I've been updating listeners and fixing things, but I'm confused if there's anything to do for this specific issue. Is there anything left to discuss here, or can it be closed?

@zepumph
Copy link
Member Author

zepumph commented Jan 2, 2020

I don't thin that there is anything else for this issue. It was mainly to get the ball rolling. Thanks for all the good work, and I'll see you in individual bug issues (if there are any left). Closing

@zepumph zepumph closed this as completed Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants