Pull Request: Split showContextMenu_#2581
Pull Request: Split showContextMenu_#2581RoboErikG merged 4 commits intoRaspberryPiFoundation:developfrom
Conversation
Split showContextMenu_ into generateContextMenu_ and showContextMenu_. This allows for custom Blockly forks to easily extend the context menu options available on all blocks by just doing something like
```
var oldGenerateContextMenu_ = Blockly.BlockSvg.prototype.generateContextMenu_;
Blockly.BlockSvg.prototype.generateContextMenu_ = function() {
var menuOptions = Blockly.BlockSvg.prototype.generateContextMenu_() || [];
// Push other options into menuOptions
return menuOptions;
}
```
Rather than having to modify the core code in such a way as to cause unnecessary maintenance overhead when upgrading Blockly versions.
RoboErikG
left a comment
There was a problem hiding this comment.
One nit then I'm happy to merge. =)
core/block_svg.js
Outdated
| * Show the context menu for this block. | ||
| * @param {!Event} e Mouse event. | ||
| * Generate the context menu for this block. | ||
| * @private |
There was a problem hiding this comment.
This should be protected instead of private since it's intended to be overwritten.
There was a problem hiding this comment.
Hrm. I'm actually not sure if protected method standard is to have an underscore or not... is this what you were expecting?
There was a problem hiding this comment.
We pretty consistently leave the underscore on there for protected methods in Blockly. I'm not sure what the wider style norms are, though.
There was a problem hiding this comment.
Sorry to butt in, but that's not quite true.
If a protected method has an underscore in Blockly, it means it used to be marked private but was obviously part of our API, so we changed the annotation without changing the name.
New protected methods have no underscore.
There was a problem hiding this comment.
Thanks for the correction, Rachel. =) Merging as is, then.
Updated to be protected rather than private.
The basics
The details
Split showContextMenu_ into generateContextMenu_ and showContextMenu_.
Resolves
No issue created, but I can create one if that would make for easier tracking.
Proposed Changes
Split showContextMenu_ into generateContextMenu_ and showContextMenu_.
Reason for Changes
This allows for custom Blockly forks to easily extend the context menu options available on all blocks by just doing something like
Rather than having to modify the core code in such a way as to cause unnecessary maintenance overhead when upgrading Blockly versions, which is our use case.
Test Coverage
Tested on:
Additional Information
This is a fairly minor change but one which will make it a lot easier when developers need to extend the context menu options for all blocks, rather than only for individual blocks.