From 2c8b431c1f31d82c1e67045370475f73dc5ec634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Mon, 15 May 2023 17:37:54 +0200 Subject: [PATCH] [fix] Allow interactions with embeds in readonly mode (#1333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow the users to interact with embeds in readonly mode. Not sure if this should apply to all embeds (cc @orangemug for thoughts on this)? For example, you can also start editing code sandbox embeds. One thing that doesn't feel quite right is that readonly mode defaults to the hand tool, so you always have to switch to select tool to get this working. I guess moving around is still the more common action though so 🤷 ### Test Plan 1. Create a multiplayer room with some embeds (youtube videos, spotify playlist). 2. Open the room in readonly mode. 3. Make sure you can interact with embeds. Double click / enter when selected, you should then be able to play the youtube videos. - [x] Unit Tests - [ ] Webdriver tests ### Release Note - Allow the users to interact with embeds in readonly mode. --- .../statechart/TLSelectTool/children/Idle.ts | 85 +++++++++++++------ .../src/lib/test/tools/TLSelectTool.test.ts | 38 +++++++++ 2 files changed, 95 insertions(+), 28 deletions(-) diff --git a/packages/editor/src/lib/app/statechart/TLSelectTool/children/Idle.ts b/packages/editor/src/lib/app/statechart/TLSelectTool/children/Idle.ts index 3139c51a2..46da243a7 100644 --- a/packages/editor/src/lib/app/statechart/TLSelectTool/children/Idle.ts +++ b/packages/editor/src/lib/app/statechart/TLSelectTool/children/Idle.ts @@ -43,7 +43,7 @@ export class Idle extends StateNode { onPointerDown: TLEventHandlers['onPointerDown'] = (info) => { if (this.app.isMenuOpen) return - const shouldEnterCropMode = this.shouldEnterCropMode(info) + const shouldEnterCropMode = this.shouldEnterCropMode(info, true) if (info.ctrlKey && !shouldEnterCropMode) { this.parent.transition('brushing', info) @@ -157,8 +157,8 @@ export class Idle extends StateNode { const { shape } = info const util = this.app.getShapeUtil(shape) - // Allow video playing - if (shape.type !== 'video' && this.app.isReadOnly) break + // Allow playing videos and embeds + if (shape.type !== 'video' && shape.type !== 'embed' && this.app.isReadOnly) break if (util.onDoubleClick) { // Call the shape's double click handler @@ -268,43 +268,72 @@ export class Idle extends StateNode { } onKeyUp = (info: TLKeyboardEventInfo) => { - if (this.app.isReadOnly) return - switch (info.code) { - case 'Enter': { - const { selectedShapes } = this.app - - if (selectedShapes.every((shape) => shape.type === 'group')) { - this.app.setSelectedIds( - selectedShapes.flatMap((shape) => this.app.getSortedChildIds(shape.id)) - ) - return + if (this.app.isReadOnly) { + switch (info.code) { + case 'Enter': { + if (this.shouldStartEditingShape() && this.app.onlySelectedShape) { + this.startEditingShape(this.app.onlySelectedShape, { + ...info, + target: 'shape', + shape: this.app.onlySelectedShape, + }) + return + } + break } + } + } else { + switch (info.code) { + case 'Enter': { + const { selectedShapes } = this.app - const { onlySelectedShape } = this.app - const util = onlySelectedShape && this.app.getShapeUtil(onlySelectedShape) - if (util && util.canEdit(onlySelectedShape)) { - this.startEditingShape(onlySelectedShape, { - ...info, - target: 'shape', - shape: onlySelectedShape, - }) - return - } + if (selectedShapes.every((shape) => shape.type === 'group')) { + this.app.setSelectedIds( + selectedShapes.flatMap((shape) => this.app.getSortedChildIds(shape.id)) + ) + return + } - if (util && util.canCrop(onlySelectedShape)) { - this.parent.transition('crop', info) + if (this.shouldStartEditingShape() && this.app.onlySelectedShape) { + this.startEditingShape(this.app.onlySelectedShape, { + ...info, + target: 'shape', + shape: this.app.onlySelectedShape, + }) + return + } + + if (this.shouldEnterCropMode(info, false)) { + this.parent.transition('crop', info) + } + break } - break } } } - private shouldEnterCropMode(info: TLPointerEventInfo): boolean { + private shouldStartEditingShape(): boolean { + const { onlySelectedShape } = this.app + if (!onlySelectedShape) return false + + const util = this.app.getShapeUtil(onlySelectedShape) + return util.canEdit(onlySelectedShape) + } + + private shouldEnterCropMode( + info: TLPointerEventInfo | TLKeyboardEventInfo, + withCtrlKey: boolean + ): boolean { const singleShape = this.app.onlySelectedShape if (!singleShape) return false const shapeUtil = this.app.getShapeUtil(singleShape) - return shapeUtil.canCrop(singleShape) && info.ctrlKey + // Should the Ctrl key be pressed to enter crop mode + if (withCtrlKey) { + return shapeUtil.canCrop(singleShape) && info.ctrlKey + } else { + return shapeUtil.canCrop(singleShape) + } } private startEditingShape(shape: TLShape, info: TLClickEventInfo | TLKeyboardEventInfo) { diff --git a/packages/editor/src/lib/test/tools/TLSelectTool.test.ts b/packages/editor/src/lib/test/tools/TLSelectTool.test.ts index 67088416f..ff6163239 100644 --- a/packages/editor/src/lib/test/tools/TLSelectTool.test.ts +++ b/packages/editor/src/lib/test/tools/TLSelectTool.test.ts @@ -5,6 +5,7 @@ let app: TestApp const ids = { box1: createCustomShapeId('box1'), + embed1: createCustomShapeId('embed1'), } jest.useFakeTimers() @@ -350,3 +351,40 @@ describe('When editing shapes', () => { it.todo('restores selection after changing styles') }) + +describe('When in readonly mode', () => { + beforeEach(() => { + app.createShapes([ + { + id: ids.embed1, + type: 'embed', + x: 100, + y: 100, + props: { opacity: '1', w: 100, h: 100, url: '', doesResize: false }, + }, + ]) + app.updateUserDocumentSettings({ isReadOnly: true }) + }) + + it('Begins editing embed when double clicked', () => { + expect(app.editingId).toBe(null) + expect(app.selectedIds.length).toBe(0) + expect(app.isReadOnly).toBe(true) + + const shape = app.getShapeById(ids.embed1) + app.doubleClick(100, 100, { target: 'shape', shape }) + expect(app.editingId).toBe(ids.embed1) + }) + + it('Begins editing embed when pressing Enter on a selected embed', () => { + expect(app.editingId).toBe(null) + expect(app.selectedIds.length).toBe(0) + expect(app.isReadOnly).toBe(true) + + app.setSelectedIds([ids.embed1]) + expect(app.selectedIds.length).toBe(1) + + app.keyUp('Enter') + expect(app.editingId).toBe(ids.embed1) + }) +})