-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat!: allow blocks to receive their own delete events #6337
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
Merged
BeksOmega
merged 18 commits into
RaspberryPiFoundation:develop
from
BeksOmega:feat/delete-events
Aug 16, 2022
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
9e8739e
feat!: allow blocks to receive their own delete events
BeksOmega 7f475c0
fix: move block tests back into main directory
BeksOmega 7c35e2c
chore: add a test for disposing of callers
BeksOmega a4a7a50
chore: add test for delete being received
BeksOmega 76e850e
chore: add comment about why we have to run the clock twice
BeksOmega 0f4abe9
chore: fix whitespace
BeksOmega 43b900e
chore: fix whitespace
BeksOmega 4a488f7
chore: fix imports in tests
BeksOmega 69abf3f
chore: bump mocha timeout
BeksOmega 80d164d
chore: bump timeout again?
BeksOmega 579c3bc
chore: eliminate the possibility that tests are actually timing out
BeksOmega 52520a0
chore: change timeout back
BeksOmega 71e160a
chore: remove tests that might be the problematic ones
BeksOmega b3fd4b9
chore: attempt enabling delete event test
BeksOmega 6f63a4b
chore: enable lists tests
BeksOmega 4b3472a
chore: try ternary test as well
BeksOmega d48cec8
chore: actually add block test files
BeksOmega 1268f40
chore: enable remaining tests
BeksOmega File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2021 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| goog.declareModuleId('Blockly.test.blockDeleteEvent'); | ||
|
|
||
| import * as eventUtils from '../../build/src/core/events/utils.js'; | ||
| import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown.js'; | ||
|
|
||
| suite('Block Delete Event', function() { | ||
| setup(function() { | ||
| sharedTestSetup.call(this); | ||
| this.workspace = new Blockly.Workspace(); | ||
| }); | ||
|
|
||
| teardown(function() { | ||
| sharedTestTeardown.call(this); | ||
| }); | ||
|
|
||
| suite('Receiving', function() { | ||
| test('blocks receive their own delete events', function() { | ||
| Blockly.Blocks['test'] = { | ||
| onchange: function(e) {}, | ||
| }; | ||
| // Need to stub the definition, because the property on the definition is | ||
| // what gets registered as an event listener. | ||
| const spy = sinon.spy(Blockly.Blocks['test'], 'onchange'); | ||
| const testBlock = this.workspace.newBlock('test'); | ||
|
|
||
| testBlock.dispose(); | ||
|
|
||
| const deleteClass = eventUtils.get(eventUtils.BLOCK_DELETE); | ||
| chai.assert.isTrue(spy.calledOnce); | ||
| chai.assert.isTrue(spy.getCall(0).args[0] instanceof deleteClass); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking this is a breaking change and should probably be mentioned in the PR description, even if it's very unlikely to ever cause anyone an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a breaking change. Before blocks did not receive their own events, so this case would have never been hit. Now I've added the
blockIdcheck so it continues to not be hit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onchangeis not private, so it could be called by code outside Blockly, which could pass in an arbitrary event.I agree this is unlikely in practice, which is why I said "strictly speaking". It's probably not useful to bump a major version number or to mention in the release notes, but it probably is worth making sure that I am not the only one who has considered this scenario, and hence the comment.