Skip to content

Conversation

@jemunk
Copy link

@jemunk jemunk commented Nov 2, 2023

Description

Fixed unhandled exception happening in _mouseMove handler on line 209, 'this.el' is undefined. Happens if element is removed just before call of _mouseMove and just after _mouseDown.

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing (yarn test)
  • Extended the README / documentation, if necessary

I am unable to run yarn test. Gets all kinds of strange errors.

I had a setup easily reproducing the issue. After the fix, I could not reproduce it anymore.

…, 'this.el' is undefined. Happens if element is removed just before call of _mouseMove and just after _mouseDown.
@jemunk jemunk changed the title Fixed unhandled exception happening in _mouseMove Fixed unhandled exception happening in _mouseMove handler Nov 2, 2023
if (this.dragTimeout) window.clearTimeout(this.dragTimeout);
delete this.dragTimeout;
if (this.dragging) this._mouseUp(this.mouseDownEvent);
if (this.mouseDownEvent) this._mouseUp(this.mouseDownEvent);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems ok, but I would really like to know why would you remove the element after mouse down, but before a move+up behavior ? Also I need to look at why we only wanted to call up if we moved >3 pixels instead of always...

Copy link
Author

@jemunk jemunk Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I my case it is being removed from a configuration. A DB change makes the element remove.
So if a user starts dragging in that moment, it is possible for it being removed just between the press and move handlers.
But the _mouseUp method seems to take fully care of all cleaning up. So it is just a matter of the destroy method doing it right.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looked at code and you are correct. _mouseUp() handles this.dragging separately so it should ways be called if down. thanks.

@adumesny adumesny merged commit e240db8 into gridstack:master Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants