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

fix($animate): applyStyles from options on leave #10068

Closed
wants to merge 1 commit into from

Conversation

PatrickJS
Copy link
Member

missing [options] API that should either be removed from the docs or added in the code

@param {object=} options an optional collection of options that will be applied to the element.

the tests weren't working for this feature

      $animate.leave(element, 'off', {
        to: { color: 'blue' }
      });
      assertColor('blue'); //nothing should happen the element is gone anyway

you can view the docs here https://ng-click.com/$animate#leave

@petebacondarwin petebacondarwin added this to the 1.3.4 milestone Nov 17, 2014
@petebacondarwin
Copy link
Contributor

@gdi2290 - thanks for this. Could you provide a unit test that checks that the options argument has an effect?
@matsko - can you verify that this is correct and merge?

@PatrickJS
Copy link
Member Author

@petebacondarwin I fixed the test and pulled in latest changed from master

// previously 
$animate.leave(element, 'off', {
  to: { color: 'blue' }
});
// should be
$animate.leave(element, {
  to: { color: 'yellow' }
});

the problem was that the test had too many arguments (due to copy/paste of $animate.removeClass)

missing <code>[options]</code> API that should either be removed from
the docs or added in the code

@param {object=} options an optional collection of options that will be
applied to the element.

the tests weren't working for this feature
```javascript
$animate.leave(element, 'off', {
to: { color: 'blue' }
});
assertColor('blue'); //nothing should happen the element is gone anyway
```

you can view the docs here https://ng-click.com/$animate#leave
@PatrickJS
Copy link
Member Author

@petebacondarwin can you remove the [needs: test] tag. The PR is ready for merge and really should have made 1.3.4 release unless I'm missing something. $animate still is broken for leave animation

@googlebot
Copy link

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

@PatrickJS
Copy link
Member Author

I also signed CLA already
https://github.com/angular/angular.js/commits?author=gdi2290
unless there was an update recently on how you manage CLAs

@pkozlowski-opensource pkozlowski-opensource modified the milestones: 1.3.5, 1.3.6 Dec 1, 2014
@caitp caitp modified the milestones: 1.3.6, 1.3.7 Dec 9, 2014
@petebacondarwin
Copy link
Contributor

@gdi2290 - thanks for the fix to the test. I have removed the tag. There were changes to the CLA process a few months ago.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA.

petebacondarwin pushed a commit that referenced this pull request Mar 4, 2015
hansmaad pushed a commit to hansmaad/angular.js that referenced this pull request Mar 10, 2015
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
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.

9 participants