From 267fea8d5a8940d4514025c0d5dbbc14833d0311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Tue, 16 May 2023 16:33:57 +0200 Subject: [PATCH] Delete an empty text shape when clicking on another text shape. (#1384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes an issue with empty text shape not being deleted when you clicked on another text shape. This correctly worked if you clicked on a shape of a different type or on canvas. Before: https://github.com/tldraw/tldraw/assets/2523721/cf79a0a5-c738-49d2-a861-4e23eafc29e5 After: https://github.com/tldraw/tldraw/assets/2523721/51a31f7e-c0da-45bc-9d04-aa0b0752a459 ### Change Type - [x] `patch` — Bug Fix ### Test Plan 1. Create a text shape and add some text. 2. Double click on the empty canvas, which creates an empty text shape. 3. Click on the first text shape. Confirm that the empty text shape was deleted and is no longer present. - [x] Unit Tests - [ ] Webdriver tests ### Release Notes - Fix a problem with empty text shapes not getting deleted if you clicked on another text shape. --- .../TLSelectTool/children/EditingShape.ts | 3 ++ .../src/lib/test/tools/TLSelectTool.test.ts | 45 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/editor/src/lib/app/statechart/TLSelectTool/children/EditingShape.ts b/packages/editor/src/lib/app/statechart/TLSelectTool/children/EditingShape.ts index e1180fe7f..55760519a 100644 --- a/packages/editor/src/lib/app/statechart/TLSelectTool/children/EditingShape.ts +++ b/packages/editor/src/lib/app/statechart/TLSelectTool/children/EditingShape.ts @@ -59,6 +59,9 @@ export class EditingShape extends StateNode { const editingShape = this.app.getShapeById(editingId) if (editingShape) { + const editingShapeUtil = this.app.getShapeUtil(editingShape) + editingShapeUtil.onEditEnd?.(editingShape) + const util = this.app.getShapeUtil(shape) // If the user has clicked onto a different shape of the same type diff --git a/packages/editor/src/lib/test/tools/TLSelectTool.test.ts b/packages/editor/src/lib/test/tools/TLSelectTool.test.ts index ff6163239..b40114663 100644 --- a/packages/editor/src/lib/test/tools/TLSelectTool.test.ts +++ b/packages/editor/src/lib/test/tools/TLSelectTool.test.ts @@ -349,6 +349,51 @@ describe('When editing shapes', () => { expect(app.onlySelectedShape?.id).toBe(ids.text2) }) + it('Double clicking the canvas creates a new text shape', () => { + expect(app.editingId).toBe(null) + expect(app.selectedIds.length).toBe(0) + expect(app.shapesArray.length).toBe(5) + app.doubleClick(750, 750) + expect(app.shapesArray.length).toBe(6) + expect(app.shapesArray[5].type).toBe('text') + }) + + it('It deletes an empty text shape when your click away', () => { + expect(app.editingId).toBe(null) + expect(app.selectedIds.length).toBe(0) + expect(app.shapesArray.length).toBe(5) + + // Create a new shape by double clicking + app.doubleClick(750, 750) + expect(app.selectedIds.length).toBe(1) + expect(app.shapesArray.length).toBe(6) + const shapeId = app.selectedIds[0] + + // Click away + app.click(1000, 1000) + expect(app.selectedIds.length).toBe(0) + expect(app.shapesArray.length).toBe(5) + expect(app.getShapeById(shapeId)).toBe(undefined) + }) + + it('It deletes an empty text shape when your click another text shape', () => { + expect(app.editingId).toBe(null) + expect(app.selectedIds.length).toBe(0) + expect(app.shapesArray.length).toBe(5) + + // Create a new shape by double clicking + app.doubleClick(750, 750) + expect(app.selectedIds.length).toBe(1) + expect(app.shapesArray.length).toBe(6) + const shapeId = app.selectedIds[0] + + // Click another text shape + app.click(50, 50, { target: 'shape', shape: app.getShapeById(ids.text1) }) + expect(app.selectedIds.length).toBe(1) + expect(app.shapesArray.length).toBe(5) + expect(app.getShapeById(shapeId)).toBe(undefined) + }) + it.todo('restores selection after changing styles') })