Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State change scrolls page to top #110

Closed
rynz opened this issue Apr 29, 2013 · 31 comments · Fixed by #715
Closed

State change scrolls page to top #110

rynz opened this issue Apr 29, 2013 · 31 comments · Fixed by #715
Labels
Milestone

Comments

@rynz
Copy link
Contributor

rynz commented Apr 29, 2013

If you create a scrollbar, eg by resizing the Sampe App. Whenever you change state, it scrolls to the top of the page.

If state changes, it shouldn't look like a browser refresh, rather change the nested elements accordingly and maintain where the scrollbar is set.

Eg:
Run Sample App.
Click Contacts.
Resize Sample App so you have a scrollbar.
Scroll to bottom
Click "Show random contact" or Email or Edit etc

The page will jump to the top.

Is this the desired effect?

@legomind
Copy link

legomind commented May 1, 2013

According to angularjs docs, the $anchorScroll service scrolls to the top of the page when $location.hash() is empty or when it contains a reference to an element that does not exist.

This behavior can be disabled in with the following code in the config section of your angular app.

$anchorScrollProvider.disableAutoScrolling()

So, yes. This is the desired behavior.

@rynz
Copy link
Contributor Author

rynz commented May 1, 2013

@legomind Unfortunately calling $state.transitionTo with url parameter in it's corresponding $state it still jumps to the top of the page even with $anchorScrollProvider.disableAutoScrolling().

@ksperling
Copy link
Contributor

ui-view has inherited the call to $anchorScroll from core-Anglar ng-view. The idea seems to be that the content added by the view's template might make a previously invalid anchor valid, thus the call to $anchorScroll.

It's all not ideal though... But I think this might need looking at properly outside of the context of just ui-router.

@rynz
Copy link
Contributor Author

rynz commented May 2, 2013

@ksperling I guess I can understand from the point of ng-view because generally that's an entire view change instead of this much nicer state change solution.

I noticed in the Sample App changing between contacts.detail.item and contacts.detail.item.edit state, even though it does not update the anchor it still scrolls to top. Perhaps a configurable option for ui-view to allow / disallow whether scroll to top is enabled on state change. Perhaps a rule based system similar to $urlRouterProvider passing true/false on whether scroll to top happens?

@jeme
Copy link
Contributor

jeme commented May 2, 2013

As @ksperling says, the scrolling feature of angular core is properly better to change, if you don't wan't it you could provide your own implementation. Like so:

var $AnchorScrollProvider = function() {
  this.$get = ['$window', '$location', '$rootScope', function($window, $location, $rootScope) {
    function scroll() {
    }
    return scroll;
  }];
}
angular.module('yourApp').provider('$anchorScroll', $AnchorScrollProvider);

that should disable scroll all together (remember to depend on the module you put the alternated provider in if not your app module)...

Even in the fine grained views, scrolling to top is the appropriate default action, think of a case where the newly loaded content was activated from the bottom of the page, but takes up more than a window can hold, would you wan't the view to show the bottom initially?... The user properly wan't to read your newly loaded content from the top.

There can be cases though where you don't wan't that, and I think that is where the current $anchorScroll is to simple, I am especially annoyed about it solely using the hash value, if I wanted /blog/post/42/comments to load the page and scroll to the comments section, I can't using $anchorScroll instead I have to use: /blog/post/42#comments

The ui-router project could overwrite it's behavior I guess. That would be one option to explore making it more. Or we could simply not use it in ui-view, but i still think we should scroll to top as the default behavior, and why not use/extend the component that is meant to do that.

@rynz
Copy link
Contributor Author

rynz commented May 2, 2013

@ksperling @jeme I do agree it is very situational and the default behavior is desirable in most cases. I also think if ui-router implemented a $urlAnchorScrollProvider or something of this nature to apply rules and returning false to disable scroll many people would take advantage of it.

Or something hopefully Angular-Core could think about.

A prime example is if we use state to generate a lightbox, something we want people to be able to link to. It will scroll to top and lose your current position of the background content.

@ksperling
Copy link
Contributor

