vue-drag-drop icon indicating copy to clipboard operation
vue-drag-drop copied to clipboard

debounce events

Open ckritzinger opened this issue 6 years ago • 9 comments

Ideally, I would have liked to let the debounce time to be passed to the component as a property, but I'm still a bit of a n00b with Vue, so I wasn't able to get that bit right (the properties are not available at the time when the debounced function is created)

Other than that, it seems to work mostly as I would expect.

Apologies if the code is crappy, I'm still on the steep part of the JS/Vue learning curve. Happy to incorporate stylistic/design changes if you suggest any.

ckritzinger avatar Feb 17 '18 19:02 ckritzinger

Thanks, nice work. I'll take a closer look at this tomorrow.

cameronhimself avatar Feb 17 '18 20:02 cameronhimself

Haven't forgotten about this.

cameronhimself avatar Feb 20 '18 14:02 cameronhimself

I'm getting some really peculiar behavior when I test this. Regular events that shouldn't be debounced are firing at strange times, and the events that should be debounced (drag and dragover) aren't being fired at all until the drag operation is complete. Additionally, it's prohibiting dragover styling in the example--it flashes darker grey briefly after the drag leaves the dropzone, whereas it should be darker grey the entire time. Take a look:

debounce

Are you not seeing this behavior when you test it?

cameronhimself avatar Feb 22 '18 03:02 cameronhimself

Apologies, you're 100% correct. Thanks for the feedback, I clearly wasn't thinking properly.

As far as I can tell, the events are now firing roughly correctly.

There is still an issue with drag-over styling in the demo app, which I'm unsure how to handle, maybe you can advise:

in Styling.vue, you have

		<drop class="drop"
			:class="{ over }"
			@dragover="over = true"
			@dragleave="over = false"
			@drop="handleDrop">
				drop
		</drop>

Now, because the dragover events are being debounced, they might end up firing after a "dragleave" event, which means the dragover styling is applied and then not removed.

There are two options:

  1. Set the dragover styling on the dragenter event with @dragenter="over = true"
  2. Don't debounce the dragover event

I prefer option 1, since the dragover event can be pretty noisy

ckritzinger avatar Feb 26 '18 07:02 ckritzinger

I appreciate your continued effort on this. I haven't tested your new commit yet, but I would say that any event that can be debounced should have the option to not be debounced at all—meaning its dispatch function should be called directly, not wrapped in any sort of debounce decorator. There are going to be times when users need the complete flow of rapidly fired drag/dragover events, and forcing them through a debounce wrapper unconditionally is going to render certain use cases impossible to implement.

When I originally started to take a crack at this, I used a debounce prop (in milliseconds) for this reason. If the prop is present, push the drag/dragover events through a debounce wrapper using the debounce prop as an argument. If the prop is 0 (which should be the default), bypass the wrapper altogether.

cameronhimself avatar Feb 26 '18 19:02 cameronhimself

Yea, agreed about making it configurable. I wanted to do it initially (see my initial comment), but only now have an idea of how to sensibly implement it. Will update the PR at some point this week

ckritzinger avatar Feb 27 '18 08:02 ckritzinger

@cameronhimself @ckritzinger Hey guys, is this PR still active? I need this change :)

sbward avatar May 09 '18 21:05 sbward

We ended up running into way too many cross-browser issues from relying on HTML5 drag. So we wrote our own implementation (that looks more like the Jquery version).

If I get around to packaging that as a vue plugin, I'll let you know.

ckritzinger avatar May 10 '18 10:05 ckritzinger

@sbward This feature isn't particularly high on my priority list. If someone else would like to pick it back up before I get to it I'd be happy to work with them, of course.

cameronhimself avatar May 10 '18 16:05 cameronhimself