Fix cropping issue with shapes that do not set isCircle#7926
Closed
jamesbvaughan wants to merge 1 commit intotldraw:mainfrom
Closed
Fix cropping issue with shapes that do not set isCircle#7926jamesbvaughan wants to merge 1 commit intotldraw:mainfrom
isCircle#7926jamesbvaughan wants to merge 1 commit intotldraw:mainfrom
Conversation
|
@jamesbvaughan is attempting to deploy a commit to the tldraw Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Sorry @jamesbvaughan, we're not accepting pull requests from external contributors at this time. See discussion here. If you'd like to report a bug or suggest a feature, please open an issue instead. |
Collaborator
|
Reopening this one, sorry for the bouncer. I'm going to add some tests here to verify everything. |
2 tasks
Collaborator
|
Closing this one again in favor of #7931. Thanks James! |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 14, 2026
In order to fix custom croppable shapes crashing when cropped (#7927), this PR prevents `getCropBox` from adding `isCircle: undefined` to the result crop object. The `isCircle` property was introduced for circle crops on image shapes. However, `getCropBox` was unconditionally copying it onto every result crop via `isCircle: crop.isCircle`. When the input crop didn't have `isCircle` set, this produced `{ isCircle: undefined }` — and `Object.keys()` includes keys with `undefined` values. Custom shapes with their own crop validators (without an `isCircle` field) would then fail with `ValidationError: Unexpected property` at `isCircle`. The fix only copies `isCircle` when it was explicitly set on the input crop (`!= null`). Closes #7927 Based on the fix from #7926 by @jamesbvaughan. ### Change type - [x] `bugfix` ### Test plan 1. Create a custom croppable shape without `isCircle` in its crop validator 2. Crop the shape — should no longer crash - [x] Unit tests ### Release notes - Fixed a crash when cropping custom shapes that don't include `isCircle` in their crop schema <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, localized change to crop serialization plus added tests; main risk is minor behavior change for callers that implicitly relied on `isCircle` always being present. > > **Overview** > Fixes a cropping regression where `getCropBox` would always copy `crop.isCircle` into the returned crop object, causing `isCircle: undefined` to appear and break custom shape crop validators. > > `getCropBox` now only includes `isCircle` in the output when it’s explicitly set (non-null/undefined) on the input crop, and new unit tests cover absence/undefined cases plus validation against a custom crop schema without `isCircle`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b6c7164. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: James Vaughan <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
huppy-bot bot
pushed a commit
that referenced
this pull request
Feb 14, 2026
In order to fix custom croppable shapes crashing when cropped (#7927), this PR prevents `getCropBox` from adding `isCircle: undefined` to the result crop object. The `isCircle` property was introduced for circle crops on image shapes. However, `getCropBox` was unconditionally copying it onto every result crop via `isCircle: crop.isCircle`. When the input crop didn't have `isCircle` set, this produced `{ isCircle: undefined }` — and `Object.keys()` includes keys with `undefined` values. Custom shapes with their own crop validators (without an `isCircle` field) would then fail with `ValidationError: Unexpected property` at `isCircle`. The fix only copies `isCircle` when it was explicitly set on the input crop (`!= null`). Closes #7927 Based on the fix from #7926 by @jamesbvaughan. ### Change type - [x] `bugfix` ### Test plan 1. Create a custom croppable shape without `isCircle` in its crop validator 2. Crop the shape — should no longer crash - [x] Unit tests ### Release notes - Fixed a crash when cropping custom shapes that don't include `isCircle` in their crop schema <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, localized change to crop serialization plus added tests; main risk is minor behavior change for callers that implicitly relied on `isCircle` always being present. > > **Overview** > Fixes a cropping regression where `getCropBox` would always copy `crop.isCircle` into the returned crop object, causing `isCircle: undefined` to appear and break custom shape crop validators. > > `getCropBox` now only includes `isCircle` in the output when it’s explicitly set (non-null/undefined) on the input crop, and new unit tests cover absence/undefined cases plus validation against a custom crop schema without `isCircle`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b6c7164. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: James Vaughan <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7927
This fixes a bug where custom shapes that are croppable end up with invalid props when cropped, causing the application to crash.
If I understand the problem correctly, this started happening with the introduction of the
isCircleproperty onTLShapeCrop. This specific code path made it possible for that property to end upundefined, which causes record validation to fail.This branch fixes that by making
getCropBoxonly setisCircleif it was set already.I know that this PR will get auto-closed with the new contribution policy. I'll also create an issue to start a discussion about this.
Change type
bugfiximprovementfeatureapiotherRelease notes