feat: Add 'reason' field to move event #6996
Conversation
There are many types of move. This addition allows one to detect what the reason for each move is.
And unsaved editor tabs.
core/events/events_block_move.ts
Outdated
| * 'cleanup' -- Workspace aligned top-level blocks. | ||
| * Reasons may become comma separated if move events merge ('drag,bump,snap'). | ||
| */ | ||
| reason?: string; |
There was a problem hiding this comment.
Can we make the list of reasons an enum? It would mean we get compile-time errors if we e.g. misspell inbounds or try to add a variation like in-bounds, plus give us autocomplete.
Also, can we have the reason property in the event be an array of reasons, instead of a single string with commas? That would be simpler for consumers of this property instead of having to parse the string into an array themselves.
There was a problem hiding this comment.
I don't think an enum would allow developers to create new reasons. Imagine an extension or application which, say, shuffled blocks around randomly to create a puzzle. They should be able to create a custom reason (e.g. 'shuffle').
Changed all reasons to be arrays.
| * 'inbounds' -- Block got pushed back into a non-scrolling workspace. | ||
| * 'connect' -- Block got connected to another block. | ||
| * 'disconnect' -- Block got disconnected from another block. | ||
| * 'create' -- Block created via XML. |
There was a problem hiding this comment.
nit: Block created via deserialization? (since I assume this would also apply for blocks created json?)
There was a problem hiding this comment.
I assumed the same, but there are no move events fired when deserializing from JSON, just when using XML.
core/events/utils.ts
Outdated
| @@ -290,6 +290,17 @@ export function filter(queueIn: Abstract[], forward: boolean): Abstract[] { | |||
| lastEvent.newParentId = moveEvent.newParentId; | |||
| lastEvent.newInputName = moveEvent.newInputName; | |||
| lastEvent.newCoordinate = moveEvent.newCoordinate; | |||
| if (moveEvent.reason) { | |||
| if (lastEvent.reason) { | |||
| // Contatenate reasons into a comma separated string | |||
There was a problem hiding this comment.
nit: Concatenate (if this comment still exists pending comment re: using an array)
There are many types of move. This addition allows one to detect what the reason for each move is.