Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/flyout_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ class Flyout extends DeleteArea {
* otherwise.
* @package
*/
isBlockCreatable_(block) {
isBlockCreatable(block) {
return block.isEnabled();
}

Expand Down
10 changes: 7 additions & 3 deletions core/gesture.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const {IBlockDragger} = goog.requireType('Blockly.IBlockDragger');
const {IBubble} = goog.requireType('Blockly.IBubble');
/* eslint-disable-next-line no-unused-vars */
const {IFlyout} = goog.requireType('Blockly.IFlyout');
/* eslint-disable-next-line no-unused-vars */
const {WorkspaceCommentSvg} = goog.require('Blockly.WorkspaceCommentSvg');
const {WorkspaceDragger} = goog.require('Blockly.WorkspaceDragger');
/* eslint-disable-next-line no-unused-vars */
const {WorkspaceSvg} = goog.requireType('Blockly.WorkspaceSvg');
Expand Down Expand Up @@ -332,7 +334,7 @@ class Gesture {
if (!this.targetBlock_) {
return false;
}
if (!this.flyout_.isBlockCreatable_(this.targetBlock_)) {
if (!this.flyout_.isBlockCreatable(this.targetBlock_)) {
return false;
}
if (!this.flyout_.isScrollable() ||
Expand Down Expand Up @@ -743,8 +745,10 @@ class Gesture {
*/
doBubbleClick_() {
// TODO (#1673): Consistent handling of single clicks.
this.startBubble_.setFocus && this.startBubble_.setFocus();
this.startBubble_.select && this.startBubble_.select();
if (this.startBubble_ instanceof WorkspaceCommentSvg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are workspace comments bubbles? That seems like not what they are.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Notes for the curious:
I think it's just to make workspace comments similar to block comments. And block comments are icons/bubbles, just like mutators.
Bubbles sit on a different canvas than blocks, and always on top. They do that so that blocks on the workspace can never look like they're inside an open mutator.

this.startBubble_.setFocus();
this.startBubble_.select();
}
}

/**
Expand Down
15 changes: 15 additions & 0 deletions core/interfaces/i_flyout.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,19 @@ IFlyout.prototype.position;
*/
IFlyout.prototype.isDragTowardWorkspace;

/**
* Does this flyout allow you to create a new instance of the given block?
* Used for deciding if a block can be "dragged out of" the flyout.
* @param {!BlockSvg} block The block to copy from the flyout.
* @return {boolean} True if you can create a new instance of the block, false
* otherwise.
* @package
*/
IFlyout.prototype.isBlockCreatable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is removing the underscore on this name a breaking change?

I still don't understand our policy on external developers overriding @package properties in subclasses. But it seems like if this is part of the interface of a flyout, then renaming part of that interface is a breaking chnage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically a breaking change, so I renamed the PR to fix!.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And also added a comment on the breaking change to the PR description.


/**
* Scroll the flyout to the beginning of its contents.
*/
IFlyout.prototype.scrollToStart;

exports.IFlyout = IFlyout;
15 changes: 15 additions & 0 deletions core/interfaces/i_toolbox_item.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,19 @@ IToolboxItem.prototype.isCollapsible;
*/
IToolboxItem.prototype.dispose;

/**
* Gets the HTML element that is clickable.
* @return {?Element} The HTML element that receives clicks.
* @public
*/
IToolboxItem.prototype.getClickTarget;

/**
* Sets whether the category is visible or not.
* For a category to be visible its parent category must also be expanded.
* @param {boolean} isVisible True if category should be visible.
* @protected
*/
IToolboxItem.prototype.setVisible_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to exist here? Or on the collapsible toolbox item interface?

Copy link
Collaborator Author

@rachel-fenichel rachel-fenichel Apr 22, 2022

Choose a reason for hiding this comment

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

It has to move all the way up to IToolboxItem because of how it's being used. @alschmiedt said that "we allow any toolbox item to be in a collapsible category" and that it should be left as is for compatibility reasons. But I noted it in my issue for follow-ups, in case we can find a better way to deal with it before the next release.


exports.IToolboxItem = IToolboxItem;
12 changes: 9 additions & 3 deletions core/keyboard_nav/ast_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,20 @@ class ASTNode {
*/
navigateBetweenStacks_(forward) {
let curLocation = this.getLocation();
if (curLocation.getSourceBlock) {
// TODO(#6097): Use instanceof checks to exit early for values of
// curLocation that don't make sense.
if ((/** @type {!IASTNodeLocationWithBlock} */ (curLocation))
.getSourceBlock) {
curLocation = /** @type {!IASTNodeLocationWithBlock} */ (curLocation)
.getSourceBlock();
}
if (!curLocation || !curLocation.workspace) {
// TODO(#6097): Use instanceof checks to exit early for values of
// curLocation that don't make sense.
const curLocationAsBlock = /** @type {!Block} */ (curLocation);
if (!curLocationAsBlock || !curLocationAsBlock.workspace) {
return null;
}
const curRoot = curLocation.getRootBlock();
const curRoot = curLocationAsBlock.getRootBlock();
const topBlocks = curRoot.workspace.getTopBlocks(true);
for (let i = 0; i < topBlocks.length; i++) {
const topBlock = topBlocks[i];
Expand Down
12 changes: 12 additions & 0 deletions core/renderers/common/i_path_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ goog.module('Blockly.blockRendering.IPathObject');
/* eslint-disable-next-line no-unused-vars */
const {BlockSvg} = goog.requireType('Blockly.BlockSvg');
/* eslint-disable-next-line no-unused-vars */
const {Connection} = goog.requireType('Blockly.Connection');
/* eslint-disable-next-line no-unused-vars */
const {ConstantProvider} = goog.requireType('Blockly.blockRendering.ConstantProvider');
/* eslint-disable-next-line no-unused-vars */
const {Theme} = goog.requireType('Blockly.Theme');
Expand Down Expand Up @@ -159,4 +161,14 @@ IPathObject.prototype.updateMovable;
*/
IPathObject.prototype.updateReplacementFade;


/**
* Add or remove styling that shows that if the dragging block is dropped,
* this block will be connected to the input.
* @param {Connection} conn The connection on the input to highlight.
* @param {boolean} enable True if styling should be added.
* @package
*/
IPathObject.prototype.updateShapeForInputHighlight;

exports.IPathObject = IPathObject;
3 changes: 2 additions & 1 deletion core/renderers/zelos/drawer.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ class Drawer extends BaseDrawer {
.pathRightDown(input.height) +
svgPaths.lineOnAxis('h', -width) +
(/** @type {!DynamicShape} */ (input.shape)).pathUp(input.height) + 'z';
this.block_.pathObject.setOutlinePath(inputName, outlinePath);
const pathObject = /** @type {!PathObject} */ (this.block_.pathObject);
pathObject.setOutlinePath(inputName, outlinePath);
}

/**
Expand Down
8 changes: 7 additions & 1 deletion core/renderers/zelos/info.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const {InputConnection} = goog.require('Blockly.blockRendering.InputConnection')
const {InRowSpacer} = goog.require('Blockly.blockRendering.InRowSpacer');
/* eslint-disable-next-line no-unused-vars */
const {Measurable} = goog.requireType('Blockly.blockRendering.Measurable');
/* eslint-disable-next-line no-unused-vars */
const {PathObject} = goog.requireType('Blockly.zelos.PathObject');
const {RenderInfo: BaseRenderInfo} = goog.require('Blockly.blockRendering.RenderInfo');
/* eslint-disable-next-line no-unused-vars */
const {Renderer} = goog.requireType('Blockly.zelos.Renderer');
Expand Down Expand Up @@ -528,8 +530,12 @@ class RenderInfo extends BaseRenderInfo {
if (Types.isInlineInput(elem) && elem instanceof InputConnection) {
const connectedBlock = elem.connectedBlock;
const innerShape = connectedBlock ?
connectedBlock.pathObject.outputShapeType :
/** @type {!PathObject} */ (connectedBlock.pathObject)
.outputShapeType :
elem.shape.type;
if (innerShape == null) {
return 0;
}
// Special case for value to stack / value to statement blocks.
if (connectedBlock && connectedBlock.outputConnection &&
(connectedBlock.statementInputCount ||
Expand Down
2 changes: 1 addition & 1 deletion core/serialization/blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ const initBlock = function(block, rendered) {
blockSvg.render(false);
// fixes #6076 JSO deserialization doesn't
// set .iconXY_ property so here it will be set
const icons = block.getIcons();
const icons = blockSvg.getIcons();
for (let i = 0; i < icons.length; i++) {
icons[i].computeIconLocation();
}
Expand Down
7 changes: 3 additions & 4 deletions core/shortcut_items.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ goog.module('Blockly.ShortcutItems');

const clipboard = goog.require('Blockly.clipboard');
const common = goog.require('Blockly.common');
/* eslint-disable-next-line no-unused-vars */
const {BlockSvg} = goog.requireType('Blockly.BlockSvg');
const {BlockSvg} = goog.require('Blockly.BlockSvg');
const {Gesture} = goog.require('Blockly.Gesture');
/* eslint-disable-next-line no-unused-vars */
const {ICopyable} = goog.requireType('Blockly.ICopyable');
Expand Down Expand Up @@ -145,8 +144,8 @@ const registerCut = function() {
preconditionFn: function(workspace) {
const selected = common.getSelected();
return !workspace.options.readOnly && !Gesture.inProgress() && selected &&
selected.isDeletable() && selected.isMovable() &&
!selected.workspace.isFlyout;
selected instanceof BlockSvg && selected.isDeletable() &&
selected.isMovable() && !selected.workspace.isFlyout;
},
callback: function() {
const selected = common.getSelected();
Expand Down
17 changes: 13 additions & 4 deletions core/toolbox/toolbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ const {Rect} = goog.require('Blockly.utils.Rect');
/* eslint-disable-next-line no-unused-vars */
const {ShortcutRegistry} = goog.requireType('Blockly.ShortcutRegistry');
/* eslint-disable-next-line no-unused-vars */
const {ToolboxCategory} = goog.requireType('Blockly.ToolboxCategory');
/* eslint-disable-next-line no-unused-vars */
const {WorkspaceSvg} = goog.requireType('Blockly.WorkspaceSvg');
/** @suppress {extraRequire} */
goog.require('Blockly.Events.ToolboxItemSelect');
Expand Down Expand Up @@ -371,8 +373,13 @@ class Toolbox extends DeleteArea {
handled = false;
break;
}
if (!handled && this.selectedItem_ && this.selectedItem_.onKeyDown) {
handled = this.selectedItem_.onKeyDown(e);
if (!handled && this.selectedItem_) {
// TODO(#6097): Figure out who implements onKeyDown and which interface it
// should be part of.
const untypedItem = /** @type {?} */ (this.selectedItem_);
if (untypedItem.onKeyDown) {
handled = untypedItem.onKeyDown(e);
}
}

if (handled) {
Expand Down Expand Up @@ -816,8 +823,10 @@ class Toolbox extends DeleteArea {
refreshTheme() {
for (let i = 0; i < this.contents_.length; i++) {
const child = this.contents_[i];
if (child.refreshTheme) {
child.refreshTheme();
// TODO(#6097): Fix types or add refreshTheme to IToolboxItem.
const childAsCategory = /** @type {ToolboxCategory} */ (child);
if (childAsCategory.refreshTheme) {
childAsCategory.refreshTheme();
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions core/toolbox/toolbox_item.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ class ToolboxItem {
* @public
*/
dispose() {}

/**
* Sets whether the category is visible or not.
* For a category to be visible its parent category must also be expanded.
* @param {boolean} _isVisible True if category should be visible.
* @protected
*/
setVisible_(_isVisible) {
// nop by default
}
}

exports.ToolboxItem = ToolboxItem;
10 changes: 6 additions & 4 deletions core/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,10 @@ exports.createDom = createDom;
* @alias Blockly.Tooltip.bindMouseEvents
*/
const bindMouseEvents = function(element) {
element.mouseOverWrapper_ =
// TODO (#6097): Don't stash wrapper info on the DOM.
(/** @type {?} */ (element)).mouseOverWrapper_ =
browserEvents.bind(element, 'mouseover', null, onMouseOver);
element.mouseOutWrapper_ =
(/** @type {?} */ (element)).mouseOutWrapper_ =
browserEvents.bind(element, 'mouseout', null, onMouseOut);

// Don't use bindEvent_ for mousemove since that would create a
Expand All @@ -308,8 +309,9 @@ const unbindMouseEvents = function(element) {
if (!element) {
return;
}
browserEvents.unbind(element.mouseOverWrapper_);
browserEvents.unbind(element.mouseOutWrapper_);
// TODO (#6097): Don't stash wrapper info on the DOM.
browserEvents.unbind((/** @type {?} */ (element)).mouseOverWrapper_);
browserEvents.unbind((/** @type {?} */ (element)).mouseOutWrapper_);
element.removeEventListener('mousemove', onMouseMove);
};
exports.unbindMouseEvents = unbindMouseEvents;
Expand Down
4 changes: 2 additions & 2 deletions core/workspace_svg.js
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ class WorkspaceSvg extends Workspace {
* the paste was not successful.
*/
paste(state) {
if (!this.rendered || !state['type'] && !state.tagName) {
if (!this.rendered || !state['type'] && !state['tagName']) {
return null;
}
if (this.currentGesture_) {
Expand Down Expand Up @@ -1938,7 +1938,7 @@ class WorkspaceSvg extends Workspace {
// Start at 1 since the 0th block was used for initialization.
for (let i = 1; i < topElements.length; i++) {
const topElement = topElements[i];
if (topElement.isInsertionMarker && topElement.isInsertionMarker()) {
if (topElement instanceof BlockSvg && topElement.isInsertionMarker()) {
continue;
}
const blockBoundary = topElement.getBoundingRectangle();
Expand Down
4 changes: 2 additions & 2 deletions tests/deps.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.