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

Implement waterfall animation #35

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Implement waterfall animation #35

wants to merge 6 commits into from

Conversation

yehee
Copy link
Contributor

@yehee yehee commented Oct 6, 2020 •
edited
Loading

👷 Changes

Implement waterfall animation using Lottie library

💭 Notes

  • No animation on mobile/tablet view (just yet)
  • Using SVG file instead of PNG for Events background events__background.svg

🔦 Testing Instructions

Amplify on web and mobile!

rmehri01
rmehri01 reviewed Oct 11, 2020
View reviewed changes
Copy link
Contributor

@rmehri01 rmehri01 left a comment

Choose a reason for hiding this comment

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

Looks pretty good, should it be on mobile too?

@yehee yehee force-pushed the waterfall branch 3 times, most recently from 882c547 to 52e2321 Compare October 11, 2020 19:31
@yehee yehee marked this pull request as ready for review October 11, 2020 19:35
@yehee
Copy link
Contributor Author

yehee commented Oct 11, 2020

Looks pretty good, should it be on mobile too?

Sorry for the confusion, the change should be there! Let me know if you have any comments

@kozr
Copy link
Contributor

kozr commented Oct 15, 2020

Can you export the boat at 1.5x the resolution? Right now there's a difference between the two bears here
image

rmehri01
rmehri01 reviewed Oct 15, 2020
View reviewed changes
Copy link
Contributor

@rmehri01 rmehri01 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think maybe its a bit low? Also the animation doesn't loop infinitely, theres like a cut but I asked design about it.

image

@yehee
Copy link
Contributor Author

yehee commented Oct 15, 2020

Can you export the boat at 1.5x the resolution? Right now there's a difference between the two bears here
image

Thanks for pointing that out! I replaced it with SVG file, would that be an issue?

kozr
kozr approved these changes Oct 16, 2020
View reviewed changes
Copy link
Contributor

@kozr kozr left a comment

Choose a reason for hiding this comment

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

Preferably 1.5x png but svg will do. Let's get this merged so we can deploy it this weekend.

@rmehri01
Copy link
Contributor

Are we not worrying about the positioning and infinite looping for now then?

@kozr
Copy link
Contributor

kozr commented Oct 16, 2020

What did design say?

@rmehri01
Copy link
Contributor

What did design say?

Sandy said it was a bit low and no response about the looping thing yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmehri01 rmehri01 rmehri01 left review comments

@kozr kozr kozr approved these changes

At least 1 approving review is required to merge this pull request.

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
@yehee @kozr @rmehri01