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

use MutationObserver to track dom node additions. #767

Merged
merged 1 commit into from
Oct 31, 2023
Merged

use MutationObserver to track dom node additions. #767

merged 1 commit into from
Oct 31, 2023

Conversation

cancan101
Copy link
Contributor

@josteink
Copy link
Collaborator

josteink commented Oct 11, 2023 •
edited
Loading

Hey @cancan101 and thanks for the super-quick fix!

The change looks simple enough. I've also tested with my own extension, everything works, and from what I can tell there are no adverse effects.

I'd be happy to merge this change. How about about you @Kravimir?

@josteink josteink mentioned this pull request Oct 11, 2023
Open
@cancan101 cancan101 marked this pull request as ready for review October 11, 2023 11:32
@josteink
Copy link
Collaborator

No feedback from @Kravimir in roughly a week... So I say this is good to merge.

@josteink josteink mentioned this pull request Oct 17, 2023
Open
@Kravimir
Copy link

Not necessarily. In my case it often means I haven't remembered to come back to look at it.

However, in this case, the change is good, although I do note that the observed node is changed from "document" to "document.body".

@josteink
Copy link
Collaborator

Not necessarily. In my case it often means I haven't remembered to come back to look at it.

I know such things happen. It happens to me too!

But that often mean an issue isn't that critical to the person involved. In those cases, if others feel a need to move on, it should be OK to do so, especially if notifying about that happening 😄

However, in this case, the change is good, although I do note that the observed node is changed from "document" to "document.body".

Nice observation. Looking into docs here, I don't think this is going to be an issue for our use.

We're never going to need to inspect for changes in the <HEAD> element are we?

@cancan101
Copy link
Contributor Author

so, can this PR be merged?

@josteink
Copy link
Collaborator

Yeah. Sure thing. Sorry about the delay!

@josteink josteink merged commit ee4553b into KartikTalwar:master Oct 31, 2023
3 checks passed
@cancan101 cancan101 deleted the drep/user-mutationObserver-add branch December 11, 2023 01:19
@cancan101
Copy link
Contributor Author

Can we can a release cut with this PR? I think gmail.observe.on('view_thread')... may be broken without it. See: #765

@josteink
Copy link
Collaborator

Indeed. 1.1.12 pushed to npmjs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josteink josteink josteink approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Synchronous 'DOMNodeInserted' DOM Mutation Event is deprecated
3 participants
@cancan101 @josteink @Kravimir