From 197649d2fa87cf90a0d06be8a58f3d3f58684eba Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Mon, 30 Aug 2021 17:23:14 -0400 Subject: [PATCH 1/3] Surface event for file-attachment-dragged The implementation for https://github.com/github/github/pull/190219 needs an event to kick off finding the cursor position, and rather than bubbling the DragEvent it seems prudent to fire a CustomEvent solely used by that consumer. That'll avoid any unintended side effects in the document at large. --- src/file-attachment-element.ts | 11 +++++++++++ test/test.js | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/file-attachment-element.ts b/src/file-attachment-element.ts index 119b027..95908d6 100644 --- a/src/file-attachment-element.ts +++ b/src/file-attachment-element.ts @@ -73,6 +73,17 @@ function onDragenter(event: DragEvent) { if (dragging) { clearTimeout(dragging) } + + const dragEvent = new CustomEvent('file-attachment-dragged', { + bubbles: true, + cancelable: true, + detail: { + dragEvent: event, + attachmentElement: target + } + }) + target.dispatchEvent(dragEvent) + dragging = window.setTimeout(() => target.removeAttribute('hover'), 200) const transfer = event.dataTransfer diff --git a/test/test.js b/test/test.js index 411092c..bd34c11 100644 --- a/test/test.js +++ b/test/test.js @@ -124,6 +124,24 @@ describe('file-attachment', function () { assert.equal('test.png', event.detail.attachments[0].file.name) assert.equal(0, input.files.length) }) + + it('fires a file-attachment-dragged event on dragenter', async function () { + const listener = once('file-attachment-dragged') + const dragEvent = new Event('dragenter', {bubbles: true}) + input.dispatchEvent(dragEvent) + const event = await listener + assert.equal(dragEvent, event.detail.dragEvent) + assert.equal(fileAttachment, event.detail.attachmentElement) + }) + + it('fires a file-attachment-dragged event on dragover', async function () { + const listener = once('file-attachment-dragged') + const dragEvent = new Event('dragover', {bubbles: true}) + input.dispatchEvent(dragEvent) + const event = await listener + assert.equal(dragEvent, event.detail.dragEvent) + assert.equal(fileAttachment, event.detail.attachmentElement) + }) }) }) From 312836ef6939e2e1b63eb9a6464724d64254fb53 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Wed, 1 Sep 2021 09:47:30 -0400 Subject: [PATCH 2/3] [pr review] use better name for event target --- src/file-attachment-element.ts | 2 +- test/test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/file-attachment-element.ts b/src/file-attachment-element.ts index 95908d6..f0516aa 100644 --- a/src/file-attachment-element.ts +++ b/src/file-attachment-element.ts @@ -79,7 +79,7 @@ function onDragenter(event: DragEvent) { cancelable: true, detail: { dragEvent: event, - attachmentElement: target + target } }) target.dispatchEvent(dragEvent) diff --git a/test/test.js b/test/test.js index bd34c11..b544b02 100644 --- a/test/test.js +++ b/test/test.js @@ -131,7 +131,7 @@ describe('file-attachment', function () { input.dispatchEvent(dragEvent) const event = await listener assert.equal(dragEvent, event.detail.dragEvent) - assert.equal(fileAttachment, event.detail.attachmentElement) + assert.equal(fileAttachment, event.detail.target) }) it('fires a file-attachment-dragged event on dragover', async function () { @@ -140,7 +140,7 @@ describe('file-attachment', function () { input.dispatchEvent(dragEvent) const event = await listener assert.equal(dragEvent, event.detail.dragEvent) - assert.equal(fileAttachment, event.detail.attachmentElement) + assert.equal(fileAttachment, event.detail.target) }) }) }) From ae3143835452b4091419c4dd4b872ad2886b13f2 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Wed, 1 Sep 2021 11:01:14 -0400 Subject: [PATCH 3/3] Bubble original event rather than a new custom one We decided we could use the `dragover` and `dragenter` events directly instead of wrapping them with a custom one. We're still calling `preventDefault()` so we won't accidentally trigger any native behavior downstream. --- src/file-attachment-element.ts | 11 ----------- test/test.js | 32 ++++++++++++++++++++++---------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/file-attachment-element.ts b/src/file-attachment-element.ts index f0516aa..286b0af 100644 --- a/src/file-attachment-element.ts +++ b/src/file-attachment-element.ts @@ -74,16 +74,6 @@ function onDragenter(event: DragEvent) { clearTimeout(dragging) } - const dragEvent = new CustomEvent('file-attachment-dragged', { - bubbles: true, - cancelable: true, - detail: { - dragEvent: event, - target - } - }) - target.dispatchEvent(dragEvent) - dragging = window.setTimeout(() => target.removeAttribute('hover'), 200) const transfer = event.dataTransfer @@ -92,7 +82,6 @@ function onDragenter(event: DragEvent) { transfer.dropEffect = 'copy' target.setAttribute('hover', '') - event.stopPropagation() event.preventDefault() } diff --git a/test/test.js b/test/test.js index b544b02..7ebdba1 100644 --- a/test/test.js +++ b/test/test.js @@ -125,22 +125,34 @@ describe('file-attachment', function () { assert.equal(0, input.files.length) }) - it('fires a file-attachment-dragged event on dragenter', async function () { - const listener = once('file-attachment-dragged') - const dragEvent = new Event('dragenter', {bubbles: true}) + it('bubbles the dragenter event after cancelling its default behavior', async function () { + const dataTransfer = new DataTransfer() + const file = new File(['hubot'], 'test.txt', {type: 'text/plain'}) + dataTransfer.items.add(file) + + const dragEvent = new DragEvent('dragenter', {bubbles: true, cancelable: true, dataTransfer}) + + const listener = once('dragenter') input.dispatchEvent(dragEvent) + const event = await listener - assert.equal(dragEvent, event.detail.dragEvent) - assert.equal(fileAttachment, event.detail.target) + assert.equal(dragEvent, event) + assert.equal(true, event.defaultPrevented) }) - it('fires a file-attachment-dragged event on dragover', async function () { - const listener = once('file-attachment-dragged') - const dragEvent = new Event('dragover', {bubbles: true}) + it('bubbles the dragover event after cancelling its default behavior', async function () { + const dataTransfer = new DataTransfer() + const file = new File(['hubot'], 'test.txt', {type: 'text/plain'}) + dataTransfer.items.add(file) + + const dragEvent = new DragEvent('dragover', {bubbles: true, cancelable: true, dataTransfer}) + + const listener = once('dragover') input.dispatchEvent(dragEvent) + const event = await listener - assert.equal(dragEvent, event.detail.dragEvent) - assert.equal(fileAttachment, event.detail.target) + assert.equal(dragEvent, event) + assert.equal(true, event.defaultPrevented) }) }) })