Functions stop working after code refactoring – JavaScript – SitePoint Forums

Using my original code, I was able to drag two small DIVs (test1 and test2) by hovering the mouse cursor over the DIVs, holding the mouse down and dragging the mouse as seen in Original_Code .

I refactored this code to make it more readable by removing the mousemove and mouseup functions from the mousedown event listener callback function and placing them somewhere else, as can be seen in the Refactored_Code. I will eventually put them in an external file. The problem now is that after the drag stops and the mouseup event fires, the mousemove event listener doesn’t seem to be suppressed by the mouseup callback and stops the DIVs from moving.

The mouseup event listener is an anonymous function, so there is no way to pass it to the removeEventListener function.
removeEventListener only works on named functions.

Hi Dennis,
Thank you for your reply. Is there a way around this?

First bind event listeners to the element being moved, then use named functions for the listeners.

let tgt = document.getElementById('test1');
tgt.addEventListener("mousemove", mouseMove, false);
tgt.addEventListener("mouse, mouseUp, false);

function mouseMove(e) {
    // code here
    }
function mouseUp(e) {
    // code here
    }

Of course draggable and resizing are out of scope, so they will need to be set where event listeners can access them.

Hi Dennis,
Thank you for your reply. Your code below does not allow ‘e’ and ‘draggable’ arguments to be passed to callback functions.

tgt.addEventListener(“mousemove”, C, false);
tgt.addEventListener(“mouse, mouseUp, false);

My current code allows these arguments to be passed to callbacks. The mousemove function works but removeEvenLlistener does not.

You said at the beginning:

Trying to work backwards from something that’s broken to something that works can be a complicated process due to many assumptions that are made.

Instead of backtracking, I’ll start with working code (always a bonus) and try to use more appropriate techniques to get the desired result instead.

Extracting the mousemove() function from the mousedown event listener, there is a problem where it says: Uncaught ReferenceError: draggable is not defined

What’s happening is that the mousedown function wants to update draggable, and mousemove wants to use that draggable information. When multiple functions want to access the same variable, you set the variable to a higher common parent location, so that one function can update it and the other function can access it.

In this case, I defined draggable in the same place as mainBox and handles. (yes, I renamed MainBox to the more appropriate mainBox too).

document.addEventListener('DOMContentLoaded', function() {

    var mainBox = document.querySelector("#MainBox");
    var handles = document.querySelectorAll(".handle");
    var draggable;

This way we can update draggable from mousedown() function:

    mainBox.addEventListener("mousedown", function(ev) {

        var isResizing = false;
        draggable = ev.target;

and we can access the draggable variable from the mousemove() function.

    function mousemove(e) {

        const draggableRect = draggable.getBoundingClientRect();
        const parentRect = mainBox.getBoundingClientRect();

Here is the updated code https://jsfiddle.net/fqo1tx0m/

Hi Paul,
Thank you for your reply. I think you didn’t fully answer the question I had. In your updated code you only extracted the mousemove function but not the mouseup function. I would also like to extract the mouseup function.

Also, I think the extracted mousemove function in my refactoredCode wasn’t the problem because it works and I was able to move my DIVs around. The problem lies in the extracted mouseup function, for some reason when this function is invoked it fails to remove the mousemove event listener.

One step after another

The mouseup, mousedown and mousemove functions have been extracted in this updated code.
No other significant changes should have occurred. https://jsfiddle.net/wz4aqke2/

Hint: When a moved item hits a wall, for example against the left wall, it would be less annoying if, instead of freezing in place, this moved item moved up and down the wall until at the same vertical position as the mouse.

Thank you again for your help. I think the reason your updated code works is because you put the extracted mousemove and mouseup functions in front of the line

BlockquotemainBox.addEventListener(“mousedown”, function(ev)

This means that mousemove and mouseup functions are defined before they are used. To be honest, I never thought this would be a problem since Javascript allows using functions before defining them.

You can only delete named event listeners. There is no identifier for the removeEventListener to identify the event listener.

See the link above for adding and removing an event listener.
For example, a click event handler

element.addEventListener('click', elClick, false);

function elClick(ev) {
// code here
let tgt = ev.target; // tgt is the element which was clicked
}

document.removeEventListener('click', elClick);

The event handler comes with an event object that contains information about the event.
Event objects have common properties as well as properties specific to the event type.
The tgt variable is the element that was clicked on, which in your case could be the div element or any child text node.
If you’re serious about js coding, you really need a good understanding of event listeners.

Hello Denis, Thank you for answering. I tried using the function named as a callback in the following below, but it didn’t work.

Blockquote window.addEventListener(“mouseup”, mouseup, false)

It seems the reason why it doesn’t work is that the mouseup function had to be defined before it was used in the line above.

Hi Paul,
It looks like you defined mousemove and mouseup functions before calling them and that’s why the code works. Before using anonymous functions to invoke mousemove and mouseup functions and pass arguments to them, I tried using named functions like the code in the following lines

window. addEventListener(“mouseup”, mouseup, false);
window. addEventListener(“mouseup”, mouseup, false);

For some reason the code doesn’t work when I declared the two mentioned functions after invoking them. My only question for you is why lifting doesn’t work in this case. Why do mousemove and mouseup functions need to be defined before invoking them in this situation?

In this updated example they are declared after the mousedown function and things still seem to work there. https://jsfiddle.net/qf51wuya/

It doesn’t seem like lifting is the cause of the problem there.

I feel like we’re going in circles.
I think it’s time to start over defining exactly what’s going on
First of all, if you plan to have the code in an external file, do it from the start.
I believe you have a container div and a child div that you want to be able to move with the mouse cursor with button 1 pressed.
My thoughts on how to achieve this:
Task 1 is to set the child div’s position bounds so that its border is never outside the parent div border. Put them in a const variable.
Task 2 When the mouse cursor enters the child div, do you want to reposition the mouse cursor to the center of the child div when the mouse button is pressed? That would definitely look better than having the child div jump to a different position like it does now.
Task 3 Prevent child div text from being selected.
Task 4 Move the child div to follow the mouse cursor while staying within the parent div.
What event listeners are required
parent div: mouseenter or mouseover
parent div: mouseleave
child div: mouse hover, mouse in, mouse leave, click, mouse click, mouse
When the mouse cursor enters the parent div, indicate the pending action by changing the border or background. And when he leaves go back.
When the cursor is on the child div, change the border and background as well.
When you click on the child div prevent text selection and force the cursor to the center of the child div
child div mousedown, change background and border force cursor to center.
mousemove child div:
calculate if border div is inside parent border and move child div if allowed
child div mouseup, redefine background and border.
Background colors and border changes can be done with CSS or class changes.
Add the event listeners to the parent div and the child div.