[Google Blockly] Modal function editor - allow edit/create buttons when modal is open#55155
[Google Blockly] Modal function editor - allow edit/create buttons when modal is open#55155mikeharv wants to merge 1 commit into
Conversation
| * @param {WorkspaceSvg} workspace The workspace containing procedures. | ||
| * @returns an array of XML block elements | ||
| */ | ||
| export function flyoutCategory(workspace, functionEditorOpen = false) { |
There was a problem hiding this comment.
functionEditorOpen was not being used and is not needed.
| export function flyoutCategory(workspace) { | ||
| const blockList = []; | ||
|
|
||
| const behaviorDefinitionBlock = { |
There was a problem hiding this comment.
The modal editor knows how to make a new behavior definition block without this. This was previously used to add a def block to the flyout if the modal editor was disabled, but we do not do that any more.
|
|
||
| const getNewBehaviorButtonWithCallback = ( | ||
| workspace, | ||
| behaviorDefinitionBlock |
There was a problem hiding this comment.
behaviorDefinitionBlock was not being used and is not needed.
| const callbackKey = 'newBehaviorCallback'; | ||
| workspace.registerButtonCallback(callbackKey, () => { | ||
| const callback = () => { | ||
| workspace.hideChaff(); |
There was a problem hiding this comment.
We want to close the flyout when a user creates a new procedure.
| * @param {WorkspaceSvg} workspace The workspace containing procedures. | ||
| * @returns an array of block objects representing the flyout blocks | ||
| */ | ||
| export function flyoutCategory(workspace, functionEditorOpen = false) { |
There was a problem hiding this comment.
functionEditorOpen is no longer needed.
| const blockList = []; | ||
|
|
||
| // Note: Blockly.Msg was undefined when this code was extracted into global scope | ||
| const functionDefinitionBlock = { |
There was a problem hiding this comment.
We only need to define this block if we aren't using the modal editor (so that we can add it directly to the flyout blocks).
Context
Currently, if a user is in the modal function editor, we hide flyout buttons for creating new behaviors/functions and also hide edit buttons from their associated call blocks. We have had to do this as a workaround due to a bug in Blockly v9. Essentially, the bug is that a field click might delete a block, and Blockly doesn't check to make sure the block exists before attempting to bring it to the front.
Proposal
By adding a short timeout, we can give Blockly time to execute the
block.bringToFrontmethod before performing our "work", which is what causes the block to get deleted.The buttons to create new procedures are part of a flyout, so they don't hit this issue. For these, I instead just had to make sure that we were registering the toolbox callbacks on the modal editor's workspace. (If we only registered the callbacks on the main workspace, Blockly can't find the callback keys and the buttons don't do anything.) Based on comments I've just removed, it seems like we were worried at one point about being able to find all the procedures if we weren't looking at the main workspace, but this doesn't seem to be a problem any more. The
flyoutCategoryfunctions now look at both the main and hidden workspaces explicitly. (See: #53154)The following video demonstrates various methods of creating new procedures while the modal editor is open:
2023-11-27.12-36-11.2023-11-27.12_36_34.mp4
Links
https://codedotorg.atlassian.net/browse/CT-124
Testing story
Deployment strategy
Follow-up work
Connect with Mark and determine if we want "edit" buttons to be rendered on blocks that are still in the flyout. If so, should the buttons be clickable, like they are in CDO Blockly, or should they (current behavior) just be part of the draggable block surface and result in moving the block to the main workspace?
Privacy
Security
Caching
PR Checklist: