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

pass on extra arguments to scrollTo to enable smooth scrolling with behaviour #2069

Closed
BorjaRafols opened this issue Feb 20, 2018 · 17 comments · Fixed by #3351 · May be fixed by #3210
Closed

pass on extra arguments to scrollTo to enable smooth scrolling with behaviour #2069

BorjaRafols opened this issue Feb 20, 2018 · 17 comments · Fixed by #3351 · May be fixed by #3210

Comments

@BorjaRafols
Copy link

What problem does this feature solve?

Smooth scrolling to same page anchor tags.

Right now smooth scrolling can only work by custom implementations, but the new Firefox spec contemplates a scrollBehaviour css property which can be polyfilled in older browsers.

Vue-router should take advantage of this new property

What does the proposed API look like?

Basically, the object returned from the scrollBehavior function should have some more parameters and change the current x, y positions into named parameters.

This is an example of the future object to be returned.

{
top: 2500, // this is the x value
left: 0, // this is the y value
behavior: 'smooth' //
};

This is getting implemented in most current browsers, couldn't find the spec thought. If anybody finds it please sahre the link.

Anyway, there is this beatyfull polyfill: https://www.npmjs.com/package/smoothscroll-polyfill

which adds this scrollTo({}) behaviour to old browsers.

I haven't dig much into the vue-router code. But it looks to me as if the only change needed is this file:
https://github.com/vuejs/vue-router/blob/dev/src/util/scroll.js

Maybe we could add some conditions on line 126 to return the current implementation or translate the old object:

{ x, y } to

{ top: x, left: y, behaviour: x }

Sorry if markup sucks, I'll do some more work if this gets atention

@posva
Copy link
Member

posva commented Feb 20, 2018

That's already possible with selector: https://router.vuejs.org/en/advanced/scroll-behavior.html

@posva posva closed this as completed Feb 20, 2018
@BorjaRafols
Copy link
Author

BorjaRafols commented Feb 20, 2018

That has nothing to do with what I'm saying.

Giving a selector will scroll to the desired DOMElement but doesn't apply any scrollBehaviour. Sorry if I didn't explain myself.

In this file: https://github.com/vuejs/vue-router/blob/dev/src/util/scroll.js

I'd like to change line 126 from

if (position) {
    window.scrollTo(position.x, position.y)
  }

to something like

if (position) {
    window.scrollTo({
      top:positon.x
      left: position.y,
     behavior: 'smooth'
  })
}

Obviously this would need to be parametrized.

This would apply a smooth scrolling in modern browser and apply a normal scroll like right now in older ones.

If you add the polyfill I said before then it smootly scrolls on every device.

@posva
Copy link
Member

posva commented Feb 20, 2018

Aaah, sorry, didn't understand it correctly.
In that case let's wait a bit until the thing becomes a standard so we can base ourselves on something that all browsers are going to follow 🙂

@BorjaRafols
Copy link
Author

Let me add this links I found. It's actually a CSS Property specification:

Firefox: https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-behavior
Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/status/cssomviewsmoothscrollapi/
Chrome: https://www.chromestatus.com/feature/5812155903377408
CSSOM: https://drafts.csswg.org/cssom-view/#smooth-scrolling

@posva
Copy link
Member

posva commented Feb 20, 2018

In the future I think it will be safe to pass any extra parameter to the scrollTo function while still using scrollTo(x, y) in older browsers, so this may work out of the box. I'll still have to figure out how to detect what syntax is supported by browsers

@BorjaRafols
Copy link
Author

Just looked at the source code of the polyfill I mentioned.

He uses this function, because as I said, this scrollBehaviour is actually a CSS prop.

https://github.com/iamdustan/smoothscroll/blob/master/src/smoothscroll.js

if ('scrollBehavior' in document.documentElement.style)

Should be feasable, I'll check this once I get home and can use IE7 :D

@posva
Copy link
Member

posva commented Feb 20, 2018

the last browser supported is ie9 lol

@BorjaRafols
Copy link
Author

This starts to look good, could you maybe reopen this issue?

IE9 -> scrollBehavior' in document.documentElement.style -> false;

But that doesn't really matter as scroll() is not available on IE9 anyway.

I'd love to help implement this one, but so far i've never commited anything to a public repo.

@posva posva changed the title scrollTo new spec implement scrollBehaviour pass on extra arguments to scrollTo to enable smooth scrolling with behaviour Feb 20, 2018
@posva
Copy link
Member

posva commented Feb 20, 2018

Ok, as long as the overhead is small enough to be worth adding 😉

@henryoliver
Copy link

Hey guys, follow the solution for any decent browser:

