search

scottcorgan / angular-scrollto

Angular directive to scroll to element by selector.
MIT License
54 stars 10 forks source link

Window not working as a container #2

Closed Maidomax closed 10 years ago

Maidomax commented 10 years ago

When using the plugin as is, it doesn't work for me. I figured out that this is caused by the default value for the container attribute. When I change it to body it works in Chrome, when I change it to html it works in Firefox, the default value window works in neither. My solution is to set the container attribute to body, html. Then it works in both. Can you tell me why is window the default value? Does this work in any browser?

scottcorgan commented 10 years ago

You are correct in asking why I did that. I don't know. Would you like to submit a pull request with the fix?

Maidomax commented 10 years ago

Sure. There is a related issue I will fix in the same pull request. In line 10 of the non-minified file you have:

var scrollY = (typeof scrollTo == "number") ? scrollTo : scrollTo.offset().top + scrollPane.scrollTop() - settings.offset;

You don't need + scrollPane.scrollTop() there. This means that every time you click the link, the offsets of the element you are targeting and the offset you are providing as an optional parameter will add up to the current offset scrolling the page further down even if you have already reached your destination. This only works if the link is on the top of the page, and you can only click it once, which is quite often how it is, but not always. That's probably why you missed it.

scottcorgan commented 10 years ago

Closed in PR https://github.com/scottcorgan/angular-scrollto/pull/3