First Basic Version - Single Animation Only

[homerjonathan]

Following the design of paper-tooltip and paper-dialog.

This is more difficult then both paper-tooltip and paper-dialog as it comes out of the box with dual animation so that makes the Animation Config much more complicated.

However here is a simple version that starts. Working on the multiple config version.

See Request #147

[valdrinkoshi]

@homerjonathan how do you plan to support multiple animations? I'm not sure that would be possible, e.g. what if you want to play 2 animations with different durations each? That is doable with Web Animations API but not so easy with CSS animation keyframes.

[homerjonathan]

I was considering supporting up to 2. Anymore is showing off! ;-)

paper-tooltip uses this:

          this.updateStyles({
            '--paper-tooltip-delay-in': newValue + "ms"
          });

With the following

animation-delay: var(--paper-tooltip-delay-in, 500ms);

Something similar to this. However with the above example this is the delay before the animation starts. We also have the duration. So it would complicated.

Since the demo uses 2. Fade in followed by expand. I was considering along those lines.

What are your thoughts?

[homerjonathan]

By the way. I think I still have a few issues with my current commit so I would wait a little before trying it.

[homerjonathan]

Messed up mixing your updates and mine! So this commit is a little more effective.

Inside the x-select.html running the demo. It has the following

return [{
              name: 'fade-in-animation',
              timing: {
                delay: 150,
                duration: 50
              }
            }, {
              name: 'expand-animation',
              timing: {
                delay: 150,
                duration: 200
              }
            }];

So that is a delay of 150ms with a mixed duration of 50ms for fade in and 200 for expand. Someone was showing off a fancy animation.

Do you know what happens if you assign two animations to one component? Googling.....

[homerjonathan]

OK.

We have this as a possible route take from MDN https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Animations/Using_CSS_animations

animation-name: fadeInOut, moveLeft300px, bounce;
animation-duration: 2.5s, 5s, 1s;

I was thinking of

.animation-example {
 animation-name: var(--animation-name)
 animation-duration: var(--animation-duration)
 animation-delay: var(--animation-delay)
}

Using the config file generate the comma separated values as per example. Then just run the updateStyles.

this.updateStyles({
                '--animation-name': animationNames,
                '--animation-duration': animationDurations,
                '--animation-delays': animationDelays
              });

Shall I give it a whirl?

[homerjonathan]

Yes it works! Not only that you can have as many animations as you want. So that will please the animation kings.

I have updated the commit with an example. I had to change the demo from expand to scale up as I don't have the css for expand yet. I will do tomorrow when I am less tired. Unless you can assist with this ;-)

One side effect is that it calls Web Animation End twice (or as many as you have configured) and thus breaks the component, calling _finishRenderOpen() twice. However that is an easy to fix with a counter or something.

[valdrinkoshi]

I'm trying to see how far can we get with supporting only entryAnimation and exitAnimation here (1 single animation name), in the same way as we do in paper-dialog.

e.g. for x-select.html, I was considering using this technique for a performant expand/collapse animations https://medium.com/@valdrinkoshi/performant-expand-collapse-animations-93d99e80f7f
I'm working on it right now, to see how far can we get with merging the opacity + collapsing animations into 1 single set of keyframes.

Btw, also paper-menu-button is another example where we use more than 2 animations in sequence, see https://github.com/PolymerElements/paper-menu-button/blob/master/paper-menu-button.html#L270

Please feel free to continue experimenting with multiple animations tho!

[homerjonathan]

I have the solution for this. However my latest commit with the example for some reason hasn't made it up. Will send when I get back to my home computer.

[homerjonathan]

OK. It works. Believe it or not you need to count commas.

          if (style.getPropertyValue('animation-name') !== 'none' &&
            style.getPropertyValue('animation-duration') !== '0ms') {
            // How many Animations are legal?  Count the commas!
            for (var count = -1, index = -2; index != -1; count++, index = style.animationName.indexOf(",",
                index + 1));
            this.noValidAnimations = count;
            return true;
          } else {
            this.noValidAnimations = 0;
            return false;
          }

If you setup two animations. One is valid and one is not. You will only get one Animation End event. Thus you can end up waiting forever _finishRenderOpened(). So just using the count of animations will fail. When you set the two animations and one fails. The animationName only has one name and thus no comma. Thus counting the commas confirms the number of Animation End Events that will fire. Thus a simple counter waits until the last one is finished.

My trick of using a mixin works. It is a great cheat because if you use the openAnimation the whole section is missed out. So it supports the old config file method with a much more efficient new way of doing it.

Hope you like it!

If so I can repeat for paper-menu-button

[homerjonathan]

Just need a tidy up and check to make sure all tests are correct etc.

[homerjonathan]

Researching the failed tests.

