sociomantic-tsunami / zoidberg

CSS Animation API
MIT License
0 stars 1 forks source link

Create the base class #14

Closed jolene-borrelli-sociomantic closed 7 years ago

jolene-borrelli-sociomantic commented 7 years ago

Should be a base class with getters and setters from which the keyframe and rule class will be extended.

john-hannagan-sociomantic commented 7 years ago

What exactly is meant to be in the base class?

I would recommend generally choosing composition over inheritance unless you have a strong is-a relationship and want to use them polymorphically.

I think what you're saying is that we want a way to validate the css props we set on the keyframes, which is a good point, but is probably something that only the keyframe class needs (and could be separated out into another module and used by composition, as its pretty generic). The keyframe class has potentially lots of various properties we can set on it, because it should let us set the css properties.

The Animation class on the other hand only has a limited set of properties, and would be better represented as normal functions on the class ie:

Things like:

  animation-duration: 3s;
  animation-name: slidein;
  animation-iteration-count: 3;
  animation-direction: alternate;

Can be set like:

const anim = new Animation()
anim.setName( 'slidein' );
anim.setIterationCount( 3 );

// or, with ES Property getters/setters:
class Animation = {
  set iterationCount( count ) {
     if( count is not a number )
         throw new Error( "count should be a number" );

     this._iterationCount = count;
  }
}

anim.iterationCount = 5;
anim.iterationCount = "cats"; // throws error

But as mentioned in the other issue I think I prefer a functional API for the classes as its not obvious which properties have been override on the object and cause side effects/throw errors.

jolene-borrelli-sociomantic commented 7 years ago

I was thinking of having a base class with getters and setters that have whitelisted props that could be set or gotten. Does that stink?

jolene-borrelli-sociomantic commented 7 years ago

Yeah so, I guess this is stinkin' up the joint so as per @john-hannagan-sociomantic 's recommendations I will make this as part of a helpers/utils file instead.