search

phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Enable supportsPanAndZoom #219

Closed ariel-phet closed 3 years ago

ariel-phet commented 4 years ago

@pixelzoom on latest dev (1.2.0-dev3) pinch to zoom did not seem to be enabled. I thought it was now on by default in master, is there currently some override or?

pixelzoom commented 4 years ago

There is no specific override in Natural Selection. @jessegreenberg?

amanda-phet commented 4 years ago

9/3/20 design meeting: not necessary for 10/1 client version, but we'd like this in the RC version.

pixelzoom commented 4 years ago

9/3/20 design meeting:

@ariel-phet said that we should opt-out for the 10/1 dev release. @jessegreenberg how do I opt out?

pixelzoom commented 4 years ago

The general issue for enabling zoom by default is https://github.com/phetsims/joist/issues/657.

pixelzoom commented 4 years ago

Zoom is now enabled by default in https://github.com/phetsims/joist/issues/657#issuecomment-686818596.

To disable for the 10/1 deliverable, add to package.json:

"phet": {
  "supportsZoom": false
jessegreenberg commented 4 years ago

Correct, that is the way to opt out (followed by grunt update to apply the change to the sim). I played with the sim with zoom for a while and it seems to be behaving well. Let me know if there's anything else I can do here!

pixelzoom commented 4 years ago

In https://github.com/phetsims/joist/issues/657#issuecomment-689048282, @jessegreenberg noted that Zoom is causing problems with WebGL sims. Natural Selection uses WebGL. So Zoom may need to be disabled for the 10/1 deliverable, which begins dev testing on 9/17. Unclear whether it would be blocking for production.

pixelzoom commented 4 years ago

Labeling blocks-sim-publication to remind me that Zoom needs to be either disabled or addressed for 10/1 deliverable.

pixelzoom commented 4 years ago

Over in https://github.com/phetsims/joist/issues/657#issuecomment-689800331, @ariel-phet said:

@pixelzoom we should go with zoom enabled at first, but if we run into any significant issues, zoom can be disabled. Zoom does not need to be enabled for the production version. Although desired, it is not a requirement. Moving forward we would certainly like to have zoom available in as many production sims as possible, but again, not a requirement for the coming production release.

@amanda-phet Note that this is a departure from the original plan, which was to publish the 10/1 dev version with Zoom disabled. If you have anything related in the Client Guide, you may need to revise.

pixelzoom commented 4 years ago

I discussed this more with @ariel-phet on Slack. Summary:

pixelzoom commented 4 years ago

Zoom was disabled in the above commit.

pixelzoom commented 4 years ago

Note that in phetsims/chipper#980, supportsZoom was changed to supportsPanAndZoom.

pixelzoom commented 3 years ago

@ariel-phet asked me to enable supportsPanAndZoom for the 1.3 release.

pixelzoom commented 3 years ago

supportsPanAndZoom has been enabled. Ready for testing in next RC.

KatieWoe commented 3 years ago

Pan and zoom is implemented, but some issues may be present such as https://github.com/phetsims/natural-selection/issues/264. Leaving open due to this. @pixelzoom feel free to close if you think these aren't connected.

pixelzoom commented 3 years ago

For phetsims/QA/issues/636.

I don't know if they are connected, and I'm not sure how to proceed. I'll start by assigning to @jessegreenberg (pan and zoom) and the PhET-iO team (@zepumph and @samreid). Please recommend whether this issue can be closed, or whether there is more to do here in order to be certain that pan and zoom is working correctly.

This is high priority, because we want to create a release branch immediately after dev test is completed.

jessegreenberg commented 3 years ago

It looks like #264 was resolved today, so it should be fixed when you are ready to take SHAs for the RC.

some issues may be present

@KatieWoe did you notice other problems besides #264 or was that the only one?

KatieWoe commented 3 years ago

264 also impacted the recording wrapper. I also didn't get much of a chance to explore other wrappers, such as state, but that was the only one I'd seen so far

pixelzoom commented 3 years ago

I guess it's still in my court to make a decision, even though I know almost nothing about pan and zoom, or what was tested. So based on comments by @jessegreenberg and @KatieWoe above, I'm going to infer that there's nothing else to do here, and this issue can be closed. If that's not the case, please reopen ASAP.

jessegreenberg commented 3 years ago

Sounds good. If that was the only issue there is nothing else we are aware of and this can be closed.