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

Simplify injecting an element #2

Open
sindresorhus opened this issue Feb 18, 2016 · 5 comments
Open

Simplify injecting an element #2

sindresorhus opened this issue Feb 18, 2016 · 5 comments

Comments

@sindresorhus
Copy link

From refined-github/refined-github#12 (comment).

I'm thinking maybe a method that accepts a CSS selector and an element/string and injects on load and when there's a mutation, but also checks that it doesn't already exist, which can happen when you click the back button in the browser because of caching.

@josephfrazier
Copy link
Member

I just ran into this problem, and solved it by using the following wrapper, which just inserts a deterministically unique element representing the entire callback function, and doesn't run the callback if the element is already present (that is, when the back button is pressed):

import injection from 'github-injection';
import md5hex from 'md5-hex';
import domify from 'domify';

export default function (global, cb) {
  const singletonId = md5hex(cb.toString());
  injection(global, () => {
    if (global.document.getElementById(singletonId)) {
      return;
    }
    global.document.body.appendChild(domify(`<div id="${singletonId}" />`));
    cb();
  });
}

@stefanbuck, would you be interested in adding this functionality as an option? We should be able to adjust it to avoid the md5-hex and domify dependencies, if that's a concern.

@stefanbuck
Copy link
Member

Smart workaround, but (I'm sorry) I like @sindresorhus suggestion more. Mainly, because his suggestion is more flexible than yours. Imagine you need to spy for changes on two different container on the same page. This won't work with your solution.

@stefanbuck
Copy link
Member

@josephfrazier did you spot this issue while working on OctoLinker? I'm asking, because there we're only interested in dom changes related to #js-repo-pjax-container. OctoLinker doesn't need to append any element to the dom at this stage.

@josephfrazier
Copy link
Member

@stefanbuck I was working on a different extension when I hit this issue. After thinking about it a bit more, I think I see what you mean about my solution not being as flexible. It looks like it wouldn't work when the page is updated but is not replaced entirely (like the "back button" does).

@Mottie
Copy link

Mottie commented Jul 23, 2017

I don't know if it'll help, but I wrote up a wiki page on how to detect all the different dynamically loaded content on GitHub. There are probably a few more that I'm missing.

  • Main content (pjax) containers
  • Progressive containers
    • Diff files
    • Discussion blocks
    • Previews

I ended up created a global mutation observer script to watch for specific changes. I would appreciate any feedback! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants