reveal() reset when using afterReveal and scrolling fast

[allaire]

How to duplicate: https://jsfiddle.net/9vcytj2b/

Scroll up and down, you'll see that the header text and the footer text keeps animating. reset is set to false by default so this should not happen. Using cleanup: true make it works, but is not friendly when you use sync().

It works as expected when you comment out the afterReveal callbacks: https://jsfiddle.net/jxcnb2wp/

The fact that it works correctly without callbacks leave me thinking that there is a bug somewhere in there.

[jlmakes]

Thanks for the report and reproduction @allaire

I'm comparing changes in element state, and focusing on { revealed: false } being set after the element has revealed. revealed = false occurs three times in the source, and only one looks guilty:

scrollreveal/src/instance/functions/initialize.js

Lines 7 to 21 in 66f44ab

	each(this.store.elements, element => {
	let styles = [element.styles.inline.generated]
	if (element.visible) {
	styles.push(element.styles.opacity.computed)
	styles.push(element.styles.transform.generated.final)
	element.revealed = true
	} else {
	styles.push(element.styles.opacity.generated)
	styles.push(element.styles.transform.generated.initial)
	element.revealed = false
	}
	element.node.setAttribute('style', styles.filter(s => s !== '').join(' '))
	})

Line 10 seems an insufficient condition, and the following else block is being evaluated. What's more, that logic suggests the initialize() function is erroneously running more than once.

Which I just confirmed: initialize() is being invoked once on initial evaluation, and two more times for each reveal() call wrapped in option.afterReveal

I'm going to keep digging...

[allaire]

Hi @jlmakes, awesome thanks for the detailed reply. Let me know if you need help to test a potential fix.

Also, while testing ScrollReveal, I also experienced this issue: #231

I know it's an old one, but I decided to reply instead of opening a new issue. I wasn't able to create a reproduction in a JSFiddle environment. Any advice how I can help with #231 ?

[simonkuran]

Any update on this? I'd like to be able to use the afterReveal option, but this bug is preventing me from implementing it.

[jlmakes]

@simonkuran I actually don't remember where I landed on this. Using option.afterReveal to invoke reveal() is not exactly a use case well suited to ScrollReveal's design. This is one of a few strategies I've seen surfaced by the community, reflecting a desire to have more control over what is revealed and when.

I'll take another look at this and see if there is something that can be improved for the current version of ScrollReveal. But for what it's worth, I am redesigning ScrollReveal 5 to offer more control to power users looking to coordinate their own reveals.