Shouldn't $anchorScroll listen for $viewContentLoaded if necessary

[jeme]

There exists a TODO in the view directive, I wanted to add an issue here where we could discuss that.

          // TODO: This seems strange, shouldn't $anchorScroll listen for $viewContentLoaded 
          // $anchorScroll might listen on event...
          $anchorScroll();

I Think this is about dependencies, if anchorScroll listened to $viewContentLoaded, it would suddenly become dependent on that event, so it shouldn't do that, if it should use event it should use a event name defined by anchorScroll and not the other way around.

But then it wouln't be much different than depending on it as a service and calling it. So I think we should just remove the TODO.

[ksperling]

Well, the difference is that the way things stand currently we've got a hard dependency on $anchorScroll, whereas the other way around $anchorScroll could perfectly well listen for an event that happens to never be fired, i.e. using $anchorScroll wouldn't force the $state service to be instantiated, but currently using $state forces $anchorScroll into existence.

This is exactly the kind of situation that event's are for -- where you don't want the publisher to have to know all the things that may or may not need to be done when something else has happened.

At this point I guess we should just leave things as they are in core AngularJS; I'll raise this as an issue on the core project.

[ksperling]

Actually looking the code again I'm wondering if that // $anchorScroll might listen on event... comment is essentially the same as my TODO comment in not so many words.

[jeme]

@ksperling true, but in that case as I said, it shouldn't be an event defined by the view, instead $anchorScroll should define what event it listens to, otherwise you end up having to fire that event in other places to trigger anchor scroll... would be really weird standing inside a controller and raise the "viewContentLoaded" event, not to mention implications when other uses the event.

Anchor scroll can be configured to autoWatch, in that case it gets called on all digest phases so I think that is what they might refer to, but otherwise it only scrolls when called... (currently it seems to be bound to hash value though, which limits its overall use imo)

[jeme]

On a side note, maybe we should remove this from the view all together, after all it makes sense to let the view handle it (scroll to top) in the case of ng-view, as there is only a single view in play...

But for ui-view, we have a number of views in play, causing multiple calls to anchor-scroll, which doesn't really make sense, if we are to make better use of scrolling, and allow for customized scroll, we need to remove the handling from the individual views and probably to the state machine instead...

[ksperling]

@jeme Yeah that's true... Let's just remove the call for now (and maybe leave a comment pointing to this issue in the code)

[stale]

This issue has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs.

This does not mean that the issue is invalid. Valid issues
may be reopened.

Thank you for your contributions.