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!
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 Event.target, etc, etc.
Don't use document events for individual elements no matter what some folks out there say. Likewise don't use Event.target 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 Event.target.
Remember, Event.target is the lowest element clicked on. Event.currentTarget is the Element the event is attached to!
Oh and avoid garbage like Element.style 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.
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.
codepen.io/jason-knight/pen/poWzbNJ
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 Event.target, etc, etc.
Don't use document events for individual elements no matter what some folks out there say. Likewise don't use Event.target 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 Event.target.
Remember, Event.target is the lowest element clicked on. Event.currentTarget is the Element the event is attached to!
Oh and avoid garbage like Element.style 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.
Wow that's a lot of fixes! Thanks for sharing your insightful thoughts 😃