If we can do something better in ui-router that would be good. Maybe coming up with what the desired behaviour would be in a number of different cases would be a good start, e.g. quite often a state will update multiple views to if they all try to scroll an arbitrary one is going to "win"... Maybe the scroll behaviour should be specified for each state as a whole?

@rynz
Copy link
Contributor Author

rynz commented May 6, 2013

@ksperling Personally I think the default behavior should be do not scroll, then have an option when configuring each state of scrollToState: true or something of that nature. So whenever we transitionTo a particular state it will decide whether to scroll or not.

The only most common case to scrollToState would be navigation states imo, where as states like a lightbox, toggling etc should not effect scroll state.

@laurelnaiad
Copy link

+1 for default being not to scroll. I'm pretty new to Angular -- are there mechanisms that an app could use to scroll when it wants to? I hope so!

@jeme
Copy link
Contributor

jeme commented May 6, 2013

@SparkiO @stu-salsbury scolling to top (not to state, what does that even entail if the state has multiple views???) would be the most desired case I think, after all I think most pages will consist of more "content" loading that dialogs.

Take this example of a page and a view-port location:

Simple page

Wouldn't you wan't the page to scroll up so you can instantly begin to read from the beginning of the article or what the main content may be?... And even if you are loading an outer view, same rule would apply, we always read from the top.

