-
Notifications
You must be signed in to change notification settings - Fork 481
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
Music: support flyout toolbox #52941
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things but LGTM! Nice addition!
| http://studio.code.org/s/allthethings/lessons/46/levels/2 | music lab script level | | ||
| http://studio.code.org/s/allthethings/lessons/46/levels/4 | music lab script level | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change this to a standalone project URL (projects/music/new) so we don't need to worry about updating it if the allthethings progression changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We depend on allthethings
for much of our UI testing, so I think it's reasonable to keep these particular levels in sync with UI tests.
"toolbox": { | ||
"Play": ["play_sound_at_current_location_simple2"] | ||
}, | ||
"simpleToolbox": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: should we nest this inside the "toolbox" field? Something like
"toolbox": {
"blocks": { "Play": [...] },
"simple": true,
// alternatively
"type": "flyout" / "category"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. I was opting for legacy compatibility, but we're still at the stage where we should build the ideal format, and this is closer to that!
This adds support for an optional Music Lab level property,
"type": "flyout"
, which will show a toolbox with all blocks shown all the time, without categories.It seems this could be helpful in early levels in a progression, when only a few blocks will be available.
The toolbox section of the level data has been changed a little, to have separate
"blocks"
and optional"type"
entries.flyout toolbox
categories toolbox (default)