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
Limited block improvements #19678
Limited block improvements #19678
Conversation
# Conflicts: # apps/package.json
0002b5b
to
38bd9d9
Compare
38bd9d9
to
ed96801
Compare
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.
couple of tiny nits, but otherwise looks great!
@@ -424,6 +424,7 @@ | |||
"englishOnlyWarning": "Sorry! This stage is not available in your language. The puzzles in this stage use a mix of English words and characters that can’t be translated right now. You can move on to Stage {nextStage}.", | |||
"enrollmentDescription": "Join your teacher's classroom by entering their section code below. Teachers will be able to see your course progress, projects, and reset your password in case you forget it.", | |||
"errorEmptyFunctionBlockModal": "There need to be blocks inside your function definition. Click \"edit\" and drag blocks inside the green block.", | |||
"errorExceededLimitedBlocks": "You did it! Now go find the pattern in your code and take out the extra blocks. You can only use {limit} of these blocks:", |
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.
👍 for "go find the pattern in your code"
apps/src/constants.js
Outdated
@@ -63,6 +63,7 @@ exports.TestResults = { | |||
LOCAL_FUNCTION_FAIL: -6, // The program contains an unexpected JavaScript local function | |||
GENERIC_LINT_FAIL: -7, // The program contains a lint error | |||
LOG_CONDITION_FAIL: -8, // The program execution log did not pass a required condition | |||
EXCEEDED_LIMITED_BLOCKS: -9, // Puzzle was solved using more than the toolbox limit of a block |
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.
super-minor nit, but something like BLOCK_LIMIT_FAIL
seems like it would better fit the pattern
apps/src/feedback.js
Outdated
case TestResults.EXCEEDED_LIMITED_BLOCKS: | ||
var exceededBlockType = this.hasExceededLimitedBlocks_(); | ||
var limit = Blockly.mainBlockSpace.blockSpaceEditor.blockLimits.getLimit(exceededBlockType); | ||
var block = `<xml><block type='${this.hasExceededLimitedBlocks_()}'></block></xml>`; |
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.
can be just type='${exceededBlockType}'
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.
Whoops 😳. Fixed in ae818e6!
Depends on code-dot-org/blockly#75.