Removing Deprecated Neon-Animation Dependency

[homerjonathan]

Another removal of Neon-Animation Dependency.

Copying much of the code design from paper-tooltip this one was much easier. I have had a look and cannot see any timer configurations so I think that this should not be much of a break fix for most users.

I have also solved an issue when the animation selected is not known.

    _isSetAnimationValid: function () {
      // If CSS Class does not have animation-name considered not be an animation
      var style = window.getComputedStyle(this);
      return (style.getPropertyValue("animation-name") === "none")
    }

This seems the least processor intensive way to find out if the animation exists. If animation does not exist it is simply ignored. Whereas if you call an animation that does not exist the event on animation end is not fired leaving the dialog unable to open or close.

[homerjonathan]

Any updates on this PR?

I have started to work on iron-dropdown and want to know if I am going in the right direction.

[valdrinkoshi]

Hi @homerjonathan, thanks for the contribution.
This looks like a breaking change as it would not support any custom animations some users might rely on for their paper-dialogs.
As per our Contributing guidelines, could you file an issue where we can discuss the issue & design potential solutions?

[valdrinkoshi]

This is great stuff!
I think we'll need to add some unit tests for stuff like:

• should not animate by default
• should remove the animation classes when done animating
• should handle only its own animations

[valdrinkoshi]

nit: can you list the animation explicitly here?

Included in the component are:
- fade-in-animation
- fade-out-animation
- scale-up-animation
- scale-down-animation

[valdrinkoshi]

I'd avoid exposing new css custom properties/mixins for now, as the user can decide to redefine all of this by doing:

<style>
  .fade-in-animation {
   /* my overrides */
   animation-duration: 1s;
  }
  <paper-dialog entry-animation="fade-in-animation">

[valdrinkoshi]

I'd remove this for now (same reasoning as above)

[valdrinkoshi]

I'd remove this for now (same reasoning as above), also the default value should be 500ms as for the other ones.

[valdrinkoshi]

I'd remove this for now (same reasoning as above)

[valdrinkoshi]

can remove

[valdrinkoshi]

animationend event bubbles, so this method will be invoked if paper-dialog contains animating content (e.g. a spinner). To repro, try nesting 2 paper-dialogs, and observe that this method gets called twice when opening the inner dialog.

We need to handle the case here. Also, we need to always remove the classes before calling finishRenderOpened/Closed:

_onAnimationEnd: function (event) {
  if (event.target !== this) {
    return;
  }
  // Removes animation classes.
  this.cancelAnimation();
  if (!this.opened) {
    this._finishRenderClosed();
  } else {
    this._finishRenderOpened();
  }
},

[valdrinkoshi]

IronOverlayBehavior already takes care of setting display: none to the overlay in _finishRenderClosed, so we don't have to worry about this here 👌

[valdrinkoshi]

nit: ' instead of " here :)

[valdrinkoshi]

It would be worth to check for animation-duration in here too, e.g. if it is 0 then we don't have to wait for animationend. WDYT?

[valdrinkoshi]

(missed this) we should remove it from the 1.x variant too (line 42)

[homerjonathan]

All changes seem reasonable. However I am in the UK.....we are now in sleep mode! ;-)

[valdrinkoshi]

SG, thanks for looking into this again @homerjonathan :)

Ps: I played with the styles a bit, they could be reduced to half by sharing setup, e.g.

:host {
  animation-duration: 500ms;
  animation-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
}

:host(.fade-in-animation) {
  animation-name: fadeInAnimation;
}

@keyframes fadeInAnimation {
  from {
    opacity: 0;
  }
}

:host(.fade-out-animation) {
  animation-name: fadeOutAnimation;
}

@keyframes fadeOutAnimation {
  to {
    opacity: 0;
  }
}

:host(.scale-up-animation) {
  animation-name: scaleUpAnimation;
}

@keyframes scaleUpAnimation {
  from {
    transform: scale(0.0);
  }
}

:host(.scale-down-animation) {
  animation-name: scaleDownAnimation;
}

@keyframes scaleDownAnimation {
  to {
    transform: scale(0.0);
  }
}