WIP: simplify $anchorScroll offset feature

[petebacondarwin]

Hi @gkalpak
I had a chat with Igor last night about this. He agreed that we should go ahead and implement your solution.
I did a bit of simplification of the feature in this PR. Obviously it is not complete with tests and docs but I think that it is easier to maintain and use in this form. Apart from anything it makes writing the directive to make use of it trivial!

I don't think we need to expose a new constant, since we can just hang the offset as a property off the $anchorScroll service.

We need to be aware that in the future we may need to support horizontal scrolling too so I changed the name to scroll.yOffset so that we could add in xOffset if required in the future without a breaking change.

I see what you mean about multiple scrolling containers. A comprehensive solution would handle this by additional properties and directives to define the scroll container for any anchor but I believe that for this release we should not try to support this but instead just document that we only scroll the main window.

I didn't quite see what your extra fix to do with items towards the bottom of the page did. It might have been a result of your specific implementation of the offset scrolling. In this modified version items towards the bottom of the page seem to be scrolled to OK. For instance try http://localhost:8000/build/docs/api/ng/service/$animate#setClass

I just noticed that my version breaks on travis - scrollBy doesn't exist on FF and IE... I will take a look now.

Do you want to review these changes, then if you are happy, knock up some tests and docs for this new feature? The aim is to get it into the next RC release, due on 6th October. If you haven't the time then I am happy to move it forward.

[petebacondarwin]

The test was mocking out $window and not providing the scrollBy method.

[gkalpak]

So, you basically adding a yOffset property to $anchorScroll which can be either an element or a function or a number. It's more "free-style" than I imagined you'd want it, but certainly flexible and works for me.

A couple of remarks:

1. Using getComputedStyle(...).height has the drawback that the returned height might or might not contain padding and border (based on the box-sizing property). Do you think we should account for that as well (it will add a small overhead and complexity).

2. About the fix for elements near the bottom of the scrolled element:
   When the element's height is less than the scrolled element's height (the "viewport height"), then (before applying the offset) the element will not appear at the top of the viewport (it can't because we have already scrolled all the way to the bottom so there is not more to scroll even if the element's top is not aligned with the viewport's top). In such cases, there is no point in applyng the whole offset, because we are scrolling up by an amount of pixels that is unnecessary to achieve our goal (i.e. align the last element's top with the scroll-offset-element's bottom).
   The line below calculates how much (if at all) we need to actually scroll in order to align the element's top with the offset-element's bottom.

   var actualOffset = offset && (offset - (elem.offsetTop - document.body.scrollTop));

But it's not that big a deal if you want to leave it out (for this first release :D).

[gkalpak]

@petebacondarwin : So, let me know what you think about points (1) and (2) above and then I can update the docs and write some tests.
BTW, I noticed that $anchorScrollProvider is not documented and does not appear at all under the corresponding section in the docs. Do you think I should split the docs so that $anchorScrollProvider and $anchorScroll are documented separately (instead of documenting the provider's method in the service itself) ?

[petebacondarwin]

1. SInce we only call the getYOffset() occasionally, when we are scrolling, I don't have a concern about adding a little extra overhead. Feel free to take into account the box-sizing property too.
2. If you can write tests to prove it works and docs to explain it then by all means let's add this in too. I would also add a comment into the code at that point explaining clearly what that little calculation is doing.
3. Yes, let's split out the provider docs from the instance docs.

Thanks for this work. It is looking great and will get you lots of beers from all those +1ers.