TransitionNode fails if options.content is not provided

[pixelzoom]

Noted while working on https://github.com/phetsims/equality-explorer/issues/75, and related to #11.

TransitionNode:

33      // {Node|null} - Optionally may have initial content
34      content: null,

But content is apparently not optional. If you don't provide it, then your first attempted transition will fail in startTransition at line 225:

      this.fromContent.pickable = false;

Failure message is "Uncaught TypeError: Cannot set property 'pickable' of null".

[pixelzoom]

All of the current uses of new TransitionNode provide content, so it looks like the "optional" case was not tested.

There are a total of 3 places in startTransition where it will fail:

225 this.fromContent.pickable = false;
229 self.fromContent.pickable = null;
234 self.removeChild( self.fromContent );

[pixelzoom]

All of the transition methods are document content like this:

    /**
     * Start a transition to replace our content with the new content, using Transition.slideLeft.
     * @public
     *
     * @param {Node|null} content - If null, the current content will still animate out (with nothing replacing it).
     * @param {Object} options - Passed as options to the Animation. Usually a duration should be included.
     */
    slideLeftTo: function( content, options ) {
      this.startTransition( content, Transition.slideLeft( this.boundsProperty.value, this.fromContent, content, options ) );
    },

But if you provide null for content, then that's going to result in this.fromContent being null, because of this assignment in startTransition:

229 self.fromContent.pickable = null;

And the next time you attempt a transition, I'm betting it's going to fail for the same reason that it fails when you provide options.content: null.

[jonathanolson]

I believe this issue should be resolved by https://github.com/phetsims/twixt/commit/d18f35ac02977975200f85c797c32cd5733c1bfb, can you verify?

[pixelzoom]

Looks OK via inspection. But how did you test this? Do we need some examples or unit tests?

[jonathanolson]

I manually tested creation, but unit tests would probably be good. I'll add those.

[pixelzoom]

In TransitionNode startTransition:

      if ( this.fromContent ) {
        this.fromContent.pickable = false;
      }
      if ( this.toContent ) {
        this.toContent.pickable = false;
      }

So it's OK if they are both null? Why would someone do that? Or should there be a check in startTransition that at least one of them is non-null?

[jonathanolson]

So it's OK if they are both null? Why would someone do that? Or should there be a check in startTransition that at least one of them is non-null?

Being able to animate to null always sounds like a helpful property. Maybe it would be better to short-circuit a null-to-null transition and not actually animate at all?

[pixelzoom]

I agree that it's useful to animate to null if fromContent is defined. But I don't understand what a null-to-null transition is, or why it's useful.