fix: proc call remain disabled as def changes name #2043
Conversation
| event.type === Blockly.Events.BLOCK_CHANGE && | ||
| event.blockId === this.id && | ||
| event.element === 'disabled' && | ||
| event.group |
There was a problem hiding this comment.
Groups are for handling undo and redo, so you shouldn't need to check for this explicitly! We don't care if it's being bundled with other events for undo/redo :P
You should also be able to remove it from your unit test.
There was a problem hiding this comment.
Gotcha! makes sense.
Unfortunately if I remove event.group the patch won't work. This is because if you disable a proc def it will trigger a proc call enable/disable call and I have no idea how to distinguish it with disabling it directly. Adding event.group in the UI seem to work but it doesn't feel like the right fix so I will close this PR for now. =)
| event.element === 'disabled' && | ||
| event.group | ||
| ) { | ||
| this.previousEnabledState_ = !event.newValue; |
There was a problem hiding this comment.
Nit: This is equivalent but I think it's a little clearer.
| this.previousEnabledState_ = !event.newValue; | |
| this.previousEnabledState_ = event.oldValue; |
There was a problem hiding this comment.
Wanted to come back here and say that as I was rewriting this, you're right that !event.newValue is the correct way to do this :P
|
Hmm did you try checking for |
|
@BeksOmega I double checked the events being outputted in these instances and the only difference I see is event.group is empty vs not. =(
Totally agree, this is a little bit out of my league however so I'll leave it to the blockly team |
The basics
The details
Resolves
Fixes #2035
Proposed Changes
When a proc call changes enabled state independently, previousEnabledState_ should be kept in sync
Reason for Changes
#2035
Test Coverage
The unit test requires a setGroup call in order to simulate what happens in UI (ie when a proc call gets disabled)
Documentation
Additional Information
@BeksOmega I am not familiar with groups. I checked that when a proc call gets disabled in the UI it has a group name but when it is disabled as part of proc def block update it does not have a group name. So I might be totally wrong here =)