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

factor out duplicated code for keypad? #118

Closed
pixelzoom opened this issue Jul 23, 2017 · 2 comments
Closed

factor out duplicated code for keypad? #118

pixelzoom opened this issue Jul 23, 2017 · 2 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

Related to code review #103, these 2 items:

KeypadLayer and KeypadPanel were copied from unit-rates, then modified slightly. I recognize them because I did the implementation in unit-rates, and I'm shown as @author.

Should something be generalized and moved to common code?

Worth noting is that the unit-rates implementation is sub-optimal. I would have preferred to have used joist.Dialog instead of creating my own "keypad layer". But Dialog had (and still has) serious problems.

This issue should not prevent publication to this sim, can be addressed later.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 26, 2017

After thinking about this... I'm reluctant to factor out common code because this implementation would be unnecessary if https://github.com/phetsims/joist/issues/359, phetsims/scenery-phet#283 and phetsims/scenery-phet#271 were addressed. So I think that a better idea would be to create issue in projectile-motion and unit-rates indicating that KeypadLayer + KeypadPanel should be replaced when those issues are addressed.

@pixelzoom
Copy link
Contributor Author

I created #127 in projectile-motion. And added a comment to the existing issue phetsims/unit-rates#153 in unit-rates.

Closing this issue.

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

2 participants