From 7530937ebcb8f352a075c78dcbd3497ff08010f7 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 22 May 2023 17:44:20 +0000 Subject: [PATCH 1/2] fix: collapsing blocks with icons --- core/rendered_connection.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index cf192a0ef00..4c957eef9dd 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -20,6 +20,7 @@ import {Connection} from './connection.js'; import type {ConnectionDB} from './connection_db.js'; import {ConnectionType} from './connection_type.js'; import * as eventUtils from './events/utils.js'; +import {hasBubble} from './interfaces/i_has_bubble.js'; import * as internalConstants from './internal_constants.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; @@ -428,9 +429,13 @@ export class RenderedConnection extends Connection { connections[j].setTracking(false); } // Close all bubbles of all children. - const icons = block.getIcons(); - for (let j = 0; j < icons.length; j++) { - icons[j].setVisible(false); + for (const icon of block.getIcons()) { + if (hasBubble(icon)) { + icon.setBubbleVisible(false); + } else if (icon.setVisible) { + // TODO (#7042): Remove old icon handling code. + icon.setVisible(false); + } } } } From 08982ff76520872a4048cfb77f85df3bdddd1f17 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 22 May 2023 20:48:55 +0000 Subject: [PATCH 2/2] chore: add tests for hiding icons of collapsed children --- core/interfaces/i_has_bubble.ts | 4 +- core/renderers/measurables/icon.ts | 2 +- tests/mocha/block_test.js | 117 ++++++++++--------------- tests/mocha/icon_test.js | 56 +----------- tests/mocha/test_helpers/icon_mocks.js | 81 +++++++++++++++++ 5 files changed, 130 insertions(+), 130 deletions(-) create mode 100644 tests/mocha/test_helpers/icon_mocks.js diff --git a/core/interfaces/i_has_bubble.ts b/core/interfaces/i_has_bubble.ts index f45c69d071b..b1f872182ec 100644 --- a/core/interfaces/i_has_bubble.ts +++ b/core/interfaces/i_has_bubble.ts @@ -6,7 +6,7 @@ export interface IHasBubble { /** @return True if the bubble is currently open, false otherwise. */ - isBubbleVisible(): boolean; + bubbleIsVisible(): boolean; /** Sets whether the bubble is open or not. */ setBubbleVisible(visible: boolean): void; @@ -15,6 +15,6 @@ export interface IHasBubble { /** Type guard that checks whether the given object is a IHasBubble. */ export function hasBubble(obj: any): obj is IHasBubble { return ( - obj.isBubbleVisible !== undefined && obj.setBubbleVisible !== undefined + obj.bubbleIsVisible !== undefined && obj.setBubbleVisible !== undefined ); } diff --git a/core/renderers/measurables/icon.ts b/core/renderers/measurables/icon.ts index f5055c4e48e..f1592f8fee0 100644 --- a/core/renderers/measurables/icon.ts +++ b/core/renderers/measurables/icon.ts @@ -41,7 +41,7 @@ export class Icon extends Measurable { // TODO(#7042): Remove references to old icon API. this.isVisible = (icon.isVisible && icon.isVisible()) || - (hasBubble(icon) && icon.isBubbleVisible()); + (hasBubble(icon) && icon.bubbleIsVisible()); this.type |= Types.ICON; const size = diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 815df161bed..9d09c715146 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -19,6 +19,7 @@ import { createChangeListenerSpy, createMockEvent, } from './test_helpers/events.js'; +import {MockIcon, MockBubbleIcon} from './test_helpers/icon_mocks.js'; suite('Blocks', function () { setup(function () { @@ -1418,38 +1419,6 @@ suite('Blocks', function () { }); suite('Icon management', function () { - 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'; @@ -1624,54 +1593,58 @@ suite('Blocks', function () { workspaceTeardown.call(this, this.workspace); }); - test('Has Icon', function () { - const block = Blockly.Xml.domToBlock( - Blockly.utils.xml.textToDom(''), + test("Collapsing the block closes its contained children's bubbles", function () { + const parentBlock = Blockly.serialization.blocks.append( + { + 'type': 'statement_block', + 'inputs': { + 'STATEMENT': { + 'block': { + 'type': 'statement_block', + }, + }, + }, + }, this.workspace ); - block.setCommentText('test text'); - block.comment.setVisible(true); - chai.assert.isTrue(block.comment.isVisible()); - block.setCollapsed(true); - chai.assert.isFalse(block.comment.isVisible()); - }); + const childBlock = parentBlock.getInputTargetBlock('STATEMENT'); + const icon = new MockBubbleIcon(); + childBlock.addIcon(icon); + icon.setBubbleVisible(true); - test('Child Has Icon', function () { - const block = Blockly.Xml.domToBlock( - Blockly.utils.xml.textToDom( - '' + - ' ' + - ' ' + - ' ' + - '' - ), - this.workspace + parentBlock.setCollapsed(true); + + chai.assert.isFalse( + icon.bubbleIsVisible(), + "Expected collapsing the parent block to hide the child block's " + + "icon's bubble" ); - const childBlock = block.getInputTargetBlock('STATEMENT'); - childBlock.setCommentText('test text'); - childBlock.comment.setVisible(true); - chai.assert.isTrue(childBlock.comment.isVisible()); - block.setCollapsed(true); - chai.assert.isFalse(childBlock.comment.isVisible()); }); - test('Next Block Has Icon', function () { - const block = Blockly.Xml.domToBlock( - Blockly.utils.xml.textToDom( - '' + - ' ' + - ' ' + - ' ' + - '' - ), + test("Collapsing a block does not close its following childrens' bubbles", function () { + const parentBlock = Blockly.serialization.blocks.append( + { + 'type': 'statement_block', + 'next': { + 'block': { + 'type': 'statement_block', + }, + }, + }, this.workspace ); - const nextBlock = block.getNextBlock(); - nextBlock.setCommentText('test text'); - nextBlock.comment.setVisible(true); - chai.assert.isTrue(nextBlock.comment.isVisible()); - block.setCollapsed(true); - chai.assert.isTrue(nextBlock.comment.isVisible()); + const nextBlock = parentBlock.getNextBlock(); + const icon = new MockBubbleIcon(); + nextBlock.addIcon(icon); + icon.setBubbleVisible(true); + + parentBlock.setCollapsed(true); + + chai.assert.isTrue( + icon.bubbleIsVisible(), + 'Expected collapsing the parent block to not hide the next ' + + "block's bubble" + ); }); }); }); diff --git a/tests/mocha/icon_test.js b/tests/mocha/icon_test.js index f873de0ae78..ab4d0aae82a 100644 --- a/tests/mocha/icon_test.js +++ b/tests/mocha/icon_test.js @@ -11,6 +11,7 @@ import { sharedTestTeardown, } from './test_helpers/setup_teardown.js'; import {defineEmptyBlock} from './test_helpers/block_definitions.js'; +import {MockIcon, MockSerializableIcon} from './test_helpers/icon_mocks.js'; suite('Icon', function () { setup(function () { @@ -22,61 +23,6 @@ suite('Icon', function () { sharedTestTeardown.call(this); }); - 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 MockSerializableIcon extends MockIcon { - constructor() { - super(); - this.state = ''; - } - - getType() { - return 'serializable icon'; - } - - getWeight() { - return 1; - } - - saveState() { - return 'some state'; - } - - loadState(state) { - this.state = state; - } - } - class MockNonSerializableIcon extends MockIcon { getType() { return 'non-serializable icon'; diff --git a/tests/mocha/test_helpers/icon_mocks.js b/tests/mocha/test_helpers/icon_mocks.js new file mode 100644 index 00000000000..aab0d6e48e6 --- /dev/null +++ b/tests/mocha/test_helpers/icon_mocks.js @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +export 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() {} +} + +export class MockSerializableIcon extends MockIcon { + constructor() { + super(); + this.state = ''; + } + + getType() { + return 'serializable icon'; + } + + getWeight() { + return 1; + } + + saveState() { + return 'some state'; + } + + loadState(state) { + this.state = state; + } +} + +export class MockBubbleIcon extends MockIcon { + constructor() { + super(); + this.visible = false; + } + + getType() { + return 'bubble icon'; + } + + updateCollapsed() {} + + bubbleIsVisible() { + return this.visible; + } + + setBubbleVisible(visible) { + this.visible = visible; + } +}