Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Sidenav causes an exception in angular-animate #2261

Closed
oliverzy opened this issue Apr 11, 2015 · 16 comments
Closed

Sidenav causes an exception in angular-animate #2261

oliverzy opened this issue Apr 11, 2015 · 16 comments
Assignees
Milestone

Comments

@oliverzy
Copy link

Today I updated to Angular 1.4 RC0, I found an exception is thrown in this line:
https://github.com/angular/material/blame/master/src/components/sidenav/sidenav.js#L292

After looking at the document, $animate.enter and $animate.leave has different signature, I changed the line to isOpen ? $animate.enter(backdrop, parent) : $animate.leave(backdrop),, the problem solved.

@ThomasBurleson ThomasBurleson added this to the 0.9.0 milestone Apr 11, 2015
@ThomasBurleson ThomasBurleson self-assigned this Apr 11, 2015
@ThomasBurleson
Copy link
Contributor

@oliverzy - Great catch.
Yes both Angular 1.3.x $animate and 1.4.x $animate have the same API:

  • $animate.enter(element, parent, [after], [options]);
  • $animate.leave(element, [options]);

The mdSideNav code is simply wrong.

@phyllisstein
Copy link

This may not be altogether germane, but it looks like some of the other changes to $animate in Angular 1.4.0-rc.0 may be causing further problems: after your change, the sidenav no longer throws an error but it doesn't exactly animate either---it just pops in and out. Downgrading to 1.4.0-beta.6 restores the animation.

@ThomasBurleson
Copy link
Contributor

@phyllisstein - The Online Docs SideNav uses the $animate changes and still animates. Please provide a CodePen or Plunkr that demonstrates your issue.

@phyllisstein
Copy link

@ThomasBurleson Right you are! Couldn't reproduce it; must be something funky in my application. Sorry for the bother!

@phyllisstein
Copy link

@ThomasBurleson On second thought, I was able to reproduce it here, in Safari and Chrome: http://plnkr.co/edit/jzI6LmIWGwFPBzjJ17y3?p=preview. The issue seems to occur only when I bootstrap the application manually, with angular.bootstrap; if I set an ng-app on the document, it works as expected.

@ThomasBurleson
Copy link
Contributor

Interesting...

@ThomasBurleson
Copy link
Contributor

@matsko - any idea why this $animate issue is occurring with a bootstrap process ?

@ThomasBurleson
Copy link
Contributor

@matsko - Here is a revised CodePen SideNav with Material and Angular.js 1.4.x that manifests this:

screen shot 2015-04-12 at 11 54 39 am

The interesting issue if you are not bootstrapping is the at the above error still occurs YET the SideNav animation still works. But if you bootstrap():

<script type="text/javascript">
      angular.element(document).ready(function() {
        angular.bootstrap(document, ['sidenav']);
      });
    </script>

Then the slidein animation no longer works.

@matsko
Copy link
Contributor

matsko commented Apr 13, 2015

@ThomasBurleson I'm not seeing the error in the provided plunkr. Did you intend to use another link?

@ThomasBurleson
Copy link
Contributor

@matsko - I do not see the type error now
The Plunkr is now updated to use angular.bootstrap().... notice the slideIn effect is not used and the SideNav jumps into view without animations.

@ThomasBurleson ThomasBurleson added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Apr 13, 2015
@matsko
Copy link
Contributor

matsko commented Apr 13, 2015

If you do:

angular.bootstrap(document.body, ['sidenav']);
angular.bootstrap(document.querySelector('html'), ['sidenav']);

Then it works. Perhaps using document directly has issues with the new ngAnimate code. I'll investigate.

@ThomasBurleson
Copy link
Contributor

Closing this issue.

@ThomasBurleson ThomasBurleson removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Apr 13, 2015
@matsko
Copy link
Contributor

matsko commented Apr 14, 2015

@ThomasBurleson this is a legitimate issue. I've created an issue and a pull request to fix it: angular/angular.js#11574 and angular/angular.js#11575

@ThomasBurleson
Copy link
Contributor

@matsko - thx for the super fast turnaround!

@matsko
Copy link
Contributor

matsko commented Apr 15, 2015

This has now been merged and fixed in master for 1.4. angular/angular.js@bee14ed

@ThomasBurleson
Copy link
Contributor

@matsko - awesome. thx for the follow-up.

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

No branches or pull requests

4 participants