-
Notifications
You must be signed in to change notification settings - Fork 2.9k
editor: add startEditingShape/stopEditingShape; deprecate setEditingShape #7276
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
| this.emit('select-all-text', { shapeId: shape.id }) | ||
| } | ||
|
|
||
| return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pattern with Editor methods is to return the Editor class instance. I'd add a canEdit method here if the user needs to check, or else check after. Its annoying but that's the pattern, I'd rather not change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha, we could still do a return this. i don't need to do the return true here
| this.setCurrentTool('select.editing_shape', { | ||
| ...(options.info ?? {}), | ||
| target: 'shape', | ||
| shape, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
select is a state that only exists at the tldraw level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah! silly mistake
|
Hey, let me put up a PR that shows a different way of getting to the same outcome. |
This deprecates
setEditingShape, which is a bit too low-level/footgun-y in favor of astartEditingShape/stopEditingShapethat has safer usage semantics.Right now it's hard to start editing shape without doing several checks to make sure that you're safe to start editing.
It currently involves calling
setEditingShape, transitioning or setting the current tool toEditingShapeall while checking if we're in readonly mode and making sure the shape is editable (via thecanEditshape API check).Addresses #7077
Change type
bugfiximprovementfeatureapiotherTest plan
Release notes
API changes
editor: add startEditingShape to better encapsulate proper ergonomics around doing an edit. deprecate setEditingShape
Note
Adds safer editing APIs
startEditingShape/stopEditingShape(with options), deprecatessetEditingShape, updates usages across editor, tools, shapes, examples, and tests, and adjusts related types/exports.Editor.startEditingShape(shapeOrId?, { info, selectAll })that validates editability, selects the shape, and transitions toselect.editing_shape(emitsselect-all-textwhen requested).Editor.stopEditingShape()to clear editing state.Editor.setEditingShapeas@deprecatedand route to internal setter.TLStartEditingShapeOptions.TLResizeShapeOptionstotypes/misc-typesand export from index; remove duplicate definition inEditor.ts.setEditingShapewithstartEditingShape/stopEditingShapeacross select tool states, text/note/arrow/frame behaviors, and helpers (removeselectHelpers.ts).startEditingShape/stopEditingShapebehavior and update existing tests to new APIs.Written by Cursor Bugbot for commit 922d268. This will update automatically on new commits. Configure here.