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

fix($animateCss): respect transition styles already on the element #13333

Closed
wants to merge 0 commits into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Nov 18, 2015

Previously, $animateCss wouldn't use transition styles that were on the element
before the animation process started. Precisely, transition property, timing-function
and delay were overwritten in the process.

This is a fix for #12656

It's a bit of a special case, because all the transition properties are sitting on the element already before $animateCss is called.

This could also affect the animation property, I haven't checked yet.

Strangely enough, this fix doesn't work when the transition property is all, because for some reason $animateCss blocks transition duration etc. when it's all - see https://github.com/angular/angular.js/blob/master/src/ngAnimate/animateCss.js#L596 Removing this doesn't break any tests, though.

@@ -1855,7 +1901,7 @@ describe("ngAnimate $animateCss", function() {

var style = element.attr('style');
expect(style).toContain('3000s');
expect(style).toContain('linear');
expect(style).toContain('ease');
Copy link
Contributor

Choose a reason for hiding this comment

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

why change this?

@Narretz Narretz force-pushed the fix-animateCss-12656 branch from 01fe1bc to 8a69ab0 Compare November 18, 2015 17:59
@@ -226,6 +226,7 @@ var DETECT_CSS_PROPERTIES = {
transitionDuration: TRANSITION_DURATION_PROP,
transitionDelay: TRANSITION_DELAY_PROP,
transitionProperty: TRANSITION_PROP + PROPERTY_KEY,
transitionTimingFunction: TRANSITION_PROP + TIMING_KEY,
Copy link
Member

Choose a reason for hiding this comment

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

See what you did ? Now you have to indent all of them 😄

@Narretz Narretz self-assigned this Nov 24, 2015
@petebacondarwin petebacondarwin modified the milestones: 1.4.8, 1.4.9 Nov 26, 2015
@matsko
Copy link
Contributor

matsko commented Dec 9, 2015

This looks good, we just need to check the ease default value and to update the timings value.

@matsko
Copy link
Contributor

matsko commented Dec 9, 2015

Please also squash both commits together since the first one fails without the second one.

@matsko
Copy link
Contributor

matsko commented Dec 9, 2015

Excellent work!

@Narretz Narretz force-pushed the fix-animateCss-12656 branch from 8a69ab0 to 2cf71b0 Compare December 9, 2015 15:49
@Narretz
Copy link
Contributor Author

Narretz commented Dec 9, 2015

Thanks for the review!
I've added two more test:

  1. Ensure that timing-function is set to ease if not defined (that should fail if a browser doesn't follow spec)
  2. Ensure that transition-property is 'all' if not defined. I had to remove a condition from the code for that to pass.

@Narretz Narretz closed this in de9777d Dec 9, 2015
matsko pushed a commit to matsko/angular.js that referenced this pull request Dec 9, 2015
Previously, $animateCss wouldn't use transition styles that were on the element
before the animation process started. Precisely, transition property, timing-function
and delay were overwritten in the process.

Closes angular#12656
Closes angular#13333
@Narretz Narretz reopened this Dec 9, 2015
@Narretz
Copy link
Contributor Author

Narretz commented Dec 9, 2015

Reopening to check what travis says.

@Narretz Narretz closed this Dec 10, 2015
@Narretz Narretz force-pushed the fix-animateCss-12656 branch from 2cf71b0 to b7b06d8 Compare December 10, 2015 09:25
Narretz added a commit to Narretz/angular.js that referenced this pull request Dec 10, 2015
Previously, $animateCss wouldn't use transition styles that were on the element
before the animation process started. Precisely, transition property, timing-function
and delay were overwritten in the process.

Closes angular#12656
Closes angular#13333
Narretz added a commit to Narretz/angular.js that referenced this pull request Dec 17, 2015
Previously, $animateCss wouldn't use transition styles that were on the element
before the animation process started. Precisely, transition property, timing-function
and delay were overwritten in the process.

Closes angular#12656
Closes angular#13333
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants