replace keypad workarounds

[pixelzoom]

Motivated by #118.

KeypadLayer and KeypadPanel are a workaround copied from unit-rates. They are a workaround for the lack of a robust Dialog (phetsims/joist#359) and the lack of closure on the next-gen keypad (phetsims/scenery-phet#283, phetsims/scenery-phet#271).

Rather than factor out "workaround" code, KeypadLayer and KeypadPanel should be replaced when the above-mentioned issues are addressed.

Given the rate of progress on those issues, they are unlikely to be addressed any time soon. So this issue is not a requirement for 1.0.0 publication. And this issue is being marked as deferred.

[pixelzoom]

7/27/17 dev meeting: @jonathanolson is using the next-gen keypad in area-model, so @andrea-phet should be able to use it in projectile-motion too. So replace KeypadLayer and KeypadPanel with the next-gen stuff prior to the 1.0.0 release.

@andrea-phet please work with @jbphet and @jonathanolson on this.

[pixelzoom]

@andrea-phet @jbphet @jonathanolson: Whatever you do in projectile-motion should be usable in unit-rates, see phetsims/unit-rates#153.

[jbphet]

@andrea-phet - I've updated and cleaned up the common keypad code, so please go ahead and use it in Projectile Motion. The code is in scenery-phet/js/keypad and there are several demos in scenery-phet/js/demo. Also, it is currently being used in Area Model Multiplication simulation, so you can find additional examples there if needed.

[andrealin]

Assigning to @jbphet to review.

[pixelzoom]

I took a quick peek at c4957e3. This address one part of the duplicated code, by replacing KeypadPanel with scenery-phet.Keypad. We'll still need to decide what to do about KeypadLayer, which was also copied from unit-rates.

[pixelzoom]

KeypadLayer still refers to KeypadPanel, which no longer exists:

44 // KeypadPanel options

[pixelzoom]

This issue should not be closed until Keypad.dispose is implemented, see phetsims/scenery-phet#283 (comment).

[pixelzoom]

I fixed the comments mentioned in #127 (comment).

[pixelzoom]

The integration of scenery.Keypad looks good, so that bit is done. But as I mentioned in #127 (comment), we need to decide what to do about the duplication in KeypadLayer (which was also copied from unit-rates).

KeypadLayer (in unit-rates) was a workaround for deficiencies and bugs in joist.Dialog. I'm not confident that Dialog is up to the task yet, since it's still being redesigned. And we would still need the parts of KeypadLayer that handle positioning the Dialog. It also looks like KeypadLayer has been modified somewhat from unit-rates, and it's difficult for me to evaluate how much code is still shared until phetsims/unit-rates#153 is completed.

So... My recommendation is:

(1) Fix the remaining issues in Keypad, as noted in phetsims/scenery-phet#283. This is a prerequisite to publishing 1.0.0.

(2) Leave this issue open, and use Dialog when it has been revamped. This is not required for publishing 1.0.0.

[jbphet]

The failing test case for the Keypad dispose function is now resolved, see phetsims/scenery-phet#283 and phetsims/scenery-phet#330. It should be safe to use a version of the keypad that is added and removed if the design moves that direction. Unassigning myself, @andrea-phet - feel free to assign back if there is any further work needed here.

[andrealin]

Okay. Again, leaving this issue open but unassigned in case Dialog should be used later.

[pixelzoom]

Changing this from on-hold to deferred, since no further work is required for 1.0.0 release.

[jbphet]

Assigning to @Denz1994 for followup once phetsims/scenery-phet#283 has been addressed.

[zepumph]

Pinging @Denz1994 for current progress on KeyPad. Is this something that is close enough to completion that we should add it before instrumenting the workarounds for PhET-iO. That would definitely be preferred.

[samreid]

@zepumph can you please re-evaluate and determine assignee?

[zepumph]

I feel like we could either wait on phetsims/scenery-phet#283, or decide that keypad is good enough to add into this release.

[zepumph]

@jbphet is Keypad far enough along to add it into PM while I'm working on this for PhET-iO?

[zepumph]

This may be getting some traction because phetsims/scenery-phet#283 is a quarterly goal right now. Assigning myself.

[zepumph]

Having looked at this issue in its entirety, the majority of work seems to be in the KeypadLayer that is duplicated here and in unit rates, and less about the use of Keypad (which seems fully opperational).

[jbphet]

@zepumph is going to take this on (thanks @zepumph!), I'm unassigning myself for now.

[matthew-blackman]

Any updates on this? Seems like it is related to #307.

[zepumph]

I think it will. We still haven't had a push to centralize a keypad container (especially a modal one) for usages like this sim and unit rates. When we do the hacky, sim-specific KeypadLayer should be replaced with common code.