search

KartikTalwar / gmail.js

Gmail JavaScript API
MIT License
3.74k stars 456 forks source link

use MutationObserver to track dom node additions. #767

Closed cancan101 closed 8 months ago

cancan101 commented 9 months ago

Closes https://github.com/KartikTalwar/gmail.js/issues/766

CC @Kravimir @josteink

josteink commented 9 months ago

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 commented 9 months ago

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

Kravimir commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

so, can this PR be merged?

josteink commented 8 months ago

Yeah. Sure thing. Sorry about the delay!

cancan101 commented 7 months ago

Can we can a release cut with this PR? I think gmail.observe.on('view_thread')... may be broken without it. See: https://github.com/KartikTalwar/gmail.js/issues/765

josteink commented 7 months ago

Indeed. 1.1.12 pushed to npmjs!