reppners / ngx-drag-drop

Angular directives using the native HTML Drag And Drop API
https://reppners.github.io/ngx-drag-drop/
BSD 3-Clause "New" or "Revised" License
300 stars 119 forks source link

Remove event.stopPropagation in dragover so it bubbles up #14

Closed mtraynham closed 6 years ago

mtraynham commented 6 years ago

Some scrollbar libraries support scrolling on dragover. This component is stopping the propagation of that event, so I've simply removed the stopPropagation to allow that event to bubble up. I checked angular-drag-and-drop-lists and it looks like this same stopPropagation has been there since the beginning. I don't believe that it's entirely necessary to stop these events if they've been handled by the component, e.g. preventing it from bubbling up.

reppners commented 6 years ago

I'm not exactly sure but the stopPropagation() could be there to support nesting drop zones. Need to check that, but I'm pretty sure there is way to enable nesting dropzones and letting the event bubble up.

mtraynham commented 6 years ago

@reppners thanks for the clarification, you're probably right. Could we instead have a flag or options object that allowed bubbling?

reppners commented 6 years ago

If there is no other way to make nested dropzones work we can introduce an API to explicitly enable/disable event bubbling but lets check first if there is another way to nest dropzones with event bubbling being the default.

Using https://developer.mozilla.org/en-US/docs/Web/API/Event/defaultPrevented should do the trick.

mtraynham commented 6 years ago

Not sure I completely follow, are you suggesting we could just bail out of the drop @HostListener's if it was already prevented by a nested component?

reppners commented 6 years ago

Yes, basically early exit in a parent dropzone handler when a child dropzone invoked preventDefault()

mtraynham commented 6 years ago

@reppners Hey man, I finally got around to getting your suggestion in (and rebased against master). I think that should handle it, but I haven't tried it yet.

reppners commented 6 years ago

@mtraynham Small change and well documented, thanks! I'll try to review this week 👍

reppners commented 6 years ago

All working as intended! Thanks for your contribution 👍