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.

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

Use mutation observer #11

Merged
merged 6 commits into from Mar 31, 2022
Merged

Conversation

Mash19
Copy link
Contributor

@Mash19 Mash19 commented Mar 28, 2022

Replacing deprecated Mutation Events with mutationObserver.
Had to move some code around so every instance did not create a new observer.
Mainly the event functions
Cleanup code now part of mutationObserver's callback.

Michael Ash added 2 commits March 27, 2022 17:53
create single instance
refactor to remove references to rover
Move event functions to higher scope
Checks to make reference target nodes
Copy link
Owner

@argyleink argyleink left a comment

Choose a reason for hiding this comment

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

overall i appreciate it!

i was able to get pretty far with it here https://codepen.io/argyleink/pen/rNpmyXL.

i made some suggestions where i needed to make code changes during testing. really wishin i had good tests for this lib right now! (see #10). making a codepen was the best i had for testing creating multiple instances and deleted them in different ways.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
check for target node as
 a descendant of deleted node
index.js Outdated Show resolved Hide resolved
Copy link
Owner

@argyleink argyleink left a comment

Choose a reason for hiding this comment

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

sorry for lots of comments, it's not a bad sign!

updated the codepen with your latest and gave it a spin (any mods i make i made comments about today, like isRtl).

can confirm it disconnects when there's no more roving elements 👍🏻
major stuff is done, just ironing out the rest. hope this feedback is coming through constructive and positive 🙂

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
Copy link
Owner

@argyleink argyleink left a comment

Choose a reason for hiding this comment

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

looks and works great!

@argyleink argyleink merged commit f9dcbd7 into argyleink:main Mar 31, 2022
@Mash19 Mash19 deleted the use-mutation-observer branch May 25, 2022 05:18
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.

None yet

2 participants