Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Detect if deleting shadow block affects selection highlight #8533

Merged
merged 4 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
14 changes: 14 additions & 0 deletions core/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,20 @@ export class Block implements IASTNodeLocation {
}
}

/**
* Returns true if this block is the same block as the given block or contains
* the provided block as a descendent.
johnnesky marked this conversation as resolved.
Show resolved Hide resolved
*
* @param other the other block to compare to.
*/
contains(other: Block | null): boolean {
while (other !== null) {
if (other === this) return true;
other = other.parentBlock_;
}
return false;
}

/**
* Dispose of this block.
*
Expand Down
12 changes: 11 additions & 1 deletion core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export class BlockSvg
this.addSelect();
}

/** Unselects this block. Unhighlights the blockv visually. */
/** Unselects this block. Unhighlights the block visually. */
unselect() {
if (this.isShadow()) {
this.getParent()?.unselect();
Expand Down Expand Up @@ -798,6 +798,16 @@ export class BlockSvg
blockAnimations.disposeUiEffect(this);
}

// Selecting a shadow block highlights an ancestor block, but that highlight
// should be removed if the shadow block will be deleted. So, before
// deleting blocks and severing the connections between them, check whether
// doing so would delete a selected block and make sure that any associated
// parent is updated.
const selection = common.getSelected();
if (selection instanceof Block && this.contains(selection)) {
selection.unselect();
}

super.dispose(!!healStack);
dom.removeNode(this.svgGroup_);
}
Expand Down
72 changes: 72 additions & 0 deletions tests/mocha/block_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import * as common from '../../build/src/core/common.js';
import {ConnectionType} from '../../build/src/core/connection_type.js';
import * as eventUtils from '../../build/src/core/events/utils.js';
import {EndRowInput} from '../../build/src/core/inputs/end_row_input.js';
Expand Down Expand Up @@ -208,6 +209,44 @@ suite('Blocks', function () {
});
});

suite('Contains', function () {
setup(function () {
this.blocks = createTestBlocks(this.workspace, true);
});

test('contains parent is false', function () {
const blocks = this.blocks;
assert.isFalse(
blocks.B.contains(blocks.A),
'Expected child to not contain parent.',
);
});

test('contains self is true', function () {
const blocks = this.blocks;
assert.isTrue(
blocks.A.contains(blocks.A),
'Expected block to contain self.',
);
});

test('contains child is true', function () {
const blocks = this.blocks;
assert.isTrue(
blocks.A.contains(blocks.B),
'Expected block to contain child.',
);
});

test('contains grand child is true', function () {
const blocks = this.blocks;
assert.isTrue(
blocks.A.contains(blocks.C),
'Expected block to contain grandchild.',
);
});
});

suite('Disposal', function () {
suite('calling destroy', function () {
setup(function () {
Expand Down Expand Up @@ -443,6 +482,39 @@ suite('Blocks', function () {
});
});
});

suite('Disposing selected shadow block', function () {
setup(function () {
this.workspace = Blockly.inject('blocklyDiv');
this.parentBlock = this.workspace.newBlock('row_block');
this.parentBlock.initSvg();
this.parentBlock.render();
this.parentBlock.inputList[0].connection.setShadowState({
'type': 'row_block',
'id': 'shadow_child',
});
this.shadowChild =
this.parentBlock.inputList[0].connection.targetConnection.getSourceBlock();
});

teardown(function () {
workspaceTeardown.call(this, this.workspace);
});

test('Disposing selected shadow unhighlights parent', function () {
const parentBlock = this.parentBlock;
common.setSelected(this.shadowChild);
assert.isTrue(
parentBlock.pathObject.svgRoot.classList.contains('blocklySelected'),
'Expected parent to be highlighted after selecting shadow child',
);
this.shadowChild.dispose();
assert.isFalse(
parentBlock.pathObject.svgRoot.classList.contains('blocklySelected'),
'Expected parent to be unhighlighted after deleting shadow child',
);
});
});
});

suite('Remove Input', function () {
Expand Down
Loading