diff --git a/CHANGES.md b/CHANGES.md index 98e3a212b9..db692c6c4b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -38,6 +38,7 @@ - Token tree should be expanded by default [Taiga #13631](https://tree.taiga.io/project/penpot/issue/13631) - Fix opacity incorrectly disabled for visible shapes [Taiga #13906](https://tree.taiga.io/project/penpot/issue/13906) - Update onboarding image [Taiga #13864](https://tree.taiga.io/project/penpot/issue/13864) +- Fix plugin modal drag interactions over iframe and close-button behavior (by @marekhrabe) [Github #8871](https://github.com/penpot/penpot/pull/8871) - Fix hot update on color-row on texts [Taiga #13923](https://tree.taiga.io/project/penpot/issue/13923) - Fix selected color tokens [Taiga #13930](https://tree.taiga.io/project/penpot/issue/13930) diff --git a/plugins/libs/plugins-runtime/src/lib/drag-handler.spec.ts b/plugins/libs/plugins-runtime/src/lib/drag-handler.spec.ts index 18ec9ef75c..8989b55eab 100644 --- a/plugins/libs/plugins-runtime/src/lib/drag-handler.spec.ts +++ b/plugins/libs/plugins-runtime/src/lib/drag-handler.spec.ts @@ -1,11 +1,45 @@ import { expect, describe, vi } from 'vitest'; import { dragHandler } from './drag-handler.js'; +type PointerLikeEvent = MouseEvent & { pointerId: number }; + +function createPointerEvent( + type: string, + init: Partial = {}, +): PointerLikeEvent { + const event = new MouseEvent(type, { + bubbles: true, + cancelable: true, + clientX: init.clientX ?? 0, + clientY: init.clientY ?? 0, + button: init.button ?? 0, + }) as PointerLikeEvent; + + Object.defineProperty(event, 'pointerId', { + configurable: true, + value: init.pointerId ?? 1, + }); + + return event; +} + describe('dragHandler', () => { let element: HTMLElement; beforeEach(() => { element = document.createElement('div'); + Object.defineProperty(element, 'setPointerCapture', { + configurable: true, + value: vi.fn(), + }); + Object.defineProperty(element, 'releasePointerCapture', { + configurable: true, + value: vi.fn(), + }); + Object.defineProperty(element, 'hasPointerCapture', { + configurable: true, + value: vi.fn().mockReturnValue(true), + }); document.body.appendChild(element); }); @@ -14,65 +48,109 @@ describe('dragHandler', () => { vi.clearAllMocks(); }); - it('should attach mousedown event listener to the element', () => { + it('should attach pointerdown event listener to the element', () => { const addEventListenerMock = vi.spyOn(element, 'addEventListener'); dragHandler(element); expect(addEventListenerMock).toHaveBeenCalledWith( - 'mousedown', + 'pointerdown', expect.any(Function), ); }); - it('should update element transform on mousemove', () => { - const mouseDownEvent = new MouseEvent('mousedown', { + it('should update element transform on pointermove', () => { + const pointerDownEvent = createPointerEvent('pointerdown', { clientX: 100, clientY: 100, }); dragHandler(element); - element.dispatchEvent(mouseDownEvent); + element.dispatchEvent(pointerDownEvent); - const mouseMoveEvent = new MouseEvent('mousemove', { + const pointerMoveEvent = createPointerEvent('pointermove', { clientX: 150, clientY: 150, }); - document.dispatchEvent(mouseMoveEvent); + element.dispatchEvent(pointerMoveEvent); expect(element.style.transform).toBe('translate(50px, 50px)'); - const mouseMoveEvent2 = new MouseEvent('mousemove', { + const pointerMoveEvent2 = createPointerEvent('pointermove', { clientX: 200, clientY: 200, }); - document.dispatchEvent(mouseMoveEvent2); + element.dispatchEvent(pointerMoveEvent2); expect(element.style.transform).toBe('translate(100px, 100px)'); }); - it('should remove event listeners on mouseup', () => { - const removeEventListenerMock = vi.spyOn(document, 'removeEventListener'); - - const mouseDownEvent = new MouseEvent('mousedown', { + it('should run lifecycle callbacks on drag start/end', () => { + const start = vi.fn(); + const end = vi.fn(); + const pointerDownEvent = createPointerEvent('pointerdown', { clientX: 100, clientY: 100, + pointerId: 2, + }); + const pointerUpEvent = createPointerEvent('pointerup', { + pointerId: 2, }); - dragHandler(element); + dragHandler(element, element, undefined, { start, end }); + element.dispatchEvent(pointerDownEvent); + element.dispatchEvent(pointerUpEvent); - element.dispatchEvent(mouseDownEvent); + expect(start).toHaveBeenCalledTimes(1); + expect(end).toHaveBeenCalledTimes(1); + expect(element.releasePointerCapture).toHaveBeenCalledWith(2); + }); - const mouseUpEvent = new MouseEvent('mouseup'); - document.dispatchEvent(mouseUpEvent); + it('should ignore pointerdown events from button targets', () => { + const start = vi.fn(); + const button = document.createElement('button'); + const icon = document.createElement('span'); + button.appendChild(icon); + element.appendChild(button); + + dragHandler(element, element, undefined, { start }); + + icon.dispatchEvent( + createPointerEvent('pointerdown', { + pointerId: 5, + button: 0, + }), + ); + + expect(start).not.toHaveBeenCalled(); + expect(element.setPointerCapture).not.toHaveBeenCalled(); + }); + + it('should remove pointer listeners on teardown', () => { + const removeEventListenerMock = vi.spyOn(element, 'removeEventListener'); + + const cleanup = dragHandler(element); + cleanup(); expect(removeEventListenerMock).toHaveBeenCalledWith( - 'mousemove', + 'pointerdown', expect.any(Function), ); expect(removeEventListenerMock).toHaveBeenCalledWith( - 'mouseup', + 'pointermove', + expect.any(Function), + ); + expect(removeEventListenerMock).toHaveBeenCalledWith( + 'pointerup', + expect.any(Function), + ); + expect(removeEventListenerMock).toHaveBeenCalledWith( + 'pointercancel', + expect.any(Function), + ); + expect(removeEventListenerMock).toHaveBeenCalledWith( + 'lostpointercapture', expect.any(Function), ); }); diff --git a/plugins/libs/plugins-runtime/src/lib/drag-handler.ts b/plugins/libs/plugins-runtime/src/lib/drag-handler.ts index 9ee5c83c93..fd79f07d3e 100644 --- a/plugins/libs/plugins-runtime/src/lib/drag-handler.ts +++ b/plugins/libs/plugins-runtime/src/lib/drag-handler.ts @@ -1,14 +1,36 @@ import { parseTranslate } from './parse-translate'; +type DragHandlerLifecycle = { + start?: () => void; + end?: () => void; +}; + export const dragHandler = ( el: HTMLElement, target: HTMLElement = el, move?: () => void, + lifecycle?: DragHandlerLifecycle, ) => { let initialTranslate = { x: 0, y: 0 }; let initialClientPosition = { x: 0, y: 0 }; + let pointerId: number | null = null; + let dragging = false; + + const endDrag = () => { + if (!dragging) { + return; + } + + dragging = false; + pointerId = null; + lifecycle?.end?.(); + }; + + const handlePointerMove = (moveEvent: PointerEvent) => { + if (!dragging || moveEvent.pointerId !== pointerId) { + return; + } - const handleMouseMove = (moveEvent: MouseEvent) => { const { clientX: moveX, clientY: moveY } = moveEvent; const deltaX = moveX - initialClientPosition.x + initialTranslate.x; const deltaY = moveY - initialClientPosition.y + initialTranslate.y; @@ -18,19 +40,70 @@ export const dragHandler = ( move?.(); }; - const handleMouseUp = () => { - document.removeEventListener('mousemove', handleMouseMove); - document.removeEventListener('mouseup', handleMouseUp); + const handlePointerUp = (upEvent: PointerEvent) => { + if (upEvent.pointerId !== pointerId) { + return; + } + + if (el.hasPointerCapture(upEvent.pointerId)) { + el.releasePointerCapture(upEvent.pointerId); + } + + endDrag(); }; - const handleMouseDown = (e: MouseEvent) => { + const handlePointerCancel = (cancelEvent: PointerEvent) => { + if (cancelEvent.pointerId !== pointerId) { + return; + } + + endDrag(); + }; + + const handleLostPointerCapture = (lostCaptureEvent: PointerEvent) => { + if (lostCaptureEvent.pointerId !== pointerId) { + return; + } + + endDrag(); + }; + + const handlePointerDown = (e: PointerEvent) => { + if (e.button !== 0) { + return; + } + + const fromButton = + e.target instanceof Element && !!e.target.closest('button'); + + if (fromButton) { + return; + } + + e.preventDefault(); + initialClientPosition = { x: e.clientX, y: e.clientY }; initialTranslate = parseTranslate(target); - document.addEventListener('mousemove', handleMouseMove); - document.addEventListener('mouseup', handleMouseUp); + + dragging = true; + pointerId = e.pointerId; + lifecycle?.start?.(); + + el.setPointerCapture(e.pointerId); }; - el.addEventListener('mousedown', handleMouseDown); + el.addEventListener('pointerdown', handlePointerDown); + el.addEventListener('pointermove', handlePointerMove); + el.addEventListener('pointerup', handlePointerUp); + el.addEventListener('pointercancel', handlePointerCancel); + el.addEventListener('lostpointercapture', handleLostPointerCapture); - return handleMouseUp; + return () => { + el.removeEventListener('pointerdown', handlePointerDown); + el.removeEventListener('pointermove', handlePointerMove); + el.removeEventListener('pointerup', handlePointerUp); + el.removeEventListener('pointercancel', handlePointerCancel); + el.removeEventListener('lostpointercapture', handleLostPointerCapture); + endDrag(); + }; }; diff --git a/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.spec.ts b/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.spec.ts new file mode 100644 index 0000000000..d7b774c2c3 --- /dev/null +++ b/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.spec.ts @@ -0,0 +1,125 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import './plugin-modal.js'; + +type PointerLikeEvent = MouseEvent & { pointerId: number }; + +function createPointerEvent( + type: string, + init: Partial = {}, +): PointerLikeEvent { + const event = new MouseEvent(type, { + bubbles: true, + cancelable: true, + clientX: init.clientX ?? 0, + clientY: init.clientY ?? 0, + button: init.button ?? 0, + }) as PointerLikeEvent; + + Object.defineProperty(event, 'pointerId', { + configurable: true, + value: init.pointerId ?? 1, + }); + + return event; +} + +describe('PluginModalElement', () => { + let setPointerCaptureSpy: ReturnType; + let releasePointerCaptureSpy: ReturnType; + let hasPointerCaptureSpy: ReturnType; + let originalSetPointerCapture: typeof HTMLElement.prototype.setPointerCapture; + let originalReleasePointerCapture: typeof HTMLElement.prototype.releasePointerCapture; + let originalHasPointerCapture: typeof HTMLElement.prototype.hasPointerCapture; + + beforeEach(() => { + originalSetPointerCapture = HTMLElement.prototype.setPointerCapture; + originalReleasePointerCapture = HTMLElement.prototype.releasePointerCapture; + originalHasPointerCapture = HTMLElement.prototype.hasPointerCapture; + + setPointerCaptureSpy = vi.fn(); + releasePointerCaptureSpy = vi.fn(); + hasPointerCaptureSpy = vi.fn().mockReturnValue(true); + + Object.defineProperty(HTMLElement.prototype, 'setPointerCapture', { + configurable: true, + value: setPointerCaptureSpy, + }); + Object.defineProperty(HTMLElement.prototype, 'releasePointerCapture', { + configurable: true, + value: releasePointerCaptureSpy, + }); + Object.defineProperty(HTMLElement.prototype, 'hasPointerCapture', { + configurable: true, + value: hasPointerCaptureSpy, + }); + }); + + afterEach(() => { + document.body.innerHTML = ''; + Object.defineProperty(HTMLElement.prototype, 'setPointerCapture', { + configurable: true, + value: originalSetPointerCapture, + }); + Object.defineProperty(HTMLElement.prototype, 'releasePointerCapture', { + configurable: true, + value: originalReleasePointerCapture, + }); + Object.defineProperty(HTMLElement.prototype, 'hasPointerCapture', { + configurable: true, + value: originalHasPointerCapture, + }); + vi.restoreAllMocks(); + }); + + it('should not start dragging on close button pointerdown', () => { + const modal = document.createElement('plugin-modal'); + modal.setAttribute('title', 'Test modal'); + modal.setAttribute('iframe-src', 'about:blank'); + document.body.appendChild(modal); + + const shadow = modal.shadowRoot; + expect(shadow).toBeTruthy(); + + const wrapper = shadow?.querySelector('.wrapper'); + const closeButton = shadow?.querySelector('button'); + + expect(wrapper).toBeTruthy(); + expect(closeButton).toBeTruthy(); + + closeButton?.dispatchEvent( + createPointerEvent('pointerdown', { + pointerId: 11, + button: 0, + }), + ); + + expect(wrapper?.classList.contains('is-dragging')).toBe(false); + expect(setPointerCaptureSpy).not.toHaveBeenCalled(); + + modal.remove(); + }); + + it('should dispatch close event when close button is clicked', () => { + const modal = document.createElement('plugin-modal'); + modal.setAttribute('title', 'Test modal'); + modal.setAttribute('iframe-src', 'about:blank'); + + const onClose = vi.fn(); + modal.addEventListener('close', onClose); + document.body.appendChild(modal); + + const closeButton = modal.shadowRoot?.querySelector('button'); + expect(closeButton).toBeTruthy(); + + closeButton?.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + cancelable: true, + }), + ); + + expect(onClose).toHaveBeenCalledTimes(1); + + modal.remove(); + }); +}); diff --git a/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.ts b/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.ts index 3346175212..c61ad7fce5 100644 --- a/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.ts +++ b/plugins/libs/plugins-runtime/src/lib/modal/plugin-modal.ts @@ -67,11 +67,6 @@ export class PluginModalElement extends HTMLElement { this.wrapper.style.maxInlineSize = '90vw'; this.wrapper.style.maxBlockSize = '90vh'; - // move modal to the top - this.#dragEvents = dragHandler(this.#inner, this.wrapper, () => { - this.calculateZIndex(); - }); - const header = document.createElement('div'); header.classList.add('header'); @@ -124,6 +119,23 @@ export class PluginModalElement extends HTMLElement { ); }); + // move modal to the top + this.#dragEvents = dragHandler( + header, + this.wrapper, + () => { + this.calculateZIndex(); + }, + { + start: () => { + this.wrapper.classList.add('is-dragging'); + }, + end: () => { + this.wrapper.classList.remove('is-dragging'); + }, + }, + ); + this.addEventListener('message', (e: Event) => { if (!iframe.contentWindow) { return; diff --git a/plugins/libs/plugins-runtime/src/lib/modal/plugin.modal.css b/plugins/libs/plugins-runtime/src/lib/modal/plugin.modal.css index 4ac13c45d9..bd987442d2 100644 --- a/plugins/libs/plugins-runtime/src/lib/modal/plugin.modal.css +++ b/plugins/libs/plugins-runtime/src/lib/modal/plugin.modal.css @@ -42,6 +42,8 @@ min-inline-size: 25px; min-block-size: 200px; resize: both; + user-select: none; + -webkit-user-select: none; &:after { content: ''; cursor: se-resize; @@ -58,7 +60,6 @@ .inner { padding: 10px; - cursor: grab; box-sizing: border-box; display: flex; flex-direction: column; @@ -78,6 +79,12 @@ justify-content: space-between; border-block-end: 2px solid var(--color-background-quaternary); padding-block-end: var(--spacing-4); + cursor: grab; + touch-action: none; +} + +.wrapper.is-dragging .header { + cursor: grabbing; } button { @@ -92,7 +99,6 @@ h1 { font-weight: var(--font-weight-bold); margin: 0; margin-inline-end: var(--spacing-4); - user-select: none; } iframe {