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.

Sign up for GitHub

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: remove specific warning text #7376

Merged
merged 3 commits into from
Aug 11, 2023
Merged

fix: remove specific warning text #7376

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c27ce5
fix: remove specific warning text
maribethb Aug 10, 2023
f8ec81a
chore: teardown rendered workspace after test
maribethb Aug 10, 2023
cc3b0c8
chore: reformat new tests
maribethb Aug 10, 2023
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
11 changes: 7 additions & 4 deletions core/block_svg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,11 +896,10 @@ export class BlockSvg
* Set this block's warning text.
*
* @param text The text, or null to delete.
* @param opt_id An optional ID for the warning text to be able to maintain
* @param id An optional ID for the warning text to be able to maintain
* multiple warnings.
*/
override setWarningText(text: string | null, opt_id?: string) {
const id = opt_id || '';
override setWarningText(text: string | null, id: string = '') {
if (!id) {
// Kill all previous pending processes, this edit supersedes them all.
for (const timeout of this.warningTextDb.values()) {
Expand Down Expand Up @@ -931,8 +930,9 @@ export class BlockSvg
}

const icon = this.getIcon(WarningIcon.TYPE) as WarningIcon | undefined;
if (typeof text === 'string') {
if (text) {
// Bubble up to add a warning on top-most collapsed block.
// TODO(#6020): This warning is never removed.
let parent = this.getSurroundParent();
let collapsedParent = null;
while (parent) {
Expand All @@ -958,6 +958,9 @@ export class BlockSvg
if (!id) {
this.removeIcon(WarningIcon.TYPE);
} else {
// Remove just this warning id's message.
icon.addMessage('', id);
// Then remove the entire icon if there is no longer any text.
if (!icon.getText()) this.removeIcon(WarningIcon.TYPE);
}
}
Expand Down
81 changes: 81 additions & 0 deletions tests/mocha/block_test.js
View file
Open in desktop
maribethb marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,87 @@ suite('Blocks', function () {
});
});

suite('Warning icons', function () {
setup(function () {
this.workspace = Blockly.inject('blocklyDiv');

this.block = this.workspace.newBlock('stack_block');
this.block.initSvg();
this.block.render();
});

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

test('Block with no warning text does not have warning icon', function () {
const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
chai.assert.isUndefined(
icon,
'Block with no warning should not have warning icon',
);
});

test('Set warning text creates new icon if none existed', function () {
const text = 'Warning Text';
this.block.setWarningText(text);
const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
chai.assert.equal(
icon.getText(),
text,
'Expected warning icon text to be set',
);
});

test('Set warning text adds text to existing icon if needed', function () {
const text1 = 'Warning Text 1';
const text2 = 'Warning Text 2';
this.block.setWarningText(text1, '1');
this.block.setWarningText(text2, '2');
const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
chai.assert.equal(icon.getText(), `${text1}\n${text2}`);
});

test('Clearing all warning text deletes the warning icon', function () {
const text = 'Warning Text';
this.block.setWarningText(text);
this.block.setWarningText(null);
const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
chai.assert.isUndefined(
icon,
'Expected warning icon to be undefined after deleting all warning text',
);
});

test('Clearing specific warning does not delete the icon if other warnings present', function () {
const text1 = 'Warning Text 1';
const text2 = 'Warning Text 2';
this.block.setWarningText(text1, '1');
this.block.setWarningText(text2, '2');
this.block.setWarningText(null, '1');
const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
chai.assert.equal(
icon.getText(),
text2,
'Expected first warning text to be deleted',
);
});

test('Clearing specific warning removes icon if it was only warning present', function () {
const text1 = 'Warning Text 1';
const text2 = 'Warning Text 2';
this.block.setWarningText(text1, '1');
this.block.setWarningText(text2, '2');
this.block.setWarningText(null, '1');
this.block.setWarningText(null, '2');
const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
chai.assert.isUndefined(
icon,
'Expected warning icon to be deleted after all warning text is cleared',
);
});
});

suite('Bubbles and collapsing', function () {
setup(function () {
this.workspace = Blockly.inject('blocklyDiv');
Expand Down