Functions Stop Working After Refactoring Code

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

I refactored this code to make it more readable by taking the mousemove and mouseup functions out of the callback function of the mousedown eventlistener and placed them elsewhere as can be seen in the Refactored_Code. Eventually I will put them in an external file. The problem now is that after the dragging has stopped and the mouseup event was triggered, the mousemove eventlistener doesn’t seem to be removed by the mouseup callback function and stop the DIVs from moving

The mouseup event listener is an anonymous function so there is no way that it can be passed to the removeEventListener function.
removeEventListener only works on named functions.

Hi Dennis,
Thanks for your reply. Is there a work around for this?

First bind the event listeners to the element that is being moved and 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 defined where the event listeners can access them.

Hi Dennis,
Thanks for your reply. Your code below does not allow the passing of the arguments “e” and “draggable” to the callback functions.

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

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

You said at the start:

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

Instead of going backwards, I’m going to start with working code (always a bonus) and attempt to use more proper techniques to achieve the desired outcome instead.

Extracting the mousemove() function out of the mousedown event listener, there is trouble where it says: Uncaught ReferenceError: draggable is not defined

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

In this case I’ve 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;

That way we can update draggable from the 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/

1 Like

Hi Paul,
Thanks for your response. I think that you have not answered the question I had fully. In your updated code you only extracted the mousemove function but not the mouseup function. I would like to extract the mouseup function as well.

Also I think that the extracted mousemove function in my refactoredCode was not the problem because it does work 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 at a time :slight_smile:

The mouseup, mousedown, and mousemove functions have been extracted in this updated code.
No other changes of significance needed to occur. https://jsfiddle.net/wz4aqke2/

Suggestion: When a dragged item runs up against a wall, such as against the left wall, it would be less distracting if instead of freezing in place, that dragged item moved up and down along the wall to the same vertical position as the mouse.

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

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

This means the mousemove and mouseup functions are defined before they are used. To be honest I never thought that it would cause problems since Javascript allows the use of functions before defining them.

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

Look at the above link for event listener adding and removing.
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 is supplied with an event object which contains information of the event.
Event objects have common as well as properties that are unique to the event type.
The tgt variable is the element which was clicked which in your case could be the div element or any child text node.
If you are serious about coding in js you really need a good understanding of event listeners.

1 Like

Hi Dennis, Thanks for responding. I tried to use named function as the callback in the following below but it did not work.

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

It looks like the reason it doen’t work is because the mouseup function needed to be defined prior to it being used in the line above.

Hi Paul,
It appears you defined the mousemove and mouseup functions before invoking them and that’s why the code is working. Prior to using anonymous functions to invoke the mousemove and mouseup functions and pass arguments to them, I did try to use named functions like the code in the following lines

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

For whatever reason the code does not work when I declared the two mentioned functions after their invocation. My only question to you is why isn’t hoisting working in this case. Why do the 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 that hoisting is the cause of the problem there.

I feel we are going round in circles.
I think it is time to start again by defining exactly what happens
First if you envision having the code in an external file, then do it right at the start.
I believe that you have a container div and a child div that you want to be able to move with the mouse cursor with button 1 down.
My thoughts on how to achieve this:
Task 1 is to set the position limits of the child div so that its border is never outside the parent div border. Put them into a const variable.
Task 2 When the mouse cursor enters the child div do you want to reposition the mouse cursor to the centre of the child div when the mouse button is pressed down? This would certainly look better than having the child div jump to a different position as at present.
Task 3 Prevent the child div text from being selected.
Task 4 Move the child div to track 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 over, mouse enter mouse leave, click, mouse down, mouse
When the mouse cursor enters the parent div indicate pending action by changing border or background. And when it leave revert back.
When the cursor is over the child div change border and background too.
When click on the child div, prevent the text selection and force the cursor to the centre of the child div
child div mousedown, change the background and border force cursor to centre.
child div mousemove:
calculate if the div border is within the parent border and move child div if permitted
child div mouseup, change back the background and border.
Background colours and border changes can be done with css or class changes.
Add the event listeners to the parent div and the child div.

1 Like