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

Add support for Legends of Learning API #423

Closed
samreid opened this issue May 28, 2017 · 8 comments
Closed

Add support for Legends of Learning API #423

samreid opened this issue May 28, 2017 · 8 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented May 28, 2017

We would like to add support for the Legends of Learning platform.

@samreid samreid self-assigned this May 28, 2017
@samreid
Copy link
Member Author

samreid commented May 28, 2017

@jessegreenberg and I experimented with using a standardized PhET postMessage API for doing this in #420 but since Legends of Learning requires a single-file solution, it seems we won't be able to use an iframe adapter to make that work. @jonathanolson helped us try out making a sim iframe data base64 but it was laggy and buggy.

So I think we should investigate another way of adding support for legends of learning messages.

@samreid
Copy link
Member Author

samreid commented May 28, 2017

I have a proposed implementation for this, but it relies on (a) putting hooks in Sim.js in 2 spots, and chipper using Query String Machine. It will probably not be able to cherry pick into all legacy branches for maintenance releases.

@samreid
Copy link
Member Author

samreid commented May 28, 2017

OK I made commits to master to support Legends of Learning messages, trying to remain as decoupled as possible but still satisfy the constraint of having a single build and one-file solution. It is not too different from our strategy for game up camera.

@jessegreenberg can you please review these joist and chipper changes?

@jessegreenberg
Copy link
Contributor

The nice thing about a standardized postMessage API was that Sim.js didn't need any hooks that were specific to a third party. @samreid what do you think about changing to something like phetsims/chipper@c4c0850 and d626170 , where ThirdPartySupport.js will delegate setting up specific listeners depending on the third party that is specified in a query parameter?

@samreid
Copy link
Member Author

samreid commented Jun 1, 2017

I see where you are going with this, but am wondering if there would be cases where 3rd party hooks shouldn't be mutually exclusive. We could have something like ThirdPartySupport.js and it would just iterates through the list:

if (legendsOfLearning) new LegendsOfLearning();
if (somethingElse) new SomethingElse();

Rather than mutually exclusive ones. This could still keep Sim.js pretty clean?

@samreid samreid assigned jessegreenberg and unassigned samreid Jun 1, 2017
@jessegreenberg
Copy link
Contributor

Yes, that makes sense. @samreid also pointed out that the lookup using ThirdPartySupport makes it much less clear when trying to understand where the third party hooks are. The recommendation in #423 (comment) is clean. I will revert to the strategy in #423 (comment).

@jessegreenberg
Copy link
Contributor

I reverted the changes for third party support so we are back to #423 (comment). @samreid is there anything else to do for this issue?

@samreid
Copy link
Member Author

samreid commented Aug 2, 2017

We published build-an-atom with LOL features we are going to test out, closing.

@samreid samreid closed this as completed Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants