ANCHOR SCROLL: chore(docs): add directive to pad heading anchors

[petebacondarwin]

Closes #2070
Closes #9360

[petebacondarwin]

@IgorMinar and @gkalpak - can you consider this solution?
I appreciate that it does require that there are further watches (although for explicit attribute interpolation it uses the more optimal $observe instead) but these are minimal and no DOM mods will occur if the values don't change. As @IgorMinar points out modifying $anchorScroll has a load of its own issues, which could also affect user experience.

[petebacondarwin]

A complete solution (for Angular.js) will require some changes to the dgeni-packages/ngdoc templates since there are situations where we are using id attributes on li elements as well as headings and anchors

[petebacondarwin]

Here is an example of it working: https://ci.angularjs.org/job/angular.js-pete/678/artifact/build/docs/guide/compiler#understanding-view
You can see that the Understanding View heading appears correctly.

It doesn't yet work for the link given in the issue: https://docs.angularjs.org/api/ng/service/%24animate#removeClass since this is one of those where the id is on an li element.

[gkalpak]

ngOffset (with lowercase s).

[gkalpak]

This is a detail, but theoretically we should check if the ng-offset property exists, because we might have an initially falsy value (in order to use the default headingOffset.value), but later on (based on some event) assign a proper value to ngOffset.

E.g.

<h1 id="heading1" ng-offset={{(someCondition) ? '120px' : ''}}></h1>

will end up using headingOffset.value forever (even after someCondition evaluates to true).

Additionally, it might be a good idea to add a check in updateStyle():

var updateStyle = function (offset) {
    offset = offset || headingOffset.value;
    if (offset) {
       ...
    }
};

[petebacondarwin]

My view was that if the ngOffset attribute is ever defined then it should never default to the global headingOffset value. Is that wrong do you think?

[gkalpak]

The idea is to support having ngOffset evaluate to something under some condition, or evaluate to undefined or null if the condition is not met and let the default headingOffset be used.

E.g. having the following HTML:

<h1 id="heading1" ng-offset="{{weAreOnSmallScreen() ? '0px' : ''}}">Heading 1</h1>

we could account for the following scenario (based on how the docs app is designed):
if we are on a small screen, the header's positioning becomes static (not fixed), so we don't need an offset. So, if weAreOnSmallScreen() evaluates to true, use the value of ngOffset (which is '0px').
If we are on a large screen, the header is fixed and we should use the default offset of headingOffset.value. weAreOnSmallScreen() evaluates to true, attrs.ngOffset evaluates to '' (empty string) and the updateStyle() function understands that it should use the default offset.

This is indeed an implementation choice (i.e. nothing inherenty wrong if we don't support it) and the added flexibility might not be worth the trouble. Up to you :)

[gkalpak]

Other than my comments above, it LTGM !

_{It's not the ideal, super versatile, I-will-do-everything-for-you solution, but it is way better than before :)}

[gkalpak]

As @IgorMinar points out modifying $anchorScroll has a load of its own issues, which could also affect user experience.

Hm... @petebacondarwin, I just took a look at $anchorScroll and it is a tiny little service and it seems super easy to incorporate the extra offset in pixels (either as a fixed value or as a function). It shouldn't be more than 5-10 lines of code.
What are those issues (or where are they pointed out) ? I am probably missing something...

[petebacondarwin]

@gkalpak - excellent review. Thanks for that. I'll fix up the PR.

[petebacondarwin]

@gkalpak - I see you have created your own PR. I will take a look. In the meantime I have updated mine with fixes.

[gkalpak]

...with your...

[petebacondarwin]

Closing in favour of #9371