Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unregister element #2

Open
dpikt opened this issue Oct 19, 2020 · 3 comments · May be fixed by #13
Open

Unregister element #2

dpikt opened this issue Oct 19, 2020 · 3 comments · May be fixed by #13
Labels
enhancement New feature or request

Comments

@dpikt
Copy link

dpikt commented Oct 19, 2020

If an element no longer needs to be dragged, could it be "unregistered" and removed from the callbacks array? This would help save memory especially in single page apps where several views could potentially be shown or rerendered.

const instance = dragmove(...)
instance.unregister()

// or

const id = dragmove(...)
enddragmove(id)
@knadh knadh added the enhancement New feature or request label Oct 20, 2020
@knadh
Copy link
Owner

knadh commented Oct 20, 2020

Ah yes, I'd forgotten about this. Will add an API to detach the event listener.

@obrejla
Copy link

obrejla commented Apr 9, 2024

Hi @knadh , not removing elements from callbacks array is a really big memory leak and shouldn't be marked as enhancement but rather as a bug, do you plan to fix it? Thanks a lot.

knadh added a commit that referenced this issue Apr 13, 2024
@knadh
Rewrite and refactor the entire script.
70483c1 
- Fully refactor the event handling approach and rewite the script to
  just have 3 global listeners instead of the earlier approach of having 2 events per item.
- Maintain items in a `WeakMap()` instead of a maintaing states in a function per object.
- Simplify logic for readability.
- Add `remove()` to allow the drag/move events on an object to be unregistered.

The new approach should is significantly more memory efficient.

Closes #2.
@knadh knadh linked a pull request Apr 13, 2024 that will close this issue
Open
@knadh
Copy link
Owner

knadh commented Apr 13, 2024

@obrejla I ended up rewriting the whole script to be more memory efficient. It also now lets you unregister elements.

You can find it here: #13

Example:

const d = dragmove(document.querySelector("#test"), document.querySelector("#test .handle"), onStart, onEnd);

d.remove();

Even if you don't remove, the new version keeps track of DOM objects using a WeakMap() that'll automatically garbage collected deleted DOM elements automatically from the map. In addition, functions also no longer leak.

There are no breaking changes. Please give the branch a shot and I'll merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rewrite and refactor the entire script. knadh/dragmove.js
3 participants
@obrejla @knadh @dpikt