const router = new VueRouter({
    mode: 'history',
    routes: [
        { path: '/', name: 'home', component: Home },
        { path: '*', component: ClientError }
    ],
    scrollBehavior(to) {
        if (to.hash) {
            return window.scrollTo({ top: document.querySelector(to.hash).offsetTop, behavior: 'smooth' });
        }
        return window.scrollTo({ top: 0, behavior: 'smooth' });
    }
});

@posva posva added this to Long term road (high prio, low complex) in Longterm Mar 26, 2019
@SylverFox
Copy link

Please be advised that this will throw an error when the hash starts with a number or the selector does not exists. This can be fixed by using getElementById and checking for element existence.

scrollBehavior(to) {
    if (to.hash) {
        const element = document.getElementById(to.hash.slice(1));
        if (element) {
            return window.scrollTo({
                top: element.offsetTop,
                behavior: 'smooth'
            });
        }
    }
    return window.scrollTo({ top: 0, behavior: 'smooth' });
}

@ilyasmez
Copy link

ilyasmez commented Jul 22, 2019

@henryoliver @SylverFox

The solution you proposed doesn't make use of scrollBehavior, as you are always returning undefined (what window.scrollTo() returns) which tells the router to not scroll.

I think it makes more sense to have this logic in the afterEach hook.

Or at least don't return anything to avoid confusion:

scrollBehavior(to) {
  let top = 0;

  if (to.hash) {
    const element = document.querySelector(to.hash);
    if (element) {
      top = element.offsetTop;
    }
  }

  window.scrollTo({ top, behavior: 'smooth' });
}

@janswist
Copy link

janswist commented Nov 3, 2020

I see it there is freshly added support of smooth scroll in scrollBehavior. Can I ask for an example to make sure I use it properly (right now I'm not even sure if I do)? I use Nuxt 2.13.3

So far I found this to be working (based on @ilyasmez solution):

scrollBehavior: function (to) {
      if (to.hash) {
        setTimeout(() => {
          const element = document.querySelector(to.hash);
          window.scrollTo({
            top: element ? element.offsetTop : 0,
            behavior: 'smooth'
          })
        }, 500)
      }
    },

Btw, do I even need to make a Promise out of it like in here?

@ilyasmez
Copy link

ilyasmez commented Nov 4, 2020

@janswist

You don't have to use a setTimeout, scrollBehavior waits for the next tick anyway.
Just copy the code from my previous comment, and it should work.

If you upgrade to v3.4.8 (the version that includes the support of behavior) you can just do:

scrollBehavior (to) {
  return { selector: to.hash, behavior: 'smooth' };
}

You don't need to check if to.hash is defined or if it's a selector of a valid element, seems like it's internally done already.
I didn't test the code above :) so please take it with a grain of salt.

@posva are there any plans to update the documentation (API and guide)?

@janswist
Copy link

janswist commented Nov 4, 2020

@ilyasmez

Thanks! I made it like this now (because there was an error with not finding has in normal pages):

return { selector: to.hash || to, behavior: 'smooth' };

Also, it only works when I'm already on the target page. When I change page there is transition going on that takes 0.5s and that's why I need that timeout. Are there any listeners that can be used instead of timeout to wait for transition to finish?

Btw, how can I check router version? Because I'm using Nuxt I don't really get to see what's going inside.

@ilyasmez
Copy link

ilyasmez commented Nov 4, 2020

@janswist

return { selector: to.hash || to, behavior: 'smooth' };

to is an object, so not a valid selector. I don't think this will work.
The previous solution throw an error because, apparently, hash is an empty string when no hash is present (which is not a valid selector).

Also, it only works when I'm already on the target page. When I change page there is transition going on that takes 0.5s and that's why I need that timeout. Are there any listeners that can be used instead of timeout to wait for transition to finish?

Ah I see, I don't know which transition you're talking about exactly, if it's a custom one then there is no harm in using setTimeout, and you can just return a promise instead, check Scroll Behavior > Async Scrolling.
That would be useful when upgrading to the new version, but if you're using an older version it won't make a difference, as behavior is supported only in the latest version (v3.4.8).

Btw, how can I check router version? Because I'm using Nuxt I don't really get to see what's going inside.

If you're using Nuxt you have to wait for a release with the vue-router upgrade (keep an eye on the Nuxt releases page).

After the release your solution will look like this:

scrollBehavior (to) {
  return new Promise((resolve) => {
    setTimeout(() => resolve({ selector: to.hash, behavior: 'smooth' }), 500);
  });
},

But until then, I recommend sticking to your initial solution, it will work just fine.

@janswist
Copy link

janswist commented Nov 5, 2020

Thanks for such detailed answer!

I know it might not be the place, but how actually returning Promise work with router scrollBehavior? I understand Promise will wait for 'something', but what exactly? Initially, I thought it's waiting until all page transitions are done and then it starts to do it's thing. Description in the docs is really short and not saying that much either.

@posva posva moved this from Long term road (high prio, low complex) to Done in Longterm Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment