HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

jQuery-Keyframes allows dynamic generation of CSS3 keyframes with callback events

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
allowseventswithkeyframescallbackjquerygenerationdynamiccss3

Problem

I want to learn best practices for structuring these kinds of small jQuery plugin libraries. I believe I am using a bad design pattern and what I was going for is fewest line numbers and minimal code.

Please advise on any better practices and any ways I can reduce the code down further.

```
(function() {
var animationSupport = false,
animationString = 'animation',
vendorPrefix = prefix = '',
domPrefixes = ['Webkit', 'Moz', 'O', 'ms', 'Khtml'];

$(window).load(function() {
var body = document.body;
if (body.style.animationName !== undefined) {
animationSupport = true;
}

if (animationSupport === false) {
for (var i = 0; i ").attr({
class: "keyframe-style",
id: id,
type: "text/css"
}).appendTo("head");
};

$.keyframe = {
getVendorPrefix: function() {
return vendorPrefix;
},
isSupported: function() {
return animationSupport;
},
generate: function(frameData) {
var frameName = frameData.name || "";
var css = "@" + vendorPrefix + "keyframes " + frameName + " {";

for (var key in frameData) {
if (key !== "name") {
css += key + " {";

for (var property in frameData[key]) {
css += property + ":" + frameData[key][property] + ";";
}

css += "}";
}
}

css = PrefixFree.prefixCSS(css + "}");

var $frameStyle = $("style#" + frameData.name);

if ($frameStyle.length > 0) {
$frameStyle.html(css);

var $elems = $("*").filter(function() {
this.style[animationString + "Name"] === frameName;
});

$elems.each(function() {
var $el, options;
$el = $(t

Solution

Interesting question, I would have pasted the whole file, it could have made for a more coherent review.

-
animationSupport is declared outside of $(window).load() even though it is only used in $(window).load(), I would declare it inside

var animationSupport = ( body.style.animationName !== undefined );


-
The same goes for domPrefixes

-
Once you have a boolean assigned to animationSupport, you can replace if( animationSupport === false ) with if(!animationSupport)

-
It is mildly funny that you declare this var body = document.body; as your sugar syntax. Since you only access body.style I would have gone for var style = document.body.style;

-
I also noted that you call the array with the vendor prefixes domPrefixes

Code Snippets

var animationSupport = ( body.style.animationName !== undefined );

Context

StackExchange Code Review Q#68541, answer score: 2

Revisions (0)

No revisions yet.