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

Added new captureOnCallback option #66

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added new captureOnCallback option #66

wants to merge 3 commits into from

Conversation

jessegavin
Copy link

@jessegavin jessegavin commented Oct 4, 2016 •
edited
Loading

This pull request implements a solution to issue #11 .

I am not sure whether captureOnCallback is the best name, but callback was already taken.

If captureOnCallback is true, we use the onCallback callback to initiate screen capture (rather than checking if the document is ready).

I tested this feature using both phantom and slimer engines and it seems to work great.

I used the following url as the test page:
https://presentation-ngwjsmdjlf.now.sh

<html>
  <head></head>
  <body>
    <h1 id="heading">My Content!</h1>
    <script>
      var h1 = document.getElementById('heading');


      // When the page load event fires, update text and make it red

      window.addEventListener('load', function() {
        h1.innerText = 'onLoad!';
        h1.style.color = 'red';
      });

      function duration() {
        var d = null;
        var parts = location.search.split('?t=');
        if (parts.length > 0) {
          d = parseInt(parts[parts.length-1], 10);
        }
        return isNaN(d) ? 0 : d;
      }


      // When this timeout fires, update text, make it green,
      // then tell PhantomJS we are ready!

      setTimeout(function() {
        h1.innerText = 'page is ready!';
        h1.style.color = 'green';
        if (typeof window.callPhantom === 'function') {
          window.callPhantom({ hello: 'world' });
        }
      }, duration());
      </script>
  </body>
</html>

I am happy to make changes if needed.

@jessegavin
Copy link
Author

This PR could definitely help folks with single page apps / dynamic UIs as well. (If they're willing to add window.callPhantom() in their code). I think issue #62 may have benefitted from a feature like this.

@jessegavin
Copy link
Author

@vbauer Any thoughts on this Pull Request?

@vbauer
Copy link
Owner

vbauer commented Oct 13, 2016

@jessegavin Sorry, I'm on a business trip now. I would be able to take a look a little bit later..

@sdlaursen
Copy link

@vbauer , I really need this in my project, so if I can anyway throw in a vote for this, please consider this my vote:-)

@vbauer
Copy link
Owner

vbauer commented Dec 14, 2016

@jessegavin Could you please resolve conflicts (and I will accept it)?

@jessegavin
Copy link
Author

I just came back to this thread, I'll resolve conflicts now.

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

Successfully merging this pull request may close these issues.
3 participants
@jessegavin @vbauer @sdlaursen