Skip to content

Conversation

@albertogasparin
Copy link

Add stableFrameDelay prop for simple list in order to allow items animation.
Closes #166

}

this.updateFrame();
if (type === 'simple' && stableFrameDelay) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to restrict this prop to the 'simple' list type. It seems like it could potentially be used for 'variable' or 'uniform' as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, based on my interpretation of the documentation, on variable you have to provide the height, so animating your elements should not interfere with it.
About uniform, I've tried to enable it on but it didn't work out of the box. So, if you think it might be useful I might give it a second try (but I might need to touch more code).

window.removeEventListener('resize', this.updateFrame);
this.scrollParent.removeEventListener('scroll', this.updateFrame, PASSIVE);
this.scrollParent.removeEventListener('mousewheel', NOOP, PASSIVE);
clearTimeout(this.updateFrameTimeoutId);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check if this.updateFrameTimeoutId exists first. I believe some JS interpreters throw when trying to clearTimeout on undefined.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the timeout is superfluous (see MDN note).

@xhinking
Copy link

xhinking commented Apr 6, 2017

Hey guys, any updates on this PR?

@caseywebdev
Copy link
Owner

I think we need to confirm which types this can work with.

@mirceapasoi
Copy link

Would a fix like this be needed for variable type ?

In order to allow items animation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants