Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rework canBind callback #3797

Merged
merged 4 commits into from
May 23, 2024
Merged

rework canBind callback #3797

merged 4 commits into from
May 23, 2024

Conversation

SomeHats
Copy link
Contributor

This PR reworks the canBind callback to work with customizable bindings. It now accepts an object with a the shape, the other shape (optional - it may not exist yet), the direction, and the type of the binding. Devs can use this to create shapes that only participate in certain binding types, can have bindings from but not to them, etc.

If you're implementing a binding, you can see if binding two shapes is allowed using editor.canBindShapes(fromShape, toShape, 'my binding type')

Change Type

  • sdk — Changes the tldraw SDK
  • improvement — Improving existing features

Release Notes

Breaking changes

The canBind flag now accepts an options object instead of just the shape in question. If you're relying on its arguments, you need to change from canBind(shape) {} to canBind({shape}) {}.

Copy link

vercel bot commented May 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
examples ✅ Ready (Inspect) Visit Preview May 23, 2024 1:17pm
tldraw-docs ✅ Ready (Inspect) Visit Preview May 23, 2024 1:17pm

@huppy-bot huppy-bot bot added improvement Improves existing features sdk Affects the tldraw sdk labels May 21, 2024
@SomeHats SomeHats requested a review from ds300 May 21, 2024 14:57
@SomeHats SomeHats mentioned this pull request May 21, 2024
2 tasks
apps/examples/src/examples/pin-bindings/PinExample.tsx Outdated Show resolved Hide resolved
Comment on lines 5231 to 5238
const canBindFrom = from
? this.getShapeUtil(from).canBind({ shape: from, otherShape: to, direction: 'from', type })
: undefined
const canBindTo = to
? this.getShapeUtil(to).canBind({ shape: to, otherShape: from, direction: 'to', type })
: undefined

return canBindFrom !== false && canBindTo !== false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels quite heavy. I wonder whether we need to actually refer to the instances of the shapes when deciding if it can bind, for the time being can we just use the shape type and then maybe add support for deciding based on the actual shape instances later if anybody needs it?

So we could have e.g. editor.canBindShapes({fromShapeType, toShapeType, bindingType}) and then the shape utils can implement the same interface canBind({ fromShapeType, toShapeType, bindingType }) too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need the actual instances in some cases (or at least... we need to go from ID to type first). i've moved those to the callsites though - i think i prefer this the way it is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves existing features sdk Affects the tldraw sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants