search

google / blockly

The web-based visual programming editor.
https://developers.google.com/blockly/
Apache License 2.0
12.34k stars 3.69k forks source link

Fix shadow blocks to suppress delete event #960

Open vicng opened 7 years ago

vicng commented 7 years ago

Replacing a shadow block unnecessarily fires a delete event on the shadow block. Assuming web adds a viewable trash can (like Android/iOS), this causes a problem where a shadow block could show up in the trash can by doing an undo, then redo.

Instead, the delete event on the shadow block should be suppressed and the parent block should handle deleting/re-creating shadow blocks based on its connection state.

AnmAtAnm commented 7 years ago

This would mean change handlers looking for such blocks might need to aware of the shadow state of the block they are looking for. That sort of required extra state awareness seems very suspect.

I also don't think we can assume "delete" means "trash" in the context of events. I would rather treat trash as a different root container that has it's own "workspace" id and event stream. In this manner, trash would also trigger a create event on that workspace.

rachel-fenichel commented 7 years ago

Shadow blocks are only deleted by other blocks being dragged over them, which is a connect event. So the shadow block deletion is deterministic based on the connect event.

On Fri, Mar 3, 2017 at 12:39 AM Andrew n marshall notifications@github.com wrote:

This would mean change handlers looking for such blocks might need to aware of the shadow state of the block they are looking for. That sort of required extra state awareness seems very suspect.

I also don't think we can assume "delete" means "trash" in the context of events. I would rather treat trash as a different root container that has it's own "workspace" id and event stream. In this manner, trash would also trigger a create event on that workspace.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/blockly/issues/960#issuecomment-283897369, or mute the thread https://github.com/notifications/unsubscribe-auth/ANDWf7R5WAFVqg75z0DuFGIJx_5Uq2j4ks5rh9GmgaJpZM4MRxoT .

RoboErikG commented 7 years ago

I think Anm is talking about workspace event listeners that do something when specific blocks are deleted/created. If one of those blocks is a shadow block the workspace listener wouldn't work.

I can't think of an existing use case, and there are ways around it, but it would be really hard to debug if you ever hit it. I still think not firing delete/create events for shadow blocks makes sense, but if a concrete use case comes up we may need to re-evaluate.