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

Warn if jsonInit() receives a colour attribute without a value. #1795

Merged
merged 3 commits into from
Apr 18, 2018

Conversation

AnmAtAnm
Copy link
Contributor

@AnmAtAnm AnmAtAnm commented Apr 17, 2018

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Part of #1790.

Proposed Changes

In jsonInit(), if the JSON object has a colour attribute, but not a value, emit a warning.

For simplicity, this will occur on every block initialization of the given block type.

Reason for Changes

Warn developers that their blocks may be referencing a removed colour constant, or similar error.

Test Coverage

Ran the graph demo that was using the old constants. Saw the new warning.
After rebasing, tested against a logic_boolean with an explicit colour: undefined.

Tested on:

  • Desktop Chrome

core/block.js Outdated
} catch (colorError) {
console.warn(
'Block "' + blockTypeName + '": Illegal color value: ', rawValue);
if ('colour' in json) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's enough happening here to warrant pulling it out into a separate (private) function. Also worth doing because scratch-blocks and blockly handle colours slightly differently, so it's a point of merge conflicts.
After that, lgtm.

Adding block type name to prior warnings.
@AnmAtAnm AnmAtAnm merged commit 2bfff4a into google:develop Apr 18, 2018
rachel-fenichel pushed a commit to rachel-fenichel/blockly that referenced this pull request Apr 18, 2018
…le#1795)

 * Warn if jsonInit() receives a colour attribute without a value.
 * Extract colour init code into function.
 * Adding block type name to prior warnings.
@AnmAtAnm AnmAtAnm deleted the colour-undefined branch May 15, 2018 17:51
This was referenced Jun 26, 2018
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.

2 participants