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

When the sim is inactive, it should be grayed out #232

Closed
samreid opened this issue Mar 31, 2015 · 15 comments
Closed

When the sim is inactive, it should be grayed out #232

samreid opened this issue Mar 31, 2015 · 15 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented Mar 31, 2015

Related to phetsims/scenery#414

@samreid
Copy link
Member Author

samreid commented Mar 31, 2015

Here is a small change set that looks like it solves the problem. Can one of @jbphet @jonathanolson @pixelzoom review it?

    // When the sim is inactive, make it non-interactive, see https://github.com/phetsims/scenery/issues/414
    var inactiveOverlay = new Node();
    this.activeProperty.lazyLink( function( active ) {
      sim.display.interactive = active;
      if ( !active ) {
        sim.showPopup( inactiveOverlay, true );
      }
      else {
        sim.hidePopup( inactiveOverlay, true );
      }
    } );

@samreid
Copy link
Member Author

samreid commented Mar 31, 2015

The above code should surround the existing sim.display.interactive = active; line in Sim.js

@pixelzoom
Copy link
Contributor

@samreid asked someone to review. I'll have a look.

@pixelzoom pixelzoom self-assigned this May 22, 2015
@pixelzoom
Copy link
Contributor

This is my first encounter with the sim.display.interactive property. I have no idea when (or why) we would be disabling mouse/touch/keyboard inputs, and can't find any documentation in joist.

Also, the code that @samreid says should be surrounded is:

// When the sim is inactive, make it non-interactive, see https://github.com/phetsims/scenery/issues/414
this.activeProperty.link( function( active ) {
  sim.display.interactive = active;
} );

Note that the comment references phetsims/scenery#141, which is closed.

@samreid and @jonathanolson, questions:

  1. Is this still an issue?
  2. When is a simulation grayed out?

@pixelzoom pixelzoom assigned jonathanolson and unassigned pixelzoom May 26, 2015
@pixelzoom
Copy link
Contributor

Assigning to @jonathanolson for feedback.

@jonathanolson
Copy link
Contributor

My understanding of the "interactive" property is that it just shuts off actual user input, but still allows playing back previously recorded inputs.

For playback, we want to fully disable the input from the user playing back previously recorded input events, since otherwise they could "insert" clicks/etc. and screw up the playback.

Showing it as a popup is probably not a great way to accomplish this. Using CSS opacity on the entire sim might be the best option.

@pixelzoom
Copy link
Contributor

@jonathanolson Another question... Is playback something that is currently used by any sims? Ie, does this need to be addressed for "Summer 2015 redeploy"?

@pixelzoom pixelzoom assigned jonathanolson and unassigned pixelzoom May 26, 2015
@samreid
Copy link
Member Author

samreid commented May 26, 2015

sim.active is used for together.js

Beaker is using it at the moment as far as I know.

@jonathanolson
Copy link
Contributor

I don't know of any usage on the sims on our website (excludes together and beaker).

@pixelzoom
Copy link
Contributor

Assigning to @jbphet, since he's working on beaker. Does this need to be addressed for "Summer 2015 redeploy"? If not, remove the label.

@pixelzoom pixelzoom assigned jbphet and unassigned jonathanolson May 26, 2015
@samreid
Copy link
Member Author

samreid commented May 26, 2015

Actually for beaker it is important for it to be inactive without being grayed out. It is unclear if/when the sim should be grayed out for other inactive cases.

@samreid
Copy link
Member Author

samreid commented May 26, 2015

I wonder if we could say this is the responsibility of the wrapper html, say, putting an opacity on the iframe..?

@jbphet
Copy link
Contributor

jbphet commented May 27, 2015

I don't agree with the intent of this issue. We currently have two customers who are using together.js to build learning systems around PhET simulations. Both are using the inactive capability for playback or at least setting of state, and I don't think either would want the sim to gray out in this situation. Assigning back to @samreid to either justify or close. His suggestion in the comment immediately above about having it be the responsibility of the wrapper page might suffice.

@jbphet jbphet assigned samreid and unassigned jbphet May 27, 2015
@pixelzoom
Copy link
Contributor

+1 for making this the responsibility of the wrapper.

@samreid
Copy link
Member Author

samreid commented May 27, 2015

Let's leave this up to the wrapper unless/until we run into a limitation that cannot be overcome by the wrapper. Closing.

@samreid samreid closed this as completed May 27, 2015
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