Skip to content
This repository was archived by the owner on Dec 19, 2024. It is now read-only.

Remove deprecated neon animation, use CSS keyframe animations - #163

Open
valdrinkoshi wants to merge 17 commits into
masterfrom
no-neon-animation
Open

Remove deprecated neon animation, use CSS keyframe animations#163
valdrinkoshi wants to merge 17 commits into
masterfrom
no-neon-animation

Conversation

@valdrinkoshi

@valdrinkoshi valdrinkoshi commented Mar 13, 2018

Copy link
Copy Markdown
Member

Fixes #161, based off work started by @homerjonathan and @DudleyAH in #160

Remove the neon-animation dependency in favor of CSS keyframe animations. <paper-dialog> now ships 4 animations, fade-in-animation, fade-out-animation, scale-up-animation, scale-down-animation, and supports custom animations too.

We keep the animationConfig property and warn that they won't be affecting the animation.

We keep all the public methods previously inherited from neon-animation-runner-behavior, and make them no-op.

This PR adds tests and addresses the feedback provided in #160

@homerjonathan

Copy link
Copy Markdown

Thanks for completing this for me, I have had other duties! Looking good.

How about iron-dropdown etc. ?

@homerjonathan

Copy link
Copy Markdown

I have used cancel-animation in paper-tooltip.

      .cancel-animation {
        animation-delay: -30s !important;
      }

If someone runs this function. I think that they expect it to stop animating and return to the fully shown or fully hidden. Not sure if you want this function in this component.

      /**
       * Cancels the animation and either fully shows or fully hides tooltip
       */
      cancelAnimation: function () {
        // Short-cut and cancel all animations and hide
        this.$.tooltip.classList.add('cancel-animation');
      },

@valdrinkoshi

Copy link
Copy Markdown
Member Author

By removing the animation class, the component should go to the opened or closed state automatically, e.g. see this smaller repro http://jsbin.com/nehacek/1/edit?html,output
If you toggle the class that causes animation before the animation is done, the element goes to its original state (opacity: 1)

@valdrinkoshi

Copy link
Copy Markdown
Member Author

Re iron-dropdown: that's the next one in line, let's use PolymerElements/iron-dropdown#147 as a tracking bug for that

@homerjonathan

Copy link
Copy Markdown

The cancelAnimation in paperTooltip was slightly different, the request was if it was called that whatever was left of the animation was to complete and it to be at the end of the state. Hence the -30s timing indicator. So the animation ran to the end. You can try out the final demo on paper-tooltip for an example.

@valdrinkoshi

Copy link
Copy Markdown
Member Author

We can use the css class to start & cancel animations, eg see http://jsbin.com/ladexiz/1/edit?html,output
Notice that after canceling the animation we still get the element to correctly render.

@homerjonathan

Copy link
Copy Markdown

I believe if I can remember the issue was that the paper-tooltip could be cancelled each way. Fade in and fade out with a opacity that was set by a mixin --paper-tooltip-opacity. You couldn't jump to the end of the animation with its final state as it is set by the animation. I think since the paper-tooltip needs the opacity value is why I did the -30s trick.

@valdrinkoshi

Copy link
Copy Markdown
Member Author

I see. I'd be curious to see how cancelAnimation is used in paper-dialog tho. If it's used only to disable animations during tests, that can be done as well by setting dialog.style.animationDuration = '0ms' now, instead of dialog.cancelAnimation(). I agree that if the animation gets canceled, the dialog should render its final opened/closed state, which would mean in this case set display: none when closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants