-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Added updateColour to Field #2413
Added updateColour to Field #2413
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.
Looks pretty good overall.
return goog.color.rgbArrayToHex(rgb); | ||
}; | ||
|
||
Blockly.Block.prototype.getColourBorder = function() { |
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.
Why does this return an object, and not just colourBorder?
Also, missing documentation, and the answer to that question probably belongs in the docstring.
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.
It returns an object because if the block isn't using a style the block sort of has two borders, a light one and a dark one. Another option would be to return either the tertiary colour, or an object containing the light and dark colors, but I thought having a consistent return type might be simpler.
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.
Okay. I think that makes sense, but let's mark this function package rather than public. Then I'll hit merge.
core/field_dropdown.js
Outdated
* Updates the dropdown arrow to match the colour/style of the block. | ||
* @package | ||
*/ | ||
Blockly.Field.prototype.updateColour = function() { |
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.
Make this Blockly.FieldDropdown.
core/block_svg.js
Outdated
for (var x = 0, input; input = this.inputList[x]; x++) { | ||
for (var y = 0, field; field = input.fieldRow[y]; y++) { | ||
field.forceRerender(); | ||
field.updateColour(); | ||
} | ||
} | ||
}; | ||
|
||
/** | ||
* Sets the colour of the border. | ||
* Removes the light and dark paths if a tertiary colour is defined. |
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.
Update comment: Removes the light and dark paths if a border colour is defined.
b98a966
to
065ccc5
Compare
a09819d
to
247aafa
Compare
The basics
The details
Resolves
Work on #1623 and sorta #1456
Proposed Changes
Adds a function so fields can be coloured based on the colour of their block.
Also creates dedicated functions for getting a blocks shadow and border colours. This way if more things in the future need access to this logic it's all in one place.
Reason for Changes
More efficient than rerendering every field when the colour updates & separates responsibilities.
Test Coverage
I added buttons to the default fields category so styles, enabling/disabling, and shadowing/unshadowing could be tested.
Classic:
Classic, style randomized:
Classic, shadow true:
Classic, shadow false (everything renables correctly):
Modern:
Modern, style randomized:
Modern, shadow true:
Modern, shadow false (everything renables correctly):
Tested on:
Additional Information
Currently fields do not react to their block being enabled/disabled, which includes not getting colour updates.
(The following are the same as on develop/master)
Classic Disabled:
Modern Disabled: