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

Update playground to load blocks in iframe instead of custom sandbox #7241

Merged
merged 9 commits into from Jun 25, 2020

Conversation

shakao
Copy link
Contributor

@shakao shakao commented Jun 23, 2020

  • Core logic is in playground.ts
  • Moved the common CSS from /tutorial-tool into a /shared folder. I thought about doing this for the shared JS too but it's kind of scattered and didn't seem entirely worth it
  • Did a pass over the samples for formatting. Might go back and do more proofreading later
  • Removed all unused files

I checked that all samples load correctly in Arcade (beta), except the text dropdown, which I think is probably due to blockly upgrade issues--I'll investigate separately.

For microsoft/pxt-arcade#1942

docs/static/playground/playground.ts Outdated Show resolved Hide resolved
Co-authored-by: Joey Wunderlich <jwunderl@users.noreply.github.com>
docs/playground.html Outdated Show resolved Hide resolved
sampleSwitcher.appendChild(sampleChapter);
]
}, {
name: "LEGO EV3",
Copy link
Member

Choose a reason for hiding this comment

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

You could load the editors.json for list of editors (since you are cleaning)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe next iteration

language: "typescript",
minimap: { enabled: false }
});
var targets = [
Copy link
Member

Choose a reason for hiding this comment

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

I've also refactored our list of editors in the 'editors.json" file, which you can fetch. WOuld be good to eventually unify all these tools to use the same list of editors.

]
}
];
var CUSTOM_FILE = "_custom.ts";
Copy link
Member

Choose a reason for hiding this comment

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

why not just "custom.ts" ? (not that it matters at all)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh i use the underscore for files that users shouldn't modify, i feel like it makes the file look less... friendly? just habit in this case haha

Copy link
Member

Choose a reason for hiding this comment

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

the editor creates a "custom.ts" file so let's drop the _

for (var _a = 0, _b = target.endpoints; _a < _b.length; _a++) {
var endpoint = _b[_a];
if (!selected || selected === target.name + "-" + endpoint.name) {
iframe.setAttribute("src", endpoint.url);
Copy link
Member

Choose a reason for hiding this comment

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

add ws=mem to avoid polluting the script list of the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated! i'll make a separate pr after this one to use editors.json, it's not a lot of code but i want to add some targets to the json so i might tweak streamer to add a list of disabled targets

Copy link
Member

Choose a reason for hiding this comment

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

you can add a flag in editors.json to specify that it is supported in particular tool...

"microbit": {
    "tutorial-tool": true,
    ...
}

to avoid interference

}
];

const CUSTOM_FILE = "_custom.ts";
Copy link
Member

Choose a reason for hiding this comment

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

custom.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated! i ended up removing ws=mem because it isn't compatible with controller=1, and controller=1 also does not pollute the user's real projects

@pelikhan pelikhan self-requested a review June 25, 2020 17:01
@shakao shakao merged commit a0814c5 into master Jun 25, 2020
@shakao shakao deleted the shakao/playground-import branch June 25, 2020 18:02
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.

None yet

4 participants