phetsims / twixt

Animation library for interactive HTML5 graphics
MIT License
1 stars 3 forks source link

Animation.js chaining failures #9

Closed samreid closed 6 years ago

samreid commented 6 years ago

For example:

    /**
     * Stops the animation (or if waiting for the delay, will not "start" the animation).
     * @public
     *
     * @returns {Animation} - Returns the this reference, to support chaining.
     */
    stop: function() {
      // If we are not already animating, do nothing
      if ( !this.runningProperty.value ) {
        return; // <--- here
      }

      // Notifications
      this.runningProperty.value = false;
      this.stopEmitter.emit();
      this.endedEmitter.emit();

      return this;
    },

Chaining fails when it was not animating. There are similar problems in step. I haven't checked the other methods.

@jonathanolson OK to add return this for chaining in these cases? Or were they intentionally omitted for a good reason?

jonathanolson commented 6 years ago

Definitely was not intended.

jonathanolson commented 6 years ago

Should be fixed in the above commit, can you review?

samreid commented 6 years ago

Looks good, thanks!