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

fix: advanced playground and playground #6021

Merged
merged 4 commits into from
Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 6 additions & 21 deletions tests/playgrounds/advanced_playground.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,14 @@
<head>
<meta charset="utf-8">
<title>Advanced Blockly Playground</title>
<script src="../../blockly_uncompressed.js"></script>
<script src="../../msg/messages.js"></script>
<script src="../themes/test_themes.js"></script>
<script src="prepare.js"></script>
<script src="./screenshot.js"></script>
<script src="../themes/test_themes.js"></script>
<script src="../../node_modules/@blockly/dev-tools/dist/index.js"></script>
<script src="../../node_modules/@blockly/theme-modern/dist/index.js"></script>

<script>
// Custom requires for the playground.
// Rendering.
goog.require('Blockly.minimalist.Renderer');
goog.require('Blockly.Themes.Zelos');

// Other.
goog.require('Blockly.WorkspaceCommentSvg');
goog.require('Blockly.libraryBlocks');
goog.require('Blockly.Dart.all');
goog.require('Blockly.JavaScript.all');
goog.require('Blockly.Lua.all');
goog.require('Blockly.PHP.all');
goog.require('Blockly.Python.all');
</script>
<script>
<script type="module">
import Blockly from './blockly.mjs';
'use strict';

function start() {
Expand Down Expand Up @@ -144,7 +129,7 @@
// Adds a default-sized workspace comment to the workspace.
menuOptions.push(Blockly.ContextMenu.workspaceCommentOption(workspace, e));
}

start();
</script>

<style>
Expand All @@ -169,7 +154,7 @@
}
</style>
</head>
<body onload="start()">
<body>
<div id="root"></div>
</body>
</html>
30 changes: 22 additions & 8 deletions tests/playgrounds/prepare.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,30 @@
});
</script>`);
} else {
// The below code is necessary for a few reasons:
// - We need an absolute path instead of relative path because the
// advanced_playground the and regular playground are in different folders.
// - We need to get the root directory for blockly because it is
// different for github.io, appspot and local.
alschmiedt marked this conversation as resolved.
Show resolved Hide resolved
const files = [
'blockly_compressed.js',
'msg/messages.js',
'blocks_compressed.js',
'dart_compressed.js',
'javascript_compressed.js',
'lua_compressed.js',
'php_compressed.js',
'python_compressed.js',
];

// We need to load Blockly in compiled mode.
const hostName = window.location.host.replaceAll('.', '\\.');
const matches = new RegExp(hostName + '\\/(.*)tests').exec(window.location.href);
const root = matches && matches[1] ? matches[1] : '';

// Load blockly_compressed.js et al. using <script> tags.
document.write('<script src="../../blockly_compressed.js"></script>');
document.write('<script src="../../msg/messages.js"></script>');
document.write('<script src="../../blocks_compressed.js"></script>');
document.write('<script src="../../dart_compressed.js"></script>');
document.write('<script src="../../javascript_compressed.js"></script>');
document.write('<script src="../../lua_compressed.js"></script>');
document.write('<script src="../../php_compressed.js"></script>');
document.write('<script src="../../python_compressed.js"></script>');
for (let i = 0; i < files.length; i++) {
document.write('<script src="/' + root + files[i] + '"></script>');
Copy link
Collaborator

Choose a reason for hiding this comment

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

For future: this seems like a good example of a place to use template strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So when writing this PR I remembered that we probably want at least the basic playground to work on ie11 to make testing easier. I don't think it currently will, but I didn't want to add another piece that we would have to go back and update.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ohhh gotcha gotcha. Sweet! That makes sense

}
}
})();
20 changes: 9 additions & 11 deletions tests/themes/test_themes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,22 @@
*/
'use strict';

goog.provide('Blockly.TestThemes');


/**
* A theme with classic colours but enables start hats.
*/
Blockly.Themes.TestHats = Blockly.Theme.defineTheme('testhats', {
const TestHatsTheme = Blockly.Theme.defineTheme('testhats', {
'base': Blockly.Themes.Classic
});
Comment on lines +12 to 14
Copy link
Collaborator

Choose a reason for hiding this comment

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

This has side effects right? And that's how we can access the theme later? Just want to clarify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this has side effects. We register themes in their constructors. However, I believe in this case the populateTestThemes method is how we are adding the themes to the advanced playground.

Blockly.Themes.TestHats.setStartHats(true);
TestHatsTheme.setStartHats(true);

/**
* A theme with classic colours but a different font.
*/
Blockly.Themes.TestFont = Blockly.Theme.defineTheme('testfont', {
const TestFontTheme = Blockly.Theme.defineTheme('testfont', {
'base': Blockly.Themes.Classic
});
Blockly.Themes.TestFont.setFontStyle({
TestFontTheme.setFontStyle({
'family': '"Times New Roman", Times, serif',
'weight': null, // Use default font-weight
'size': 16
Expand All @@ -33,9 +31,9 @@ Blockly.Themes.TestFont.setFontStyle({
* @type {!Object<string, Blockly.Theme>}
* @private
*/
Blockly.Themes.testThemes_ = {
'Test Hats': Blockly.Themes.TestHats,
'Test Font': Blockly.Themes.TestFont
const testThemes_ = {
'Test Hats': TestHatsTheme,
'Test Font': TestFontTheme
};

/**
Expand All @@ -45,7 +43,7 @@ Blockly.Themes.testThemes_ = {
* @package
*/
function getTestTheme(value) {
return Blockly.Themes.testThemes_[value];
return testThemes_[value];
}

/**
Expand All @@ -54,7 +52,7 @@ function getTestTheme(value) {
*/
function populateTestThemes() {
var themeChanger = document.getElementById('themeChanger');
var keys = Object.keys(Blockly.Themes.testThemes_);
var keys = Object.keys(testThemes_);
for (var i = 0, key; (key = keys[i]); i++) {
var option = document.createElement('option');
option.setAttribute('value', key);
Expand Down