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

Adding Blockly blocks to Content levels. #32484

Merged
merged 4 commits into from Jan 6, 2020
Merged

Conversation

daynew
Copy link
Member

@daynew daynew commented Dec 19, 2019

Description

In Content levels, such as External levels, level creators were taking
screenshots of Blockly blocks and adding them to the instructions in
order to give a preview to the student of what the blocks look like.
This has become a problem because screenshots are not translatable and
we are expanding our content to be used internationally.

  • Added common and turtle CDO blocks to Content levels.
  • blocks in Markdown content render as Blockly blocks.

Screenshots

Example of translations working

image

Working examples of all the existing screenshots used in External levels.

image

Links

Testing story

  • Added UI/eyes test

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@daynew daynew requested a review from Hamms December 19, 2019 19:01
// Install the common Blockly blocks
commonBlocks.install(window.Blockly, {});
// Install the custom CDO blocks for the following level/app types.
var options = [{skin: {id: 'birds'}, app: 'maze'}, {skin: {}, app: 'turtle'}];
Copy link
Contributor

Choose a reason for hiding this comment

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

note this is inconsistent with the above comment, which claims that we are making all the apps available.

I don't think I'm in favor of this approach; in particular, the existing app-specific locale files are not intended to be used together and there is no guarantee that they won't have conflicts. I'd recommend that we install only the common blockly blocks for now, and as follow-up work look into either adding an app association to content levels or unifying the app locales in general.

Copy link
Contributor

Choose a reason for hiding this comment

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

s/locale/block

Copy link
Member Author

Choose a reason for hiding this comment

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

From our conversation offline, I have created TODO's and a follow up JIRA task which we will get prioritized for Jan 2020.
https://codedotorg.atlassian.net/browse/FND-972

Copy link
Member Author

Choose a reason for hiding this comment

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

I talked with Bryan and found out he is mostly interested in being able to render Turtle Blockly blocks. So I have removed the second level type "maze" from the blocks imported. This way we don't have to do cleanup in January and we will still do the dynamic level type loading later in H1.

TESTING.md Show resolved Hide resolved
In Content levels, such as External levels, level creators were taking
screenshots of Blockly blocks and adding them to the instructions in
order to give a preview to the student of what the blocks look like.
This has become a problem because screenshots are not translatable and
we are expanding our content to be used internationally.
* Added common, maze, and turtle CDO blocks to Content levels.
* <xml> blocks in Markdown content render as Blockly blocks.
Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for following up on the 'multiple block types' thing, I'm much happier with where we landed.

@daynew daynew merged commit 9ec2814 into staging Jan 6, 2020
@daynew daynew deleted the enable_blockly_and_markdown branch January 6, 2020 20:22
@bcjordan
Copy link
Contributor

bcjordan commented Jan 7, 2020

This is so cool!! Remember always wanted to support this 😄

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

3 participants