[bindings] beforeUnbind/afterUnbind to replace beforeDelete/afterDelete (#3761)
Before this PR the interface for doing cleanup when shapes/bindings were deleted was quite footgunny and inexpressive. We were abusing the shape beforeDelete callbacks to implement copy+paste, which doesn't work in situations where cascading deletes are required. This caused bugs in both our pin and sticker examples, where copy+paste was broken. I noticed the same bug in my experiment with text labels, and I think the fact that it took us a while to notice these bugs indicates other users are gonna fall prey to the same bugs unless we help them out. One suggestion to fix this was to add `onAfterDelete(From|To)Shape` callbacks. The cascading deletes could happen in those, while keeping the 'commit changes' kinds of updates in the `before` callbacks and theoretically that would fix the issues with copy+paste. However, expecting people to figure this out on their own is asking a heckuva lot IMO, and it's a heavy bit of nuance to try to convey in the docs. It's hard enough to convey it here. Plus I could imagine for some users it might easily even leave the store in an inconsistent state to allow a bound shape to exist for any length of time after the shape it was bound to was already deleted. It also just makes an already large and muddy API surface area even larger and muddier and if that can be avoided let's avoid it. This PR clears things up by making it so that there's only one callback for when a binding is removed. The callback is given a `reason` for why it is being called The `reason` is one of the following: - The 'from' is being deleted - The 'to' shape is being deleted - The binding is being deleted on it's own. Technically a binding might end up being deleted when both the `from` and `to` shapes are being deleted, but it's very hard to know for certain when that is happening, so I decided to just ignore it for now. I think it would only matter for perf reasons, to avoid doing useless work. So this PR replaces the `onBeforeDelete`, `onAfterDelete`, `onBeforeFromShapeDelete` and `onBeforeToShapeDelete` (and the prospective `onAfterFromShapeDelete` and `onAfterToShapeDelete`) with just two callbacks: - `onBeforeUnbind({binding, reason})` - called before any shapes or the binding have been deleted. - `onAfterUnbind({binding, reason})` - called after the binding and any shapes have been deleted. This still allows all the same behaviour as before, without having to spread the logic between multiple callbacks. It's also just clearer IMO since you only get one callback invocation per unbinding rather than potentially two. It also fixes our copy+paste footgun since we can now implement that by just deleting the bindings rather than invoking the `onBeforeDelete(From|To)Shape` callbacks. I'm not worried about losing the explicit before/after delete callbacks for the binding record or shape records because sdk users still have the ability to detect all those situations with full nuance in obvious ways. The one thing that would even require extra bookkeeping is getting access to a shape record after the shape was deleted, but that's probably not a thing anybody would want to do 🤷🏼 ### Change Type <!-- ❗ Please select a 'Scope' label ❗️ --> - [x] `sdk` — Changes the tldraw SDK - [ ] `dotcom` — Changes the tldraw.com web app - [ ] `docs` — Changes to the documentation, examples, or templates. - [ ] `vs code` — Changes to the vscode plugin - [ ] `internal` — Does not affect user-facing stuff <!-- ❗ Please select a 'Type' label ❗️ --> - [ ] `bugfix` — Bug fix - [ ] `feature` — New feature - [x] `improvement` — Improving existing features - [ ] `chore` — Updating dependencies, other boring stuff - [ ] `galaxy brain` — Architectural changes - [ ] `tests` — Changes to any test code - [ ] `tools` — Changes to infrastructure, CI, internal scripts, debugging tools, etc. - [ ] `dunno` — I don't know ### Test Plan 1. Add a step-by-step description of how to test your PR here. 2. - [ ] Unit Tests - [ ] End to end tests ### Release Notes - Add a brief release note for your PR here.
This commit is contained in:
parent
5b21ad96ae
commit
48fa9018f4
8 changed files with 311 additions and 93 deletions
|
@ -1,6 +1,7 @@
|
|||
import {
|
||||
BindingOnShapeChangeOptions,
|
||||
BindingOnShapeDeleteOptions,
|
||||
BindingOnUnbindOptions,
|
||||
BindingUnbindReason,
|
||||
BindingUtil,
|
||||
Box,
|
||||
DefaultFillStyle,
|
||||
|
@ -250,9 +251,10 @@ class PinBindingUtil extends BindingUtil<PinBinding> {
|
|||
}
|
||||
|
||||
// when the thing we're stuck to is deleted, delete the pin too
|
||||
override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions<PinBinding>): void {
|
||||
const pin = this.editor.getShape<PinShape>(binding.fromId)
|
||||
if (pin) this.editor.deleteShape(pin.id)
|
||||
override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions<PinBinding>): void {
|
||||
if (reason === BindingUnbindReason.DeletingToShape) {
|
||||
this.editor.deleteShape(binding.fromId)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
import {
|
||||
BindingOnShapeChangeOptions,
|
||||
BindingOnShapeDeleteOptions,
|
||||
BindingOnUnbindOptions,
|
||||
BindingUnbindReason,
|
||||
BindingUtil,
|
||||
Box,
|
||||
DefaultToolbar,
|
||||
|
@ -152,9 +153,10 @@ class StickerBindingUtil extends BindingUtil<StickerBinding> {
|
|||
}
|
||||
|
||||
// when the thing we're stuck to is deleted, delete the sticker too
|
||||
override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions<StickerBinding>): void {
|
||||
const sticker = this.editor.getShape<StickerShape>(binding.fromId)
|
||||
if (sticker) this.editor.deleteShape(sticker.id)
|
||||
override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions<StickerBinding>): void {
|
||||
if (reason === BindingUnbindReason.DeletingToShape) {
|
||||
this.editor.deleteShape(binding.fromId)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -187,12 +187,6 @@ export interface BindingOnCreateOptions<Binding extends TLUnknownBinding> {
|
|||
binding: Binding;
|
||||
}
|
||||
|
||||
// @public (undocumented)
|
||||
export interface BindingOnDeleteOptions<Binding extends TLUnknownBinding> {
|
||||
// (undocumented)
|
||||
binding: Binding;
|
||||
}
|
||||
|
||||
// @public (undocumented)
|
||||
export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
|
||||
// (undocumented)
|
||||
|
@ -204,11 +198,21 @@ export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
|
|||
}
|
||||
|
||||
// @public (undocumented)
|
||||
export interface BindingOnShapeDeleteOptions<Binding extends TLUnknownBinding> {
|
||||
export interface BindingOnUnbindOptions<Binding extends TLUnknownBinding> {
|
||||
// (undocumented)
|
||||
binding: Binding;
|
||||
// (undocumented)
|
||||
shape: TLShape;
|
||||
reason: BindingUnbindReason;
|
||||
}
|
||||
|
||||
// @public (undocumented)
|
||||
export enum BindingUnbindReason {
|
||||
// (undocumented)
|
||||
DeletingBinding = "deleting_binding",
|
||||
// (undocumented)
|
||||
DeletingFromShape = "deleting_from_shape",
|
||||
// (undocumented)
|
||||
DeletingToShape = "deleting_to_shape"
|
||||
}
|
||||
|
||||
// @public (undocumented)
|
||||
|
@ -228,17 +232,13 @@ export abstract class BindingUtil<Binding extends TLUnknownBinding = TLUnknownBi
|
|||
// (undocumented)
|
||||
onAfterCreate?(options: BindingOnCreateOptions<Binding>): void;
|
||||
// (undocumented)
|
||||
onAfterDelete?(options: BindingOnDeleteOptions<Binding>): void;
|
||||
onAfterUnbind?(options: BindingOnUnbindOptions<Binding>): void;
|
||||
// (undocumented)
|
||||
onBeforeChange?(options: BindingOnChangeOptions<Binding>): Binding | void;
|
||||
// (undocumented)
|
||||
onBeforeCreate?(options: BindingOnCreateOptions<Binding>): Binding | void;
|
||||
// (undocumented)
|
||||
onBeforeDelete?(options: BindingOnDeleteOptions<Binding>): void;
|
||||
// (undocumented)
|
||||
onBeforeDeleteFromShape?(options: BindingOnShapeDeleteOptions<Binding>): void;
|
||||
// (undocumented)
|
||||
onBeforeDeleteToShape?(options: BindingOnShapeDeleteOptions<Binding>): void;
|
||||
onBeforeUnbind?(options: BindingOnUnbindOptions<Binding>): void;
|
||||
// (undocumented)
|
||||
onOperationComplete?(): void;
|
||||
// (undocumented)
|
||||
|
|
|
@ -124,12 +124,12 @@ export {
|
|||
} from './lib/constants'
|
||||
export { Editor, type TLEditorOptions, type TLResizeShapeOptions } from './lib/editor/Editor'
|
||||
export {
|
||||
BindingUnbindReason,
|
||||
BindingUtil,
|
||||
type BindingOnChangeOptions,
|
||||
type BindingOnCreateOptions,
|
||||
type BindingOnDeleteOptions,
|
||||
type BindingOnShapeChangeOptions,
|
||||
type BindingOnShapeDeleteOptions,
|
||||
type BindingOnUnbindOptions,
|
||||
type TLBindingUtilConstructor,
|
||||
} from './lib/editor/bindings/BindingUtil'
|
||||
export { HistoryManager } from './lib/editor/managers/HistoryManager'
|
||||
|
|
|
@ -118,7 +118,12 @@ import { getIncrementedName } from '../utils/getIncrementedName'
|
|||
import { getReorderingShapesChanges } from '../utils/reorderShapes'
|
||||
import { applyRotationToSnapshotShapes, getRotationSnapshot } from '../utils/rotation'
|
||||
import { uniqueId } from '../utils/uniqueId'
|
||||
import { BindingUtil, TLBindingUtilConstructor } from './bindings/BindingUtil'
|
||||
import {
|
||||
BindingOnUnbindOptions,
|
||||
BindingUnbindReason,
|
||||
BindingUtil,
|
||||
TLBindingUtilConstructor,
|
||||
} from './bindings/BindingUtil'
|
||||
import { bindingsIndex } from './derivations/bindingsIndex'
|
||||
import { notVisibleShapes } from './derivations/notVisibleShapes'
|
||||
import { parentsToChildren } from './derivations/parentsToChildren'
|
||||
|
@ -349,10 +354,16 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
|
||||
this.sideEffects = this.store.sideEffects
|
||||
|
||||
let deletedBindings = new Map<TLBindingId, BindingOnUnbindOptions<any>>()
|
||||
const deletedShapeIds = new Set<TLShapeId>()
|
||||
const invalidParents = new Set<TLShapeId>()
|
||||
let invalidBindingTypes = new Set<string>()
|
||||
this.disposables.add(
|
||||
this.sideEffects.registerOperationCompleteHandler(() => {
|
||||
// this needs to be cleared here because further effects may delete more shapes
|
||||
// and we want the next invocation of this handler to handle those separately
|
||||
deletedShapeIds.clear()
|
||||
|
||||
for (const parentId of invalidParents) {
|
||||
invalidParents.delete(parentId)
|
||||
const parent = this.getShape(parentId)
|
||||
|
@ -375,6 +386,14 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
}
|
||||
}
|
||||
|
||||
if (deletedBindings.size) {
|
||||
const t = deletedBindings
|
||||
deletedBindings = new Map()
|
||||
for (const opts of t.values()) {
|
||||
this.getBindingUtil(opts.binding).onAfterUnbind?.(opts)
|
||||
}
|
||||
}
|
||||
|
||||
this.emit('update')
|
||||
})
|
||||
)
|
||||
|
@ -461,18 +480,13 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
invalidParents.add(shape.parentId)
|
||||
}
|
||||
|
||||
deletedShapeIds.add(shape.id)
|
||||
|
||||
const deleteBindingIds: TLBindingId[] = []
|
||||
for (const binding of this.getBindingsInvolvingShape(shape)) {
|
||||
invalidBindingTypes.add(binding.type)
|
||||
if (binding.fromId === shape.id) {
|
||||
this.getBindingUtil(binding).onBeforeDeleteFromShape?.({ binding, shape })
|
||||
deleteBindingIds.push(binding.id)
|
||||
}
|
||||
if (binding.toId === shape.id) {
|
||||
this.getBindingUtil(binding).onBeforeDeleteToShape?.({ binding, shape })
|
||||
deleteBindingIds.push(binding.id)
|
||||
}
|
||||
}
|
||||
this.deleteBindings(deleteBindingIds)
|
||||
|
||||
const deletedIds = new Set([shape.id])
|
||||
|
@ -510,11 +524,26 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
this.getBindingUtil(bindingAfter).onAfterChange?.({ bindingBefore, bindingAfter })
|
||||
},
|
||||
beforeDelete: (binding) => {
|
||||
this.getBindingUtil(binding).onBeforeDelete?.({ binding })
|
||||
const util = this.getBindingUtil(binding)
|
||||
// No need to track this binding if it's util doesn't care about the unbind operation
|
||||
if (!util.onBeforeUnbind && !util.onAfterUnbind) return
|
||||
// We only want to call this once per binding and it might be possible that the onBeforeUnbind
|
||||
// callback will trigger a nested delete operation on the same binding so let's bail out if
|
||||
// that is happening
|
||||
if (deletedBindings.has(binding.id)) return
|
||||
const opts: BindingOnUnbindOptions<any> = {
|
||||
binding,
|
||||
reason: deletedShapeIds.has(binding.fromId)
|
||||
? BindingUnbindReason.DeletingFromShape
|
||||
: deletedShapeIds.has(binding.toId)
|
||||
? BindingUnbindReason.DeletingToShape
|
||||
: BindingUnbindReason.DeletingBinding,
|
||||
}
|
||||
deletedBindings.set(binding.id, opts)
|
||||
this.getBindingUtil(binding).onBeforeUnbind?.(opts)
|
||||
},
|
||||
afterDelete: (binding) => {
|
||||
invalidBindingTypes.add(binding.type)
|
||||
this.getBindingUtil(binding).onAfterDelete?.({ binding })
|
||||
},
|
||||
},
|
||||
page: {
|
||||
|
@ -2313,7 +2342,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
const { currentScreenPoint, currentPagePoint } = this.inputs
|
||||
const { screenBounds } = this.store.unsafeGetWithoutCapture(TLINSTANCE_ID)!
|
||||
|
||||
// compare the next page point (derived from the curent camera) to the current page point
|
||||
// compare the next page point (derived from the current camera) to the current page point
|
||||
if (
|
||||
currentScreenPoint.x / z - x !== currentPagePoint.x ||
|
||||
currentScreenPoint.y / z - y !== currentPagePoint.y
|
||||
|
@ -2809,7 +2838,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
* editor.zoomToUser(myUserId, { animation: { duration: 200 } })
|
||||
* ```
|
||||
*
|
||||
* @param userId - The id of the user to aniamte to.
|
||||
* @param userId - The id of the user to animate to.
|
||||
* @param opts - The camera move options.
|
||||
* @public
|
||||
*/
|
||||
|
@ -4314,7 +4343,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
|
||||
if (this.isShapeOfType(shape, 'frame')) {
|
||||
// On the rare case that we've hit a frame, test again hitInside to be forced true;
|
||||
// this prevents clicks from passing through the body of a frame to shapes behhind it.
|
||||
// this prevents clicks from passing through the body of a frame to shapes behind it.
|
||||
|
||||
// If the hit is within the frame's outer margin, then select the frame
|
||||
const distance = geometry.distanceToPoint(pointInShapeSpace, hitInside)
|
||||
|
@ -4396,7 +4425,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
inMarginClosestToEdgeHit = shape
|
||||
}
|
||||
} else if (!inMarginClosestToEdgeHit) {
|
||||
// If we're not within margin distnce to any edge, and if the
|
||||
// If we're not within margin distance to any edge, and if the
|
||||
// shape is hollow, then we want to hit the shape with the
|
||||
// smallest area. (There's a bug here with self-intersecting
|
||||
// shapes, like a closed drawing of an "8", but that's a bigger
|
||||
|
@ -4472,7 +4501,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
const { hitInside = false, margin = 0 } = opts
|
||||
const id = typeof shape === 'string' ? shape : shape.id
|
||||
// If the shape is masked, and if the point falls outside of that
|
||||
// mask, then it's defintely a miss—we don't need to test further.
|
||||
// mask, then it's definitely a miss—we don't need to test further.
|
||||
const pageMask = this.getShapeMask(id)
|
||||
if (pageMask && !pointInPolygon(point, pageMask)) return false
|
||||
|
||||
|
@ -4792,7 +4821,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
|
||||
const shapesToReparent = compact(ids.map((id) => this.getShape(id)))
|
||||
|
||||
// The user is allowed to re-parent locked shapes. Unintutive? Yeah! But there are plenty of
|
||||
// The user is allowed to re-parent locked shapes. Unintuitive? Yeah! But there are plenty of
|
||||
// times when a locked shape's parent is deleted... and we need to put that shape somewhere!
|
||||
const lockedShapes = shapesToReparent.filter((shape) => shape.isLocked)
|
||||
|
||||
|
@ -4900,7 +4929,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
*
|
||||
* @param ids - The ids of the shapes to get descendants of.
|
||||
*
|
||||
* @returns The decscendant ids.
|
||||
* @returns The descendant ids.
|
||||
*
|
||||
* @public
|
||||
*/
|
||||
|
@ -8347,7 +8376,7 @@ export class Editor extends EventEmitter<TLEventMap> {
|
|||
|
||||
let behavior = wheelBehavior
|
||||
|
||||
// If the camera behavior is "zoom" and the ctrl key is presssed, then pan;
|
||||
// If the camera behavior is "zoom" and the ctrl key is pressed, then pan;
|
||||
// If the camera behavior is "pan" and the ctrl key is not pressed, then zoom
|
||||
if (inputs.ctrlKey) behavior = wheelBehavior === 'pan' ? 'zoom' : 'pan'
|
||||
|
||||
|
@ -8705,8 +8734,7 @@ function withoutBindingsToUnrelatedShapes<T>(
|
|||
callback: (bindingsWithBoth: Set<TLBindingId>) => T
|
||||
): T {
|
||||
const bindingsWithBoth = new Set<TLBindingId>()
|
||||
const bindingsWithoutFrom = new Set<TLBindingId>()
|
||||
const bindingsWithoutTo = new Set<TLBindingId>()
|
||||
const bindingsToRemove = new Set<TLBindingId>()
|
||||
|
||||
for (const shapeId of shapeIds) {
|
||||
const shape = editor.getShape(shapeId)
|
||||
|
@ -8719,11 +8747,8 @@ function withoutBindingsToUnrelatedShapes<T>(
|
|||
bindingsWithBoth.add(binding.id)
|
||||
continue
|
||||
}
|
||||
if (!hasFrom) {
|
||||
bindingsWithoutFrom.add(binding.id)
|
||||
}
|
||||
if (!hasTo) {
|
||||
bindingsWithoutTo.add(binding.id)
|
||||
if (!hasFrom || !hasTo) {
|
||||
bindingsToRemove.add(binding.id)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -8732,31 +8757,7 @@ function withoutBindingsToUnrelatedShapes<T>(
|
|||
|
||||
editor.history.ignore(() => {
|
||||
const changes = editor.store.extractingChanges(() => {
|
||||
const bindingsToRemove: TLBindingId[] = []
|
||||
|
||||
for (const bindingId of bindingsWithoutFrom) {
|
||||
const binding = editor.getBinding(bindingId)
|
||||
if (!binding) continue
|
||||
|
||||
const shape = editor.getShape(binding.fromId)
|
||||
if (!shape) continue
|
||||
|
||||
editor.getBindingUtil(binding).onBeforeDeleteFromShape?.({ binding, shape })
|
||||
bindingsToRemove.push(binding.id)
|
||||
}
|
||||
|
||||
for (const bindingId of bindingsWithoutTo) {
|
||||
const binding = editor.getBinding(bindingId)
|
||||
if (!binding) continue
|
||||
|
||||
const shape = editor.getShape(binding.toId)
|
||||
if (!shape) continue
|
||||
|
||||
editor.getBindingUtil(binding).onBeforeDeleteToShape?.({ binding, shape })
|
||||
bindingsToRemove.push(binding.id)
|
||||
}
|
||||
|
||||
editor.deleteBindings(bindingsToRemove)
|
||||
editor.deleteBindings([...bindingsToRemove])
|
||||
|
||||
try {
|
||||
result = Result.ok(callback(bindingsWithBoth))
|
||||
|
|
|
@ -18,14 +18,22 @@ export interface BindingOnCreateOptions<Binding extends TLUnknownBinding> {
|
|||
}
|
||||
|
||||
/** @public */
|
||||
export interface BindingOnChangeOptions<Binding extends TLUnknownBinding> {
|
||||
bindingBefore: Binding
|
||||
bindingAfter: Binding
|
||||
export enum BindingUnbindReason {
|
||||
DeletingFromShape = 'deleting_from_shape',
|
||||
DeletingToShape = 'deleting_to_shape',
|
||||
DeletingBinding = 'deleting_binding',
|
||||
}
|
||||
|
||||
/** @public */
|
||||
export interface BindingOnDeleteOptions<Binding extends TLUnknownBinding> {
|
||||
export interface BindingOnUnbindOptions<Binding extends TLUnknownBinding> {
|
||||
binding: Binding
|
||||
reason: BindingUnbindReason
|
||||
}
|
||||
|
||||
/** @public */
|
||||
export interface BindingOnChangeOptions<Binding extends TLUnknownBinding> {
|
||||
bindingBefore: Binding
|
||||
bindingAfter: Binding
|
||||
}
|
||||
|
||||
/** @public */
|
||||
|
@ -35,12 +43,6 @@ export interface BindingOnShapeChangeOptions<Binding extends TLUnknownBinding> {
|
|||
shapeAfter: TLShape
|
||||
}
|
||||
|
||||
/** @public */
|
||||
export interface BindingOnShapeDeleteOptions<Binding extends TLUnknownBinding> {
|
||||
binding: Binding
|
||||
shape: TLShape
|
||||
}
|
||||
|
||||
/** @public */
|
||||
export abstract class BindingUtil<Binding extends TLUnknownBinding = TLUnknownBinding> {
|
||||
constructor(public editor: Editor) {}
|
||||
|
@ -63,17 +65,15 @@ export abstract class BindingUtil<Binding extends TLUnknownBinding = TLUnknownBi
|
|||
|
||||
onOperationComplete?(): void
|
||||
|
||||
onBeforeUnbind?(options: BindingOnUnbindOptions<Binding>): void
|
||||
onAfterUnbind?(options: BindingOnUnbindOptions<Binding>): void
|
||||
|
||||
// self lifecycle hooks
|
||||
onBeforeCreate?(options: BindingOnCreateOptions<Binding>): Binding | void
|
||||
onAfterCreate?(options: BindingOnCreateOptions<Binding>): void
|
||||
onBeforeChange?(options: BindingOnChangeOptions<Binding>): Binding | void
|
||||
onAfterChange?(options: BindingOnChangeOptions<Binding>): void
|
||||
onBeforeDelete?(options: BindingOnDeleteOptions<Binding>): void
|
||||
onAfterDelete?(options: BindingOnDeleteOptions<Binding>): void
|
||||
|
||||
onAfterChangeFromShape?(options: BindingOnShapeChangeOptions<Binding>): void
|
||||
onAfterChangeToShape?(options: BindingOnShapeChangeOptions<Binding>): void
|
||||
|
||||
onBeforeDeleteFromShape?(options: BindingOnShapeDeleteOptions<Binding>): void
|
||||
onBeforeDeleteToShape?(options: BindingOnShapeDeleteOptions<Binding>): void
|
||||
}
|
||||
|
|
|
@ -2,7 +2,8 @@ import {
|
|||
BindingOnChangeOptions,
|
||||
BindingOnCreateOptions,
|
||||
BindingOnShapeChangeOptions,
|
||||
BindingOnShapeDeleteOptions,
|
||||
BindingOnUnbindOptions,
|
||||
BindingUnbindReason,
|
||||
BindingUtil,
|
||||
Editor,
|
||||
IndexKey,
|
||||
|
@ -60,7 +61,9 @@ export class ArrowBindingUtil extends BindingUtil<TLArrowBinding> {
|
|||
}
|
||||
|
||||
// when the shape the arrow is pointing to is deleted
|
||||
override onBeforeDeleteToShape({ binding }: BindingOnShapeDeleteOptions<TLArrowBinding>): void {
|
||||
override onBeforeUnbind({ binding, reason }: BindingOnUnbindOptions<TLArrowBinding>): void {
|
||||
// don't need to do anything if the arrow is being deleted
|
||||
if (reason === BindingUnbindReason.DeletingFromShape) return
|
||||
const arrow = this.editor.getShape<TLArrowShape>(binding.fromId)
|
||||
if (!arrow) return
|
||||
unbindArrowTerminal(this.editor, arrow, binding.props.terminal)
|
||||
|
|
210
packages/tldraw/src/test/bindings.test.tsx
Normal file
210
packages/tldraw/src/test/bindings.test.tsx
Normal file
|
@ -0,0 +1,210 @@
|
|||
import {
|
||||
BindingOnUnbindOptions,
|
||||
BindingUnbindReason,
|
||||
BindingUtil,
|
||||
TLShapeId,
|
||||
TLUnknownBinding,
|
||||
createBindingId,
|
||||
createShapeId,
|
||||
} from '@tldraw/editor'
|
||||
import { TestEditor } from './TestEditor'
|
||||
import { TL } from './test-jsx'
|
||||
|
||||
let editor: TestEditor
|
||||
|
||||
const ids = {
|
||||
box1: createShapeId('box1'),
|
||||
box2: createShapeId('box2'),
|
||||
box3: createShapeId('box3'),
|
||||
box4: createShapeId('box4'),
|
||||
}
|
||||
|
||||
const mockOnBeforeUnbind = jest.fn() as jest.Mock<void, [BindingOnUnbindOptions<TLUnknownBinding>]>
|
||||
const mockOnAfterUnbind = jest.fn() as jest.Mock<void, [BindingOnUnbindOptions<TLUnknownBinding>]>
|
||||
|
||||
class TestBindingUtil extends BindingUtil {
|
||||
static override type = 'test'
|
||||
|
||||
static override props = {}
|
||||
|
||||
override getDefaultProps(): object {
|
||||
return {}
|
||||
}
|
||||
|
||||
override onBeforeUnbind(options: BindingOnUnbindOptions<TLUnknownBinding>): void {
|
||||
mockOnBeforeUnbind(options)
|
||||
}
|
||||
|
||||
override onAfterUnbind(options: BindingOnUnbindOptions<TLUnknownBinding>): void {
|
||||
mockOnAfterUnbind(options)
|
||||
}
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
editor = new TestEditor({ bindingUtils: [TestBindingUtil] })
|
||||
|
||||
editor.createShapesFromJsx([
|
||||
<TL.geo id={ids.box1} x={0} y={0} />,
|
||||
<TL.geo id={ids.box2} x={0} y={0} />,
|
||||
<TL.geo id={ids.box3} x={0} y={0} />,
|
||||
<TL.geo id={ids.box4} x={0} y={0} />,
|
||||
])
|
||||
|
||||
mockOnBeforeUnbind.mockClear()
|
||||
mockOnAfterUnbind.mockClear()
|
||||
})
|
||||
|
||||
function bindShapes(fromId: TLShapeId, toId: TLShapeId) {
|
||||
const bindingId = createBindingId()
|
||||
editor.createBinding({
|
||||
id: bindingId,
|
||||
type: 'test',
|
||||
fromId,
|
||||
toId,
|
||||
})
|
||||
return bindingId
|
||||
}
|
||||
|
||||
test('deleting the from shape causes the reason to be "deleting_from_shape"', () => {
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.deleteShape(ids.box1)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingFromShape })
|
||||
)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingFromShape })
|
||||
)
|
||||
})
|
||||
|
||||
test('deleting the to shape causes the reason to be "deleting_to_shape"', () => {
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.deleteShape(ids.box2)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingToShape })
|
||||
)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingToShape })
|
||||
)
|
||||
})
|
||||
|
||||
test('deleting the binding itself causes the reason to be "deleting_binding"', () => {
|
||||
const bindingId = bindShapes(ids.box1, ids.box2)
|
||||
editor.deleteBinding(bindingId)
|
||||
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
|
||||
)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
|
||||
)
|
||||
})
|
||||
|
||||
test('copying both bound shapes does not trigger the unbind operation', () => {
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.select(ids.box1, ids.box2)
|
||||
editor.copy()
|
||||
expect(mockOnBeforeUnbind).not.toHaveBeenCalled()
|
||||
expect(mockOnAfterUnbind).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('copying the from shape on its own does trigger the unbind operation', () => {
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.select(ids.box1)
|
||||
editor.copy()
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
|
||||
)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
|
||||
)
|
||||
})
|
||||
|
||||
test('copying the to shape on its own does trigger the unbind operation', () => {
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.select(ids.box2)
|
||||
editor.copy()
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
|
||||
)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(1)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: BindingUnbindReason.DeletingBinding })
|
||||
)
|
||||
})
|
||||
|
||||
test('cascading deletes in afterUnbind are handled correctly', () => {
|
||||
mockOnAfterUnbind.mockImplementation((options) => {
|
||||
if (options.reason === BindingUnbindReason.DeletingFromShape) {
|
||||
editor.deleteShape(options.binding.toId)
|
||||
}
|
||||
})
|
||||
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
bindShapes(ids.box2, ids.box3)
|
||||
bindShapes(ids.box3, ids.box4)
|
||||
|
||||
editor.deleteShape(ids.box1)
|
||||
|
||||
expect(editor.getShape(ids.box1)).toBeUndefined()
|
||||
expect(editor.getShape(ids.box2)).toBeUndefined()
|
||||
expect(editor.getShape(ids.box3)).toBeUndefined()
|
||||
expect(editor.getShape(ids.box4)).toBeUndefined()
|
||||
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(3)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
test('cascading deletes in beforeUnbind are handled correctly', () => {
|
||||
mockOnBeforeUnbind.mockImplementation((options) => {
|
||||
if (options.reason === BindingUnbindReason.DeletingFromShape) {
|
||||
editor.deleteShape(options.binding.toId)
|
||||
}
|
||||
})
|
||||
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
bindShapes(ids.box2, ids.box3)
|
||||
bindShapes(ids.box3, ids.box4)
|
||||
|
||||
editor.deleteShape(ids.box1)
|
||||
|
||||
expect(editor.getShape(ids.box1)).toBeUndefined()
|
||||
expect(editor.getShape(ids.box2)).toBeUndefined()
|
||||
expect(editor.getShape(ids.box3)).toBeUndefined()
|
||||
expect(editor.getShape(ids.box4)).toBeUndefined()
|
||||
|
||||
expect(mockOnBeforeUnbind).toHaveBeenCalledTimes(3)
|
||||
expect(mockOnAfterUnbind).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
test('beforeUnbind is called before the from shape is deleted or the binding is deleted', () => {
|
||||
mockOnBeforeUnbind.mockImplementationOnce(() => {
|
||||
expect(editor.getShape(ids.box1)).toBeDefined()
|
||||
expect(editor.getShape(ids.box2)).toBeDefined()
|
||||
expect(editor.getBindingsFromShape(ids.box1, 'test')).toHaveLength(1)
|
||||
})
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.deleteShape(ids.box1)
|
||||
|
||||
expect.assertions(3)
|
||||
})
|
||||
|
||||
test('beforeUnbind is called before the to shape is deleted or the binding is deleted', () => {
|
||||
mockOnBeforeUnbind.mockImplementationOnce(() => {
|
||||
expect(editor.getShape(ids.box1)).toBeDefined()
|
||||
expect(editor.getShape(ids.box2)).toBeDefined()
|
||||
expect(editor.getBindingsToShape(ids.box2, 'test')).toHaveLength(1)
|
||||
})
|
||||
bindShapes(ids.box1, ids.box2)
|
||||
editor.deleteShape(ids.box2)
|
||||
|
||||
expect.assertions(3)
|
||||
})
|
Loading…
Reference in a new issue