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

Run the animations when the page renders #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joaopms
Copy link

@joaopms joaopms commented Feb 11, 2021

This allows the animations to be properly initialised without scrolling the page.

@vycoder
Copy link
Owner

vycoder commented Feb 11, 2021

Hi I'm sorry but can you further elaborate what's the issue this PR is trying to solve? What was the use case or scenario that you encountered?

Thanks!

@joaopms
Copy link
Author

joaopms commented Feb 12, 2021

I'm sorry, my bad.

Please watch the following short videos, where I reload the page and then start scrolling:

The first section "Air for Sound Effects" doesn't animate until you start scrolling (even though it's visible). This patch fixes that while keeping the other scrolling animations intact.


I want to take a moment to thank you for this awesome library and for replying so quickly - thank you so much!

@joaopms
Copy link
Author

joaopms commented Feb 19, 2021

Hi @vycoder, any thoughts on my latest comment? Thanks!

@vycoder
Copy link
Owner

vycoder commented Feb 19, 2021

Hi @joaopms apologies, was quite busy the past few weeks. I'm a bit hesitant to merge it, the fix seems like a workaround, by no fault of your solution. It seems like a symptom of a different issue, maybe this directive's approach might be wrong or something.

I'm wondering whether it's something we should fix within the directive. The purpose of this directive is to animate elements as it scrolls into view not on appear. If that make sense? Animating an element as it appears is built-in on Vue Transition.

When I created this directive, the use-case I had in mind, is to animate things that are placed (one screenfull) below the page as they scroll into view. Not much animate elements on render, since I could just use <transition> for that.

You could also do something like:

<transtion appear appear-class="animated flip">
   <div v-animate-on-scroll="{up: 'animated flip'}" />
</transition>

@joaopms
Copy link
Author

joaopms commented Feb 19, 2021

No worries, I really appreciate the time you've dedicated to this library and this issue.

When I created this directive, the use-case I had in mind, is to animate things that are placed (one screenfull) below the page as they scroll into view. Not much animate elements on render, since I could just use <transition> for that.

The key here is that your use-case only expects to animate elements that appear after scrolling, which is totally acceptable and really useful in a lot of occasions.

My use-case is a responsive page that contains a header that is animated as soon as it is rendered. That header might cause the following elements to be placed below the page or it might not, since it only occupies the space it needs.
This means those elements can always be animated on scroll when the user is on a phone, for example, but some of them might need to be animated on render if the user is on a tablet or desktop, meaning I can't really use your proposed solution - I do, however, really appreciate it.

If you feel like your library shouldn't cover this issue, I totally understand - after all, the library does animate elements as they scroll into view. :)

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.

None yet

2 participants