Then I think we should be able to do things like:

  • scrollTo:top - scroll to top, explicitly stated. (This also enables one to override another scrollTo from a parent)
  • scrollTo:null - don't scroll, not even to top.
  • scrollTo:@viewname - scroll to a view.
  • scrollTo:elementid - scroll to an element id
  • scrollTo:['$stateParams', function($stateParams) { return stateParams.section; } - scroll to element with id or view if starts with @

Then the default for a state would be: (pseudo code)

state.scrollTo = state.scrollTo || (state.parent && state.parent.scrollTo) || 'top'...

@laurelnaiad
Copy link

I like your thinking about the options, @jeme. I guess the scrolling might
relate to whether one is in the mindset of web app or web site (or
where we all probably live which is somewhere on the spectrum in between).
I like the inheritance idea (assuming the default stretches not just to
the parent state (or view) but all the way up to the root) because the
developer can make a decision at the root that essentially becomes the
site/app default.

@jeme
Copy link
Contributor

jeme commented May 6, 2013

@stu-salsbury it would, since that if the parent state isn't a root, it would have done the same trick to calculate it's scroll value, the "scroll" var was meant to be set on the internal wrapper object for states. That might not have been clear from the example. I have updated the comment to try and make that more clear.

However, since there is no "global root" in ui-router, it would have to be an option for the $stateProvider to maybe set some defaults... like: $stateProvider.defaults({ ... }); or another way.

@rynz
Copy link
Contributor Author

rynz commented May 6, 2013

@jeme I like it. I also like the idea of setting the default behavior depending on which you need more. Scroll to top, or scroll to null.

@lpaulger
Copy link

+1
Also to fix this for me, instead of writing my own $anchorScroll service, I just did this:

angular.module('Application',[]).value('$anchorScroll', angular.noop);

@gaelollivier
Copy link

+1 would be a great feature ! Very useful especially on mobiles where the freshly loaded view has great chances not to be in the viewport and the user might expect to see what has just been loaded.
In my case, I have a sidebar displaying a list of items and when I click one of the I got its details view on the right side. On mobiles, the list appear above the details view so when the user click one item the window has to scroll down to it.

Because in my case, simply erasing $anchorScroll doesn't work (I need to scroll top in a lot of cases, though) I wrote this little patch that is very crappy but I don't see how to make it proper since I'm new to AngularJS.

So here is my solution, for whose who need a temporary one until the ui-router team has solved the issue:
I start by erasing the $anchorScroll service in a way like:

angular.module('myapp', [...]).value('$anchorScroll', angular.noop)...

Then I register a $viewContentLoaded event listener (not $stateChangeSuccess because I sometimes need to scroll to an anchor that just have been loaded) to scroll to the desired view, depending on state params:

    run(['$state', '$stateParams', '$window', function($state, $stateParams, $window){
        $rootScope.$on('$viewContentLoaded', function(){
            var state = $state.$current;
            if (state.scrollTo == undefined) $window.scrollTo(0, 0);
            else {
                var to = 0;
                if (state.scrollTo.id != undefined)
                    to = $(state.scrollTo.id).offset().top;
                if ($($window).scrollTop() == to)
                    return;
                if (state.scrollTo.animated)
                    $(document.body).animate({scrollTop:to});
                else
                    $window.scrollTo(0, to);
            }
        });
    }])

Then I can configure my states like this:

state('applications.details', {         
    ...
    scrollTo    : {id:'#applications-content-anchor', animated: true}
}).

It's far from perfect but it does the job. Hope it can help !

@carloc
Copy link

carloc commented May 24, 2013

@lpaulger YOU ARE MY HERO! :D Saved the day, thanks!

@maowug
Copy link

maowug commented May 30, 2013

@gaelduplessix +1.

@arush
Copy link

arush commented Jun 8, 2013

@lpaulger omg thank you for this hack!

+1 for @jeme 's solution

ui-router fundamentally changes the UX for hashchange events, so don't agree that you can ignore this side-effect. It is the 'expected behaviour' for $routeProvider, not $stateProvider

@Nek-
Copy link

Nek- commented Sep 1, 2013

That's an ugly hack. Nothing was did in the @gaelduplessix way ?

@pablocaselas
Copy link

You can achieve the behaviour of not scrolling to the top of the page by simply injecting this...

angular.module('ui', []).value('$anchorScroll', angular.noop);

But there's a little glitch when loading a nested view that is not already chached...

Anybody solved this, properly?

@nateabele
Copy link
Contributor

This seems like a really good use case for uiView to have an autoscroll attribute like ngInclude has. If you agree, please take note: we are actively accepting pull requests.

Also, for anyone who suggested that scrolling should somehow be a part of $state / $stateProvider: your API design privileges are revoked until further notice.

@laurelnaiad
Copy link

You can achieve the behaviour of not scrolling to the top of the page by simply injecting this...
angular.module('ui', []).value('$anchorScroll', angular.noop);
But there's a little glitch when loading a nested view that is not already chached...
Anybody solved this, properly?

Have you tried $anchorScrollProvider. disableAutoScrolling()? It might be a finer surgical instrument than nulling out the whole service. I haven't tried it (and I don't know how longs it's been in the Angular codebase).

If that doesn't work or isn't in your version of Angular, you might use the decorator service to alter its behavior instead of clobbering it. Just a thought.

@rynz
Copy link
Contributor Author

rynz commented Sep 6, 2013

+1 @nateabele suggestion of uiView incorporating autoscroll attribute like ngInclude has.

@juanpujol
Copy link

+1 for this fix.

@bernardolm
Copy link

+1

@Nek-
Copy link

Nek- commented Oct 18, 2014

@bernardolm did you notice this issue is solved by #715 ?

@bernardolm
Copy link

Hi @Nek-!
In my app, I'm using:

"angular": "~1.2.25", "angular-resource": "~1.2.25", "angular-sanitize": "~1.2.25", "angular-route": "~1.2.25", "angular-messages": "1.3.0-build.3067+sha.d250dd4",

And when ng-model values changes, even the ng-model change asynchronous and not from clicks, the window scrolls up to top. I test the approachs here quoted and doesn't worked. But I have a particularity, the model changed is in a directive and not on parent controller scope.

'Cause that, I post "+1".

@wkonkel
Copy link

wkonkel commented Mar 19, 2015

Here is how I solved the scroll-to-top-on-new-page problem:

http://stackoverflow.com/questions/21055952/changing-route-doesnt-scroll-to-top-in-the-new-page/29153678#29153678

@zwacky
Copy link

zwacky commented Oct 23, 2015

the following solved my problem. maybe someone comes across the same.

if ('scrollRestoration' in history) {
    history.scrollRestoration = 'manual';
}

@rafaelune
Copy link

Tks a lot @gaelduplessix!!

@yaplas
Copy link

yaplas commented Dec 28, 2015

that is the solution zwacky, thanks

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

Successfully merging a pull request may close this issue.