First was a problem with the test config file not having a .timing value. So I have added testing to check and not assign if it is missing.

Most of the test fails are when the openAnimnationConfig since it does a assert.deepEqual that checks for node: context. This is simply not required by the updated component so it no longer required. So it now just checks that the values assigned are set. Some of the tests in the section correct animationConfig setup is also now no longer useful and just commented out as a placeholder for a new test. I have in mind the best would be using something similar to _willAnimate function. To get the style to check to see if it is assigned. Will have a think and add it shortly.

[homerjonathan]

Fails with a problem in the iron-dropdown.d.ts.

Its a generated file but my environment is not generating it. So it has a reference to

/// <reference path="../neon-animation/neon-animation-runner-behavior.d.ts" />

Which is no longer required. Any pointers to fix it?

[valdrinkoshi]

oh right you need to run npm run update-types and commit the updated files. The travis build will check that the branch has the correct updated types

[valdrinkoshi]

I noticed that the animations are not playing properly

The content should expand from top to bottom, and it doesn't seem to work when opening the second time.

Btw, I won't be able to review in detail this PR for now, but I just wanted to share the progress I did so far with the approach I mentioned in #153 (comment), it's in the no-neon-animation branch, and I used the technique described in https://medium.com/@valdrinkoshi/performant-expand-collapse-animations-93d99e80f7f - I wanted to check how this fits with paper-menu-button, but that will need to wait for a bit on my side :/

[homerjonathan]

Looking at the neon-animation code for expand it does this:

this._effect = new KeyframeEffect(node, [{
        height: (height / 2) + 'px'
      }, {
        height: height + 'px'
      }], this.timingFromConfig(config));

My version starts with height with a value of 0. So correcting this to 0.5 if I am reading how this works correctly. My first design grew both upwards and downwards at the same time, which seems nicer then the original. However I noticed that the expand-animation with neon-animation goes downwards.

So I have adjusted to go downwards. It actually feels better than the original. Try them both side by side. I think the new version is smoother. I am biased however ;-)

[homerjonathan]

Stuck with issue #95 on https://github.com/Polymer/gen-typescript-declarations/issues/95 so I can run the npm run update-types on Windows.

So can't create the iron-dropdown.d.ts without the Neon-Animation Depends. Suppose I can alter it manually but that is cheating..

[valdrinkoshi]

