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

Add Heart-Beat animation #830

Merged
merged 9 commits into from
Jul 21, 2018
Merged

Add Heart-Beat animation #830

merged 9 commits into from
Jul 21, 2018

Conversation

jamesgeorge007
Copy link
Contributor

@jamesgeorge007 jamesgeorge007 commented Jul 14, 2018

This is in reference to the issue #829 which deals about adding a Heart-Beat animation.

Check out the demo here

  • Modified animate-config.json to include the heartBeat animation.
  • Add heartBeat.css to the source directory within attention-seekers.
  • Update Readme to include heartBeat class within the list of classes provided.

@jamesgeorge007 jamesgeorge007 changed the title Add HeartBeat animation Add Heart-Beat animation Jul 14, 2018
@phlegx
Copy link

phlegx commented Jul 16, 2018

Nice! Thank you!

@phlegx
Copy link

phlegx commented Jul 19, 2018

Can this PR please be merged in a new release?

@warengonzaga
Copy link
Member

We will review it first :) @eltonmesquita can you please have a look on this?

@eltonmesquita eltonmesquita requested a review from daneden July 20, 2018 23:29
@eltonmesquita
Copy link
Collaborator

Hey @daneden . I'd love to hear your opinion about this one ;D

@warengonzaga
Copy link
Member

@eltonmesquita and @daneden actually I am using this one with the latest feature :) I agree to add this one... but I am checking with the code if it will have a conflict or not following the way @daneden write animations.

@daneden
Copy link
Collaborator

daneden commented Jul 21, 2018

I like it! ❤️

We actually don't want to add the webkit rules to the source: we only need the unprefixed CSS.

@warengonzaga
Copy link
Member

@daneden will look in the code and fix it and submit another PR :) need to finish the feature first before that :) thanks for being active today @daneden :)

@eltonmesquita
Copy link
Collaborator

Cool @daneden ! @jamesgeorge007 , I'll happily merge this when you remove the -webkit parts.

@jamesgeorge007
Copy link
Contributor Author

@eltonmesquita Sure, Thank you.

@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jul 21, 2018

@eltonmesquita @daneden Removed -webkit prefixes from the file within source directory.

@warengonzaga
Copy link
Member

@jamesgeorge007 alright well done... @eltonmesquita please merge this one...

@@ -9,7 +9,8 @@
"swing": true,
"tada": true,
"wobble": true,
"jello": true
"jello": true,
"hearBeat": true
Copy link
Collaborator

Choose a reason for hiding this comment

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

We actually need to fix this typo first!

@jamesgeorge007
Copy link
Contributor Author

@daneden Typos fixed! 👍

@daneden
Copy link
Collaborator

daneden commented Jul 21, 2018

Awesome! I actually just noticed that animate.min.css hasn't been updated in this change: can you run gulp from the command line to make sure that's also updated?

@jamesgeorge007
Copy link
Contributor Author

jamesgeorge007 commented Jul 21, 2018

@daneden I'm not sure what lead to conflicts. I just installed the dependencies as listed within the package.json file and ran npm start from the terminal which triggered gulp adding heartBeat animation to the animate.min.css file.
What went wrong ?

@eltonmesquita
Copy link
Collaborator

@jamesgeorge007 your branch has just become outdated because of some merges that happened before. Just build and commit it and everything will be ok.

@daneden
Copy link
Collaborator

daneden commented Jul 21, 2018

This is adding package-lock.json and some changes to package.json too, but I also ran into those when I recently updated. I'm ok merging since we'll be coming up on a 3.7.0 release soon (probably this weekend!)

@daneden daneden merged commit 3098654 into animate-css:master Jul 21, 2018
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.

5 participants