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

Removing Deprecated Neon-Animation Dependency #160

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,42 @@ this element.

### Animations

Set the `entry-animation` and/or `exit-animation` attributes to add an animation when the dialog
is opened or closed. See the documentation in
[PolymerElements/neon-animation](https://github.com/PolymerElements/neon-animation) for more info.
Animations used to use the neon-animation, however the library has since been @deprecated.

For example:
You can still use the set the `entry-animation` and/or `exit-animation` attributes to add an animation when the dialog is opened or closed. You can also create your own animation class.

For example the below the CSS is first declared (this one slides from off screen). You then point to the animation by selecting the class updating the `entry-animation`. Note that the
`exit-animation` is fade-out-animation which fades when the dialog is closed.
```html
<link rel="import" href="../neon-animation/web-animations.html">
<link rel="import" href="../neon-animation/animations/scale-up-animation.html">
<link rel="import" href="../neon-animation/animations/fade-out-animation.html">

<paper-dialog entry-animation="scale-up-animation"
<style>
@keyframes keyFrameSlideDownIn {
0% {
transform: translateY(-2000px);
opacity: 0;
}
10% {
opacity: 0.2;
}
100% {
transform: translateY(0);
opacity: 1;
}
}

.my-animation {
transform: translateY(-2000px);
opacity: 0;
animation-delay: 0;
animation-name: keyFrameSlideDownIn;
animation-iteration-count: 1;
animation-timing-function: cubic-bezier(0.0, 0.0, 0.2, 1);
animation-duration: 500ms;
animation-fill-mode: forwards;
}
</style>

<paper-dialog entry-animation="my-animation"
exit-animation="fade-out-animation">
<h2>Header</h2>
<div>Dialog body</div>
Expand Down
1 change: 0 additions & 1 deletion bower.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"homepage": "https://github.com/PolymerElements/paper-dialog",
"ignore": [],
"dependencies": {
"neon-animation": "PolymerElements/neon-animation#1 - 2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"paper-dialog-behavior": "PolymerElements/paper-dialog-behavior#1 - 2",
"iron-overlay-behavior": "PolymerElements/iron-overlay-behavior#1 - 2",
"polymer": "Polymer/polymer#1.9 - 2"
Expand Down
2 changes: 0 additions & 2 deletions demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
<link rel="import" href="../paper-dialog.html">
<link rel="import" href="../../paper-button/paper-button.html">
<link rel="import" href="../../paper-dialog-scrollable/paper-dialog-scrollable.html">
<link rel="import" href="../../neon-animation/web-animations.html">
<link rel="import" href="../../neon-animation/neon-animations.html">
<link rel="import" href="../../paper-dropdown-menu/paper-dropdown-menu-light.html">
<link rel="import" href="../../paper-listbox/paper-listbox.html">
<link rel="import" href="../../paper-item/paper-item.html">
Expand Down
245 changes: 201 additions & 44 deletions paper-dialog.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
-->

<link rel="import" href="../polymer/polymer.html">
<link rel="import" href="../neon-animation/neon-animation-runner-behavior.html">
<link rel="import" href="../paper-dialog-behavior/paper-dialog-behavior.html">
<link rel="import" href="../paper-dialog-behavior/paper-dialog-shared-styles.html">
<!--
Expand Down Expand Up @@ -43,19 +42,11 @@ <h2>Header</h2>
### Animations

Set the `entry-animation` and/or `exit-animation` attributes to add an animation when the dialog
is opened or closed. See the documentation in
[PolymerElements/neon-animation](https://github.com/PolymerElements/neon-animation) for more info.
is opened or closed. Included in the component are fade-in-animation (and out), scale-up-animation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


For example:

<link rel="import" href="components/neon-animation/animations/scale-up-animation.html">
<link rel="import" href="components/neon-animation/animations/fade-out-animation.html">

<paper-dialog entry-animation="scale-up-animation"
exit-animation="fade-out-animation">
<h2>Header</h2>
<div>Dialog body</div>
</paper-dialog>
@deprecated This animation was based on the deprecated Neon-Animation component. It now
uses CSS Web Animations. This change reduces code size, and uses the platform. Any CSS3 animation
class can be used.

### Accessibility

Expand All @@ -70,40 +61,206 @@ <h2>Header</h2>

<dom-module id="paper-dialog">
<template>
<style include="paper-dialog-shared-styles"></style>
<style include="paper-dialog-shared-styles">
@keyframes keyFrameScaleUp {
0% {
transform: scale(0.0);
}
100% {
transform: scale(1.0);
}
}

@keyframes keyFrameFadeInOpacity {
0% {
opacity: 0;
}
100% {
opacity: 1;
}
}

@keyframes keyFrameFadeOutOpacity {
0% {
opacity: 1;
}
100% {
opacity: 0;
}
}

@keyframes keyFrameScaleDown {
0% {
transform: scale(1.0);
}
100% {
transform: scale(0.0);
}
}

:host(.fade-in-animation) {
opacity: 0;
animation-delay: 0;
animation-name: keyFrameFadeInOpacity;
animation-iteration-count: 1;
animation-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
animation-duration: var(--paper-dialog-duration-in, 500ms);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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">

animation-fill-mode: forwards;
@apply --paper-dialog-animation;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

:host(.fade-out-animation) {
opacity: 1;
animation-delay: 0;
animation-name: keyFrameFadeOutOpacity;
animation-iteration-count: 1;
animation-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
animation-duration: var(--paper-dialog-duration-out, 1500ms);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

animation-fill-mode: forwards;
@apply --paper-dialog-animation;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

:host(.scale-up-animation) {
transform: scale(0);
opacity: 1;
animation-delay: 0;
animation-name: keyFrameScaleUp;
animation-iteration-count: 1;
animation-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
animation-duration: var(--paper-dialog-duration-in, 500ms);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

animation-fill-mode: forwards;
@apply --paper-dialog-animation;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

.scale-down-animation {
transform: scale(1);
opacity: var(--paper-dialog-opacity, 0.9);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this for now (same reasoning as above), and set it to 1

animation-delay: 0;
animation-name: keyFrameScaleDown;
animation-iteration-count: 1;
animation-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
animation-duration: var(--paper-dialog-duration-out, 500ms);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

animation-fill-mode: forwards;
@apply --paper-dialog-animation;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

</style>
<slot></slot>
</template>
</dom-module>

<script>
Polymer({
is: 'paper-dialog',

behaviors: [
Polymer.PaperDialogBehavior,
Polymer.NeonAnimationRunnerBehavior
],

listeners: {
'neon-animation-finish': '_onNeonAnimationFinish'
},

_renderOpened: function() {
this.cancelAnimation();
this.playAnimation('entry');
},

_renderClosed: function() {
this.cancelAnimation();
this.playAnimation('exit');
},

_onNeonAnimationFinish: function() {
if (this.opened) {
this._finishRenderOpened();
} else {
this._finishRenderClosed();
Polymer({
is: 'paper-dialog',

behaviors: [
Polymer.PaperDialogBehavior
],

listeners: {
'webkitAnimationEnd': '_onAnimationEnd',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be animationend?

},

properties: {
/**
* Set Entry Animation
*/
entryAnimation: {
value: "fade-in-animation",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would remove this default value, as it forces animations for all paper-dialogs

type: String
},
/**
* Set Exit Animation
*/
exitAnimation: {
type: String
},
_showing: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be required, as it's enough to use this.opened (defined in IronOverlayBehavior which is inherited by PaperDialogBehavior)

type: Boolean,
value: false
}
},

/**
* @return {void}
*/
attached: function () {
this._addListeners();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't require to add/remove listeners on attached/detached: when a node is detached, it won't fire animationend events anyway.
It should be fine to remove attached, detached, _addListeners, _removeListeners

},

/**
* @return {void}
*/
detached: function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove

this._removeListeners();
},

_renderOpened: function () {
this._showing = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove (we can use this.opened instead)

this.cancelAnimation();
this._entryAnimation();
},

_renderClosed: function () {
this._showing = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove (we can use this.opened instead)

this.cancelAnimation();
this._exitAnimation();
},

_addListeners: function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove

this.listen(this, 'animationend', '_onAnimationEnd');
},

_removeListeners: function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove

this.unlisten(this, 'animationend', '_onAnimationEnd');
},

cancelAnimation: function () {
// Short-cut and cancel all animations and hide
this.classList.remove(this.entryAnimation);
this.classList.remove(this.exitAnimation);
},

_entryAnimation: function () {
if (!this.entryAnimation) {
this._onAnimationEnd();
} else {
this.classList.add(this.entryAnimation);
// If not animation _onAnimationEnd will not be called and will fail to change state
if (this._isSetAnimationValid()) {
this._onAnimationEnd();
}

}
},

_exitAnimation: function () {
if (!this.exitAnimation) {
this._onAnimationEnd();
} else {
this.classList.add(this.exitAnimation);
// If not animation _onAnimationEnd will not be called and will fail to change state
if (this._isSetAnimationValid()) {
this._onAnimationEnd();
}
}
},

_onAnimationEnd: function () {
Copy link
Member

@valdrinkoshi valdrinkoshi Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
  }
},

// If no longer showing add class hidden to completely hide dialog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (!this._showing) {
this.classList.remove(this.exitAnimation);
this._finishRenderClosed();
} else {
this._finishRenderOpened();
}
},

_isSetAnimationValid: function () {
// If CSS Class does not have animation-name considered not be an animation
var style = window.getComputedStyle(this);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

return (style.getPropertyValue("animation-name") === "none")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ' instead of " here :)

}
}
});
</script>

});
</script>