re animation: while it would be fine to change it in the iron-dropdown demo, we'd still need to support exactly that type of animation in paper-menu-button (e.g. it has similar open/close animations, see https://github.com/PolymerElements/paper-menu-button/blob/master/paper-menu-button-animations.html), which is based on Material Design.
In particular, we have to be able to expand/collapse (meaning the content shouldn't be squeezed).

I tried to recreate the same exact animation in the no-neon-animation branch, but still needs more work (e.g. see the size of the content got changed):

master branch	no-neon-animation branch

[homerjonathan]

Still having issues with npm run update-types. So have it raised.

However trying to get the animations working exactly the same with the height. With my applying animations to this.$.contentWrapper I can't access the height of the text. You can if you use the containedElement() which returns the correct node. However if you assign the animations to that you end up with the slotted() and @Keyframes as highlighted here WICG/webcomponents#748

I was going to use the window.getComputedStyle to get the height then I was intending to use a mixin to inject the height into the animation so getting past the animation restriction of not animating height:auto or perhaps using max-height. However until I can get past the slotted scope issue I am stuck. The animations are considered valid by the _willAnimate function but are clearly not. I note that you have an inverse animation which you have almost got working. So that path may be left open.

Any suggestions for our next attempt!?

[valdrinkoshi]

You can get the computed height with this.$.contentWrapper.getBoundingClientRect().height which will give you the value as a number

[homerjonathan]

The issue with the update-types has been resolved.

I have managed to get the height of the text. However when setting the values the CSS animations still ignores the values. It needs both height from and to not to be auto. So far despite spending far too long on it I haven't manage to get it to work. Should have sometime later this week to have another attempt.

[homerjonathan]

Nope. The update-type scripts has failed. It hasn't been tested well enough on windows machines. I can see from the file generated they are backward slashes rather than forward. Thus has failed the travis build test. Will raise it as an issue!

[homerjonathan]

Issue raised here https://github.com/Polymer/gen-typescript-declarations/issues/105

Thinking I should go Unix....

[valdrinkoshi]

Uhm, I wonder if we could have update-types be run after the polymer tests, as in move this

iron-dropdown/.travis.yml

Lines 6 to 9 in 97ec5d4

	- >-
	npm run update-types && git diff --exit-code || (echo -e
	'\n\033[31mERROR:\033[0m Typings are stale. Please run "npm run
	update-types".' && false)

as last script in the script block

iron-dropdown/.travis.yml

Lines 24 to 28 in 97ec5d4

	script:
	- xvfb-run polymer test
	- >-
	if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then polymer test -s 'default';
	fi

This would at least run the polymer tests... @aomarks WDYT?

[aomarks]

This would at least run the polymer tests... @aomarks WDYT?

I did it this way because in my experience some of our elements tests are really flaky, or even just long-term broken, and I think we sometimes merge even when tests fail, which means we would never know if the typings were ok. It should have been relatively a fast and deterministic check, the problem we have here is that the generator doesn't work correctly under Windows (sorry about this, thought it was fixed this weekend but obviously there are still issues, will try to address soon).

A quick fix here would be to manually fix up the paths in the typings.

[homerjonathan]

Fantastic. I think I have exactly simulated the Height Increase animation.

First altered the contentWrapper as so

      #contentWrapper.animating ::slotted(*) {
        overflow: hidden;
        pointer-events: none;
        max-height: inherit !important;
        height: inherit;
      }

This forces the content to size to the animation frame.
Inject the animation style pushing the max-height.

          this.updateStyles({
              '--animation-height': this.containedElement.style.maxHeight
            });

Finally alter the animation

      @keyframes expand-animation {
        from {
          transform-origin: top;
          /* transform: scaleY(0.5); */
          max-height: calc(var(--animation-height) / 2);
        }
        to {
          transform-origin: top;
          /* transform: scaleY(1.0); */
          max-height: var(--animation-height);
        }
      }

This seems to work. 50% height animating to height 100% without any text shrinking.

[homerjonathan]

Any updates on this?

[valdrinkoshi]

Hey @homerjonathan, I've tested this locally and noticed issues with the animation when using Polymer 1.x - you can test it by installing the variants & serving them with the Polymer CLI:

$ polymer install --variants
$ polymer serve -o
  info:    Started multiple servers with different variants:
      View the Polyserve console here: http://127.0.0.1:8003

Unfortunately I don't think we can remove the neon-animation dependency without introducing a breaking change to users of iron-dropdown. Just the fact that it exposes open/closeAnimationConfig properties, and these are used by both paper-menu-button and paper-dropdown-menu.

My suggestion would be to rather implement a new element which is neon-animation-free. I'm happy to assist in reviewing the implementation, but unfortunately we won't be able to ship it with iron-dropdown.

[homerjonathan]

Yes I noticed the open/closeAnimation. It does actually correctly process them and merge. So it should work with Polymer 1.0. I will have a look to see why there are issues. It should correctly work with paper-menu-button and paper-dropdown-meni. It may simply be another animation is needed.

[valdrinkoshi]

Even if we provide the default animations for paper-dropdown-menu and paper-menu-button, we cannot handle custom animations that users might set through the public properties. That's the main reason why we cannot really merge this PR into iron-dropdown, because it would be a breaking change for these users.

My suggestion would be to create a new element, e.g. <simple-dropdown> which doesn't use neon-animation

[homerjonathan]

That's not the reason why this component breaks with the variation.

The function _updateAnimationConfig will handle all the animations that you can throw at it. So any user custom animations will be handled.

From first looks the elements and slots have moved around. This means the animation is not correctly applied to the right node. The animation fails and thus doesn't fire the animation end event so the this._finishRenderOpened(); isn't called.

I think that this is fixable.

[homerjonathan]

If a new version is the only solution it will mean that neon-animation will never be deprecated with this version of iron-dropdown. Does this mean that iron-dropdown also needs to be deprecated?

[valdrinkoshi]

This means that we won't be able to remove the dependency in iron-dropdown 2.x.

Also, we plan to use the 3.0 release for migrating all elements away from bower to npm, without any other breaking change.

We'll be able to see where we stand after all this is done, most likely after the Google I/O 👌

[homerjonathan]

As far as I can work out its the updateStyles that is going wrong.

https://www.polymer-project.org/2.0/docs/upgrade#use-updatestyles-instead-of-customstyle

So the styles are not updated. Thus that is why the Hybrid variant fails for 1.9.

[valdrinkoshi]

@homerjonathan I've had some time to further look into this - I think that we'd be better off allowing only simple animations on the :host, and for more complex animations (e.g. animations on children of <iron-dropdown>) delegate it to the user of iron-dropdown.

I've taken a stab at it, and got both iron-dropdown and paper-menu-button working \o/ see #154 and PolymerElements/paper-menu-button#137

I'm still a bit hesitant on removing those properties - I'd really like to not support those anymore. I already tested those PRs on internal projects and it doesn't seem to break them, so promising..The work you did in this PR could be useful - the part where you support these properties & convert them to strings. Again, my hope would be to get rid of that complexity completely.