DEV Community

Discussion on: 🚀 A powerful drag and drop implementation in just 16 lines of JavaScript

deathshadow60 profile image
deathshadow60 • Edited on

I'd suggest adding {} around the outside creating a scope so LET serves a purpose. "AppendChild" will automatically do a "removeChild" so you don't need that line. The overhead of the "function for nothing" is doing you no favors, the use of "target" instead of "currentTarget" is why some people are seeing behavioral bugs. Not even sure why you're messing around with setting the data type to text/plain... did you just blindly copy that part from a tutorial?

Though the worst problem is the hardcoding in the markup of the event preventing re-usability, lack of target restrictions, and that you hooked "document" instead of the actual elements!

Here's a "fixed" fork.

Uses data- attributes so that each "set" can self-target. A lot of the logic you were tracking isn't necessary because it targets elements, it auto-hooks itself to all relevant elements, doesn't have those pesky false drop targets because again, it hooks the elements with Event.currentTarget not document and, etc, etc.

Don't use document events for individual elements no matter what some folks out there say. Likewise don't use it can give you the wrong origin and/or destination if a cascade of elements is involved. You start putting tags inside your draggables, those elements will report as

Remember, is the lowest element clicked on. Event.currentTarget is the Element the event is attached to!

Oh and avoid garbage like if you can. Unless you're actually calculating values, that too is just bad code. Why? 1) Presentation has no more business in your JS than it does your HTML, 2) it makes it harder for people to style it since you have to dive into logic to just set the colours.

Anyhow, the script's not even 100 bytes larger despite all the added functionality.

siddharthshyniben profile image
Siddharth Author

Wow that's a lot of fixes! Thanks for sharing your insightful thoughts 😃