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

Manual Instantiation #11

Closed
wants to merge 1 commit into from
Closed

Manual Instantiation #11

wants to merge 1 commit into from

Conversation

mstaicu
Copy link

@mstaicu mstaicu commented Jan 27, 2014

After creating and inserting the HTML into the DOM, initialize the script with 'new window.scrollReveal()'

…and inserting the HTML into the DOM, initialize the script with 'new window.scrollReveal()'
@mstaicu
Copy link
Author

mstaicu commented Jan 27, 2014

My use case consisted in getting some data from a web service, binding it with a templating library and inserting it in the DOM. If I were to just insert the script at the bottom of the page, the initialization that is done at the bottom of the script would ignore all my newly inserted HTML i.e. nothing would happen, because scrollReveal.js expects the HTML to already be there.

@shprink
Copy link

shprink commented Jan 29, 2014

Yep that's a shame. We need a "reload" method that would register newly created elements otherwise we cannot dynamically insert DOM...
Good luck

@mstaicu
Copy link
Author

mstaicu commented Jan 29, 2014

The pull request that I did does just that. For example, after I insert a new batch of elements in the DOM, I recall the scrollReveal constructor, recreating a new object for each batch. I don't know if this is efficient but it works.

@jlmakes
Copy link
Owner

jlmakes commented Jan 29, 2014

Thanks for this @mstaicu and @shprink

I was reading about DOM Mutation Events, maybe something like this will help scrollReveal adapt to asynchronously HTML insertion; I would love it if, without manual effort or configuration, scrollReveal could handle this.

@mstaicu
Copy link
Author

mstaicu commented Feb 8, 2014

The pull request has not been merged so all I can do is share with you a copy of the library as it stands, modified by me. Use this code in a *.js file: https://gist.github.com/mstaicu/8889208/raw/929d15b0648c55b01b5f927196b50e874056b258/gistfile1.js and then call window.scrollReveal = new scrollReveal(); after you loaded up your HTML. Let me know if it works.

@jlmakes jlmakes closed this Feb 9, 2014
@jlmakes jlmakes reopened this Feb 9, 2014
@mstaicu
Copy link
Author

mstaicu commented Feb 9, 2014

Like I mentioned in my previous replay, I am +1 for using mutation events. I propose to not merge my changes yet, and run some tests using these events. I will do some tests of my own, since I already have the envrionment set up for asynchronous insertion and let you know how well do these events scale.

@jlmakes
Copy link
Owner

jlmakes commented Feb 10, 2014

It seems DOM Mutation Observers will replace DOM Mutation Events due to performance issues.

I’m sure it’s still worth exploring, but for the time being, I’m thinking manual instantiation and re-initialization is a fairly straight forward and reliable solution. Something like this issue (#14) would also benefit from this change.

What do you think?

@shprink
Copy link

shprink commented Feb 10, 2014

+1

@jlmakes jlmakes added bug and removed enhancement labels Feb 10, 2014
@mstaicu
Copy link
Author

mstaicu commented Feb 13, 2014

Sorry for the late replay, I didn't have time these days to focus on this project since I had several exams. I would like to revisit my approach one more time before giving you my go, is that ok?

jlmakes pushed a commit that referenced this pull request Feb 13, 2014
@jlmakes jlmakes closed this Feb 13, 2014
jlmakes pushed a commit that referenced this pull request Feb 13, 2014
@jlmakes
Copy link
Owner

jlmakes commented Feb 13, 2014

scrollReveal no longers instantiates automatically by including the script, you must now manually instantiate it:

  // window.sr = new scrollReveal()

If you modify the DOM, you can re-initialize like so:

  // sr.init()

This API is no longer supported.

@jlmakes jlmakes removed bug labels Apr 23, 2014
@jlmakes jlmakes changed the title Script doesn't work with asynchronous HTML insertion Manual Instantiation Oct 13, 2014
@jlmakes jlmakes added the Legacy label Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants