fix: work on calling icon hooks (#7100)

* fix: work on calling icon hooks

* chore: PR comments

* chore: format
This commit is contained in:
Beka Westberg
2023-05-19 15:36:44 -07:00
committed by GitHub
parent 83c6c73817
commit 4dc2869778
8 changed files with 160 additions and 38 deletions

View File

@@ -39,7 +39,7 @@ import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js'
import type {IBoundedElement} from './interfaces/i_bounded_element.js';
import type {CopyData, ICopyable} from './interfaces/i_copyable.js';
import type {IDraggable} from './interfaces/i_draggable.js';
import type {IIcon} from './interfaces/i_icon.js';
import {IIcon, isIcon} from './interfaces/i_icon.js';
import * as internalConstants from './internal_constants.js';
import {ASTNode} from './keyboard_nav/ast_node.js';
import {TabNavigateCursor} from './keyboard_nav/tab_navigate_cursor.js';
@@ -197,9 +197,14 @@ export class BlockSvg
for (let i = 0, input; (input = this.inputList[i]); i++) {
input.init();
}
const icons = this.getIcons();
for (let i = 0; i < icons.length; i++) {
icons[i].createIcon();
for (const icon of this.getIcons()) {
if (isIcon(icon)) {
// icon.initView();
icon.updateEditable();
} else {
// TODO (#7042): Remove old icon handling.
icon.createIcon();
}
}
this.applyColour();
this.pathObject.updateMovable(this.isMovable());
@@ -527,17 +532,21 @@ export class BlockSvg
}
}
for (const icon of this.getIcons()) {
if (isIcon(icon)) {
icon.updateCollapsed();
} else if (collapsed) {
// TODO(#7042): Remove old icon handling code.
icon.setVisible(false);
}
}
if (!collapsed) {
this.updateDisabled();
this.removeInput(collapsedInputName);
return;
}
const icons = this.getIcons();
for (let i = 0, icon; (icon = icons[i]); i++) {
icon.setVisible(false);
}
const text = this.toString(internalConstants.COLLAPSE_CHARS);
const field = this.getField(collapsedFieldName);
if (field) {
@@ -1042,6 +1051,9 @@ export class BlockSvg
override addIcon<T extends IIcon>(icon: T): T {
super.addIcon(icon);
if (this.rendered) {
// icon.initView();
icon.applyColour();
icon.updateEditable();
// TODO: Change this based on #7024.
this.render();
this.bumpNeighbours();
@@ -1062,7 +1074,7 @@ export class BlockSvg
// TODO: remove this implementation after #7038, #7039, and #7040 are
// resolved.
override getIcons(): AnyDuringMigration[] {
const icons = [];
const icons: AnyDuringMigration = [...this.icons];
if (this.commentIcon_) icons.push(this.commentIcon_);
if (this.warning) icons.push(this.warning);
if (this.mutator) icons.push(this.mutator);

View File

@@ -38,6 +38,7 @@ export abstract class Icon implements IIcon {
const svgBlock = this.sourceBlock as BlockSvg;
this.svgRoot = dom.createSvgElement(Svg.G, {'class': 'blocklyIconGroup'});
svgBlock.getSvgRoot().appendChild(this.svgRoot);
this.updateSvgRootOffset();
browserEvents.conditionalBind(
this.svgRoot,
'pointerdown',
@@ -74,12 +75,25 @@ export abstract class Icon implements IIcon {
}
}
hideForInsertionMarker(): void {
if (!this.svgRoot) return;
this.svgRoot.style.display = 'none';
}
isShownWhenCollapsed(): boolean {
return false;
}
setOffsetInBlock(offset: Coordinate): void {
this.offsetInBlock = offset;
this.updateSvgRootOffset();
}
private updateSvgRootOffset(): void {
this.svgRoot?.setAttribute(
'transform',
`translate(${this.offsetInBlock.x}, ${this.offsetInBlock.y})`
);
}
onLocationChange(blockOrigin: Coordinate): void {

View File

@@ -44,13 +44,19 @@ export interface IIcon {
/** @return The dimensions of the icon for use in rendering. */
getSize(): Size;
/** Notifies the icon that the block's colour has changed. */
/** Updates the icon's color when the block's color changes.. */
applyColour(): void;
/** Notifies the icon that the block's editability has changed. */
/** Hides the icon when it is part of an insertion marker. */
hideForInsertionMarker(): void;
/** Updates the icon's editability when the block's editability changes. */
updateEditable(): void;
/** Notifies the icon that the block's collapsed-ness has changed. */
/**
* Updates the icon's collapsed-ness/view when the block's collapsed-ness
* changes.
*/
updateCollapsed(): void;
/**
@@ -88,6 +94,7 @@ export function isIcon(obj: any): obj is IIcon {
obj.getWeight !== undefined &&
obj.getSize !== undefined &&
obj.applyColour !== undefined &&
obj.hideForInsertionMarker !== undefined &&
obj.updateEditable !== undefined &&
obj.updateCollapsed !== undefined &&
obj.isShownWhenCollapsed !== undefined &&

View File

@@ -5,6 +5,7 @@
*/
import {BlockSvg} from './block_svg.js';
import {isIcon} from './interfaces/i_icon.js';
import {Coordinate} from './utils/coordinate.js';
/** The set of all blocks in need of rendering which don't have parents. */
@@ -84,8 +85,9 @@ function doRenders() {
if (block.getParent()) continue;
renderBlock(block);
updateConnectionLocations(block, block.getRelativeToSurfaceXY());
updateIconLocations(block);
const blockOrigin = block.getRelativeToSurfaceXY();
updateConnectionLocations(block, blockOrigin);
updateIconLocations(block, blockOrigin);
}
for (const workspace of workspaces) {
workspace.resizeContents();
@@ -138,12 +140,20 @@ function updateConnectionLocations(block: BlockSvg, blockOrigin: Coordinate) {
*
* @param block The block to update the icon locations of.
*/
function updateIconLocations(block: BlockSvg) {
function updateIconLocations(block: BlockSvg, blockOrigin: Coordinate) {
if (!block.getIcons) return;
for (const icon of block.getIcons()) {
icon.computeIconLocation();
if (isIcon(icon)) {
icon.onLocationChange(blockOrigin);
} else {
// TODO (#7042): Remove old icon handling code.
icon.computeIconLocation();
}
}
for (const child of block.getChildren(false)) {
updateIconLocations(child);
updateIconLocations(
child,
Coordinate.sum(blockOrigin, child.relativeCoords)
);
}
}

View File

@@ -22,6 +22,7 @@ import {Types} from '../measurables/types.js';
import {isDynamicShape} from './constants.js';
import type {ConstantProvider, Notch, PuzzleTab} from './constants.js';
import type {RenderInfo} from './info.js';
import {isIcon} from '../../interfaces/i_icon.js';
/**
* An object that draws a block based on the given rendering information.
@@ -305,12 +306,18 @@ export class Drawer {
}
}
if (Types.isIcon(fieldInfo)) {
svgGroup.setAttribute('display', 'block');
svgGroup.setAttribute(
'transform',
'translate(' + xPos + ',' + yPos + ')'
);
(fieldInfo as Icon).icon.computeIconLocation();
const icon = (fieldInfo as Icon).icon;
if (isIcon(icon)) {
icon.setOffsetInBlock(new Coordinate(xPos, yPos));
} else {
// TODO (#7042): Remove old icon handling code.
svgGroup.setAttribute('display', 'block');
svgGroup.setAttribute(
'transform',
'translate(' + xPos + ',' + yPos + ')'
);
(fieldInfo as Icon).icon.computeIconLocation();
}
} else {
svgGroup.setAttribute(
'transform',
@@ -321,7 +328,15 @@ export class Drawer {
if (this.info_.isInsertionMarker) {
// Fields and icons are invisible on insertion marker. They still have to
// be rendered so that the block can be sized correctly.
svgGroup.setAttribute('display', 'none');
// TODO (#7042): Figure out a better way to handle the types here,
// possibly by splitting this method into submethods.
if (isIcon((fieldInfo as Icon).icon)) {
(
(fieldInfo as Icon).icon as AnyDuringMigration
).hideForInsertionMarker();
} else {
svgGroup.setAttribute('display', 'none');
}
}
}

View File

@@ -13,12 +13,18 @@ import type {ConstantProvider} from '../common/constants.js';
import {Measurable} from './base.js';
import {Types} from './types.js';
import {hasBubble} from '../../interfaces/i_has_bubble.js';
import {isIcon} from '../../interfaces/i_icon.js';
/**
* An object containing information about the space an icon takes up during
* rendering.
*/
export class Icon extends Measurable {
/**
* @deprecated Will be removed in v11. Create a subclass of the Icon
* measurable if this data is necessary for you.
*/
isVisible: boolean;
flipRtl = false;
@@ -32,10 +38,15 @@ export class Icon extends Measurable {
constructor(constants: ConstantProvider, public icon: BlocklyIcon) {
super(constants);
this.isVisible = icon.isVisible();
// TODO(#7042): Remove references to old icon API.
this.isVisible =
(icon.isVisible && icon.isVisible()) ||
(hasBubble(icon) && icon.isBubbleVisible());
this.type |= Types.ICON;
const size = icon.getCorrectedSize();
const size =
(icon.getCorrectedSize && icon.getCorrectedSize()) ||
(isIcon(icon) && icon.getSize());
this.height = size.height;
this.width = size.width;
}

View File

@@ -1158,7 +1158,7 @@ suite('Blocks', function () {
});
});
suite('Add Connections Programmatically', function () {
test('Output', function () {
test('Output', async function () {
const block = createRenderedBlock(this.workspace, 'empty_block');
block.setOutput(true);
@@ -1418,7 +1418,39 @@ suite('Blocks', function () {
});
suite('Icon management', function () {
class MockIconA {
class MockIcon {
getType() {
return 'mock icon';
}
initView() {}
dispose() {}
getWeight() {}
getSize() {
return new Blockly.utils.Size(0, 0);
}
applyColour() {}
hideForInsertionMarker() {}
updateEditable() {}
updateCollapsed() {}
isShownWhenCollapsed() {}
setOffsetInBlock() {}
onLocationChange() {}
onClick() {}
}
class MockIconA extends MockIcon {
getType() {
return 'A';
}
@@ -1428,7 +1460,7 @@ suite('Blocks', function () {
}
}
class MockIconB {
class MockIconB extends MockIcon {
getType() {
return 'B';
}

View File

@@ -29,11 +29,29 @@ suite('Icon', function () {
initView() {}
dispose() {}
getWeight() {}
getSize() {
return new Blockly.utils.Size(0, 0);
}
applyColour() {}
hideForInsertionMarker() {}
updateEditable() {}
updateCollapsed() {}
isShownWhenCollapsed() {}
setOffsetInBlock() {}
onLocationChange() {}
onClick() {}
}
class MockSerializableIcon extends MockIcon {
@@ -90,7 +108,7 @@ suite('Icon', function () {
return createUninitializedBlock(workspace);
}
suite.skip('Hooks getting properly triggered by the block', function () {
suite('Hooks getting properly triggered by the block', function () {
suite('Triggering view initialization', function () {
test('initView is not called by headless blocks', function () {
const workspace = createHeadlessWorkspace();
@@ -106,7 +124,7 @@ suite('Icon', function () {
);
});
test('initView is called by headful blocks during initSvg', function () {
test.skip('initView is called by headful blocks during initSvg', function () {
const workspace = createWorkspaceSvg();
const block = createUninitializedBlock(workspace);
const icon = new MockIcon();
@@ -124,7 +142,7 @@ suite('Icon', function () {
);
});
test(
test.skip(
'initView is called by headful blocks that are currently ' +
'rendered when the icon is added',
function () {
@@ -231,7 +249,7 @@ suite('Icon', function () {
block.addIcon(icon);
applyColourSpy.resetHistory();
block.setDisabled(true);
block.setEnabled(false);
chai.assert.isTrue(
applyColourSpy.calledOnce,
'Expected applyColour to be called'
@@ -357,13 +375,15 @@ suite('Icon', function () {
const workspace = createWorkspaceSvg();
const block = createInitializedBlock(workspace);
const icon = new MockIcon();
const updateCollapsedSpy = sinon.spy(icon, 'updateCollapsed');
const updateCollapsedSpy = sinon.stub(icon, 'updateCollapsed');
block.addIcon(icon);
updateCollapsedSpy.resetHistory();
block.setCollapsed(true);
this.clock.runAll();
chai.assert.isTrue(
updateCollapsedSpy.calledOnce,
updateCollapsedSpy.called,
'Expected updateCollapsed to be called'
);
});
@@ -377,10 +397,11 @@ suite('Icon', function () {
block.setCollapsed(true);
block.setCollapsed(false);
this.clock.runAll();
chai.assert.isTrue(
updateCollapsedSpy.calledTwice,
'Expected updateCollapsed to be called twice'
updateCollapsedSpy.called,
'Expected updateCollapsed to be called'
);
});
});