Keyframes/jQuery.Keyframes

question about why "generate" uses .append(css)

augustc4 opened this issue · 4 comments

On line 67, why did you choose .append(css) instead of .html(css)?
I'm creating my definitions dynamically at point of use, and .append(css) causes these definitions to grow infinitely large. Switching to .html(css) fixes the problem, and seems more "dynamic".

I can create a PR with the same if you'd like.

append sucks, I totally agree. It's a slow function and should be improved upon.

The reason it's used there is because of this line, where multiple keyframes can be defined using an array. In that instance, append is correct. We need a solution that can work for both.

Thanks, I see it now: Adding multiple frame styles.

Continuing with my initial suggestion, switching to .html(css) seems non-breaking for that functionality. Meaning, the below call continues to work, creating two separate style blocks with the ids foo and bar:

define([{
  name: 'foo',
  from: {'transform': 'translate3d(0,0,0)'},
  to: {'transform': 'translate3d(0,10,0)'}
},
{
  name: 'bar',  
  from: {'transform': 'translate3d(0,0,0)'},
  to: {'transform': 'translate3d(0,20,0)'}
}]);

But it will fix the following call where foo is defined twice:

define([{
  name: 'foo',
  from: {'transform': 'translate3d(0,0,0)'},
  to: {'transform': 'translate3d(0,10,0)'}
},
{
  name: 'foo',  
  from: {'transform': 'translate3d(0,0,0)'},
  to: {'transform': 'translate3d(0,20,0)'}
}]);

current output with .append(css):

<style id="foo" class="keyframe-style" type="text/css">
@keyframes foo {from {transform:translate3d(0,0,0);}to {transform:translate3d(0,10,0);}}
@keyframes foo {from {transform:translate3d(0,0,0);}to {transform:translate3d(0,20,0);}}
</style>

vs. desired output with .html(css):

<style id="foo" class="keyframe-style" type="text/css">
@keyframes foo {from {transform:translate3d(0,0,0);}to {transform:translate3d(0,20,0);}}
</style>

Doh! Thanks for your effort on this, I'll gladly accept the pull request.

Thanks for having another look. Pull request is here: #50
Not sure how you're generating the .min file, so it's hasn't been included.