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

onDone not triggered on programatic call when no scroll is needed #111

Closed
trafium opened this issue Dec 6, 2018 · 6 comments
Closed

onDone not triggered on programatic call when no scroll is needed #111

trafium opened this issue Dec 6, 2018 · 6 comments

Comments

@trafium
Copy link

trafium commented Dec 6, 2018

There is a closed issue #27 considering probably same case (@ilkome did not specify whether he did it programatically). I am not sure if my message would be seen there since it is closed, sorry if I'm not doing it right.
Fiddle: https://jsfiddle.net/cfsv3dz8/
Tested on MacOS Sierra, Google Chrome Version 70.0.3538.110

@trafium trafium changed the title onDone not triggered on programatic call onDone not triggered on programatic call when no scroll is needed Dec 6, 2018
@rigor789
Copy link
Owner

rigor789 commented Dec 6, 2018

You are passing body as the scroll to element, try passing a child element of body, as otherwise the scrollTop will always be 0 and no scrolling will occur.

@trafium
Copy link
Author

trafium commented Dec 17, 2018

Try to scroll down manually a bit and then click the button - you will see that both scroll and callback indeed can and do happen.
This GIF should illustrate the issue: callbacks not fired when no scroll occurred.
giphy

UPD: Thank you, I can now see, that it indeed happens only when body is targeted, still not sure that it is the intended behaviour.

@brophdawg11
Copy link
Contributor

It think this is still an issue when not targeting body. If no scroll is needed, the onDone callback should fire. Otherwise, consuming libraries have no idea if they can proceed to their next "action".

Here is an example showing the problem: https://codepen.io/brophdawg11/pen/exEwgX

The problematic code seems to be here: https://github.com/rigor789/vue-scrollto/blob/master/src/scrollTo.js#L193

In our specific use case, we're promisify-ing this library through a mixin, and in the case that an element is in the viewport, the promise will never resolve, unless we implement the same logic for checking vieport presence:

export default {
    methods: {
        $scrollToElement(element) {
            return new Promise((resolve) => {
                VueScrollTo.scrollTo(element, 500, {
                    onDone: resolve,
                    onCancel: resolve,
                    force: false,
                });
            });
        },
    },
};

I'm going to try to get a PR together to fix this now

@rchl
Copy link
Contributor

rchl commented Mar 18, 2019

Can the PR be integrated please? I really need this fix.

@rchl
Copy link
Contributor

rchl commented Mar 18, 2019

@rigor789 Sorry but this didn't fix my problem because issue also exists with force: true.

@gdsowl
Copy link

gdsowl commented Apr 15, 2019

Please check whether 'onDone' is defined before calling!

#140

rigor789 pushed a commit that referenced this issue Sep 10, 2019
Change applies in case there is no need to scroll.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants