-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix ?allowLinks behavior #830
Comments
Also, any usage of |
Original issue: #592 |
@zepumph - Can you review JO's fix to allowLinks? You actually recognized this need previously in the original issue - in this comment #592 (comment), but I think you were on vacation and then the thought got lost in the final review and sign off on that issue. Behavior currently doesn't hide the option (e.g. PhET Website ...) will be visible, but no action is taken when you click on it. Nothing happens. @arouinfar and I felt this was fine behavior for this purpose. If you are a PhET-iO client, you can do something more sophisticated and hide the option or something, but for the PhET brand sims, we just want to prevent students from clicking their way away from the simulation. @jonathanolson - I think you were also going to review and make sure this captured all of the cases that you needed to capture for redirects. |
@jonathanolson - also, this is a candidate for a maintenance release to PhET brand, once we are sure we have fixed all cases and are happy with the PhET brand behavior. |
Adding behavior for openPopup makes sense, but it doesn't capture the spirit of converting query parameters to behavior that is covered/duplicated by the PhET-iO API so that it can be mutated in PhET-iO and thus supported in the standard wrapper. To do that, this would need to run off of a Property, which is kinda a no-no in phet-core. UPDATE: Noting that this issue is in part coming out of https://github.com/phetsims/phet-io/issues/1871 where we also want to have the PhET-iO support to control this query parameter. |
@jonathanolson and I discussed this, and in order to have allowLinks running from a Property, we are going to move openPopup to sun, so that it can depend on axon Property. |
Steps for this issue:
Assigning to whoever gets to it first. |
@samreid, @arouinfar, @kathy-phet and I discussed this a bit more, and we have a new recommendation.
@kathy-phet asked us to also assign @jonathanolson to this issue. @jonathanolson, please make sure to move the git history of openPopup. Please don't use the shell script because openPopup has been renamed to ts. Instead use |
Maintenance complete, closing. |
There are a few places where links / opportunities for students to navigate away from the simulation are still available when
?allowLinks=false
Specifically, these links are still active:
We need a solution for this when allowLinks=false.
Options off the top of my head:
The text was updated successfully, but these errors were encountered: