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

Issue#135 interactive programs #152

Merged
merged 4 commits into from
Apr 4, 2019
Merged

Conversation

luchotc
Copy link
Contributor

@luchotc luchotc commented Mar 28, 2019

Resolves #135

@luchotc luchotc force-pushed the issue#135-interactive_programs branch from 69abf5d to b9c980c Compare March 28, 2019 18:05
@ghost ghost added in progress and removed in progress labels Mar 28, 2019
@luchotc luchotc requested a review from afska March 28, 2019 19:13
})
.catch(() => {
mumuki.kids.showResult({status: 'aborted'})
});
this.serverPromise = promise;

editor.toggleInteractiveMode();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this only if editor.interactiveMode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could add an if here. It doesn't affect the current layout, as it is just toggling .play-mode that doesn't change anything without .mu-kids-interactive

Copy link
Member

@flbulgarelli flbulgarelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked only public API selectors are used?

@afska
Copy link
Collaborator

afska commented Mar 28, 2019

I don't like the fact that editor.html is getting huge as we add new features. I'd like to split it, but right now it's kind of complicated since we need to:

So maybe another solution could be compiling several files in one (like vulcanize does). I'll create another issue for that.

@luchotc luchotc force-pushed the issue#135-interactive_programs branch from 02fb627 to 300227c Compare March 28, 2019 20:05
@ghost ghost removed the in progress label Mar 28, 2019
@luchotc luchotc force-pushed the issue#135-interactive_programs branch from 300227c to 7b62b60 Compare March 28, 2019 20:38
@ghost ghost added the in progress label Mar 28, 2019
@flbulgarelli
Copy link
Member

Looks good, just please import hammer directly from the other assets, so that no extra import on laboratory is required.

@ghost ghost removed the in progress label Apr 4, 2019
@luchotc luchotc merged commit 51b66f4 into master Apr 4, 2019
@ghost ghost removed the pending-review label Apr 4, 2019
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

Successfully merging this pull request may close these issues.

3 participants