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

scroll goes too soon #38

Open
Kukunin opened this issue Jul 23, 2015 · 8 comments
Open

scroll goes too soon #38

Kukunin opened this issue Jul 23, 2015 · 8 comments

Comments

@Kukunin
Copy link

Kukunin commented Jul 23, 2015

Hello here. I'm trying to use this directive for my case, and it seems it doesn't work properly.

I have scrollable div, with AJAX messages there. So, I expect, that after message is added, container will scroll down for it. But, it doesn't work.

My inverstigations:

$window.addEventListener('resize', scrollIfGlued, false); doesn't call the callback. Callback is called from scope.$watch(scrollIfGlued); immediately after AJAX query done (it's noticeable from stacktrace), and scroll function is called too early, when el.scrollHeight isn't enought high.

The solution is

scope.$watch(function() {                                   
  $timeout(scrollIfGlued, 0, false);                                  
}); 

I'd commit pull request, but test fails for me, and I can't repair them.

I understand, it sounds like edge case, but it will make directive more robust

@musikele
Copy link

I can confirm that the "fix" from kukunin is working for me, I have a similar case where messages are updated asynchronically .

@jahvi
Copy link

jahvi commented Oct 12, 2015

👍 Looking forward for a fix to async messages as well

@jahvi
Copy link

jahvi commented Oct 12, 2015

My bad it does seem to be fixed in the latest version, npm is still on 2.0.4 that's why it wasn't working on my end.

@Smolations
Copy link

Yea I have requested NPM parity in a separate issue, but have not heard from the author. It has been 4.5 months since I created that issue. I think this author is not-so-committed to this being a public project. Maybe someone will eventually fork and be committed to maintaining?

@twelve6
Copy link

twelve6 commented Feb 19, 2016

Just want to note that:

scope.$watch(function() { $timeout(scrollIfGlued, 0, false); });

Did not work for me, but:

scope.$watch(function() { $timeout(scrollIfGlued, 100, false); });

Did.

@dbrgn
Copy link

dbrgn commented Jun 25, 2018

I can confirm this. The scrollIfGlued function is called for every $digest cycle. This can cause problems like threema-ch/threema-web#18 ("scrolling up sometimes fails") if a lot of events are dispatched (e.g. when there's an animation running at the same time in the same scope), because the function is called before the scrolling event has finished and is out of the "glue" zone.

In addition to running the function at the end of the $digest cycle, I also wanted to delay it a bit (like @twelve6 mentioned) and add some debouncing. I ended up at this:

scope.$watch(() => {
    const ctx = this;
    if (ctx.watchPromise === undefined) {
        ctx.watchPromise = $timeout(() => {
            scrollIfGlued();
            ctx.watchPromise = undefined;
        }, 10, false);
    }
});

This way, the scrollIfGlued function is not called more often than once every 10ms.

@oliversalzburg
Copy link
Collaborator

It seems reasonable to debounce this behavior. If you could provide the change as a pull request, that'd be great.

@dbrgn
Copy link

dbrgn commented Jun 25, 2018

Actually it still does not work properly in all cases 🙁 Still debugging...

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

No branches or pull requests

7 participants