fix!: various drag-adjacent interfaces #7975
Conversation
github-actions
bot
added
breaking change
BeksOmega
force-pushed
the
fix/gesture-interfaces
branch
from
d381540
to
6106214
Compare
| @@ -105,7 +108,9 @@ export function registerCopy() { | |||
| !workspace.options.readOnly && | |||
| !Gesture.inProgress() && | |||
| selected != null && | |||
| isDeletable(selected) && | |||
There was a problem hiding this comment.
I see what it's doing, but this code feels weird: isDeletable(selected) && selected.isDeletable(). Without context I would have no idea what eat part of that is doing. Is there anything you can do with the name to make it clearer? Maybe implementsDeletable(selected) && selected.isDeletable()?
There was a problem hiding this comment.
Now draggable, selectable, and deletable are inconsistent. Can you change all of them to implementsX?
If no because of other dependencies, better to have them consistently be isX.
There was a problem hiding this comment.
I think the issue is we already have a bunch of type guards that are isX, so unless we want to break all of those we're going to have inconsistency anyway. Is it better to have a group of type guards be different or just the one?
There was a problem hiding this comment.
Okay, then revert to isDeletable.
BeksOmega
force-pushed
the
fix/gesture-interfaces
branch
from
59d9e5b
to
6106214
Compare
github-actions
bot
added
breaking change
The basics
The details
Resolves
Fixes N/A
Proposed Changes
Fixes up the
ISelectableandIDeletableinterfaces to confirm to the proposed design. Also switches everything over to the newIDraggableinterfaces. This is a breaking change becauseIDraggableclasses are no longer required to beIDeletableas well.Test Coverage
Manually tested that all the dragging and drag targets continue to work.
Documentation
N/A
Additional Information
N/A
Breaking changes / To fix
This change modifies the
IDeleteableinterface to add adisposeand asetDeleteStylemethod. If you were implementing this interface, you should add those two methods. It is unlikely you were or should be implementing this interface, because there is no way to add new rendered elements to Blockly.This change modifies the
ISelectableinterface to stop extendingIDeletableorIMovable. If you were relying on callingselectable.isDeletableon anISelectableyou should change this toBlockly.isDeletable(selectable) && selectable.isDeletable(). The first call ensures that the block has anisDeletablemethod. If you were relying on callingselectable.isMovableon anISelectableyou should change this toBlockly.isDraggable(selectable) && selectable.isMovable(). The first call ensures that the block has anisMovablemethod.