-
Notifications
You must be signed in to change notification settings - Fork 617
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: remove references to colour fields and blocks #2241
fix: remove references to colour fields and blocks #2241
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.
One fix, otherwise just Qs
Are we going to run into problems with the code demo in core not having these blocks anymore? I can't remember if we still update that demo or not.
Similarly for the demos site. I think that's all stored locally, so when we go to update those, anyone who dragged a color block into a demo is going to break. Maybe we should clear their local state if serialization errors instead of them having to clear their local state themselves? Otherwise I think we might get a bunch of bug reports about this.
@@ -771,84 +771,6 @@ export default { | |||
}, | |||
], | |||
}, | |||
{ | |||
kind: '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.
Oof this is making me realize I probably need to go an update some devsite screenshots... blehhh
@@ -649,71 +649,6 @@ let toolboxJson = { | |||
], | |||
}, | |||
|
|||
{ | |||
// Colour Category | |||
kind: '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.
We don't save any of the state of the devsite landing demo right?
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.
Correct
plugins/block-test/src/mutators.js
Outdated
@@ -50,23 +50,23 @@ const COLOR_CHANGE_MUTATOR = { | |||
* @this {Blockly.Block} | |||
*/ | |||
domToMutation: function (xmlElement) { | |||
this.setColour(xmlElement.getAttribute('colour')); | |||
this.setFieldValue('LABEL', xmlElement.getAttribute('arbitrary')); |
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.
I think these params are reversed, should be value then name. https://developers.google.com/blockly/reference/js/blockly.block_class.setfieldvalue_1_method.md
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 catch. I went back to the test page and for each mutator test block I dragged into the workspace, opened the mutator, modified the block, closed the mutator, and then copy/pasted the block to confirm it was correct.
I have an issue tracking verifying the storage demo won't cause problems: Updating the code demo is now tracked in google/blockly#7938 |
* chore: delete uses of colour blocks from most other plugins * chore: delete colour blocks from shadow block converter * chore: stop using colour fields in block-test plugin * fix: param order
* chore: delete uses of colour blocks from most other plugins * chore: delete colour blocks from shadow block converter * chore: stop using colour fields in block-test plugin * fix: param order
* chore: delete uses of colour blocks from most other plugins * chore: delete colour blocks from shadow block converter * chore: stop using colour fields in block-test plugin * fix: param order
* fix: remove references to colour fields and blocks (#2241) * chore: delete uses of colour blocks from most other plugins * chore: delete colour blocks from shadow block converter * chore: stop using colour fields in block-test plugin * fix: param order * chore: remove uses of the multiline block or field in other plugins (#2239)
* fix: remove references to colour fields and blocks (google#2241) * chore: delete uses of colour blocks from most other plugins * chore: delete colour blocks from shadow block converter * chore: stop using colour fields in block-test plugin * fix: param order * chore: remove uses of the multiline block or field in other plugins (google#2239)
The basics
The details
Resolves
Part of #2194
Proposed Changes
Cleans up references to colour blocks and fields in other plugins and examples.
Updates the mutator test blocks to use text instead of colour.
Reason for Changes
I removed these fields and blocks from core and don't want to have to cross-link plugins to get those fields and blocks back.
Test Coverage
Mutator test blocks updated, and I opened every category in the blocks-test toolbox to confirm they all still worked + interacted with mutator test blocks.