swup / scroll-plugin Goto Github PK

• Currently, shouldResetScrollPosition passes in a link element
• We should make sure this still works for form requests or API requests
• The API of ignoreVisit probably makes sense to emulate here

options = {
  shouldResetScrollPosition: (url: string, { el, event }: { el?: Element; event?: Event }) => boolean
}

How important is this feature to you?

• Nice to have
• Would make my life a lot easier
• I cannot use swup without it

Hi,

Loving the new plugin setup, got upgraded from v1 and sorted really quickly. Just one outstanding issue, I am using the smooth scroll plugin on the site, but don't want to use it for some specific links. This is the code I used to achieve this with v1:

// Don't smooth scroll gallery filter links

  var galleryLink = document.querySelectorAll('a.filter-link');

  for (var i = 0, len = galleryLink.length; i < len; i++) {
    galleryLink[i].addEventListener('click', function(){
      swup.options.scroll = false;
    });
  }

document.addEventListener('swup:animationInDone', function() {
    swup.options.scroll = true;
});

What is the best option in Swup v2 for excluding specific links from the scroll behaviour?

Thanks - really enjoying working with Swup :)

Describe the problem

When clicking a hash link on a page, swup scrolls to the matching target element. But the :target selector doesn't have any effect. This example will paint the element #my-element red without swup enabled, but won't do anything with swup.

<a href="#element">Scroll to element</a>

<div id="element">Element</div>

<style>
  :target {
    background: red;
  }
</style>

Describe the proposed solution

From the discussion of the original issue on the core repo:

Seems like fitting aria-current values could be location or simply true. I worry that using that attribute could interfere with consumer-scripts, though (they may be using the aria-current attribute already to mark an active tab or the like).

Maybe a simple swup-scroll-target or swup-current-target or swup-hash-target attribute would be less intrusive? In that case, it should be part of the Scroll Plugin, as you first suggested.

Research

A few links with prior discussions or solutions:

• Discussion on html working group
• Test case for :target behavior on pushstate
• Test case for dynamic update of :target style
• Example jQuery plugin for emulating targetchange event

How important is this feature to you?

• Nice to have
• Would make my life a lot easier
• I cannot use swup without it

It would be nice if we had more fine-grained control over the behavior of animateScroll. For example, animating the scroll on samePage and samePageWithHash might be desired, but animating it on contentReplaced could not be wanted.

I have been able to hack this behavior together (smooth scrolling for samePage and samePageWithHash, no smooth scrolling for regular transitions) like this:

// safe the scrollPlugin instance, so we can access it later in the livecycle:
const scrollPlugin = new SwupScrollPlugin({
  animateScroll: false
});

// instantiate swup with the scrollPlugin instance:
const swup = new Swup({
  // ...
  plugins: [
    scrollPlugin,
  ]
});

// Disable the handlers for scrollPlugin by default:
swup.off('samePage', scrollPlugin.onSamePage);
swup.off('samePageWithHash', scrollPlugin.onSamePageWithHash);

// opt-in to smooth scrolling for `samePage` and `samePageWithHash`:
swup.on('samePage', e => {
  scrollPlugin.options.animateScroll = true
  scrollPlugin.onSamePage()
});
swup.on('samePageWithHash', e => {
  scrollPlugin.options.animateScroll = true
  scrollPlugin.onSamePageWithHash(e)
});

// reset animateScroll each time after a scroll
swup.on('scrollDone', e => {
  scrollPlugin.options.animateScroll = false
});

I could imagine an API like this:

const swup = new Swup({
  // ...
  plugins: [
    new SwupScrollPlugin({
      animateScroll: false,
      animateScrollSamePage: true,
      animateScrollSamePageWithHash: true,
    }),
  ]
});

Or, using an optional object for animateScroll:

const swup = new Swup({
  // ...
  plugins: [
    new SwupScrollPlugin({
      animateScroll: {
        default: false,
        samePage: true,
        samePageWithHash: true
      },
    }),
  ]
});

What would you prefer? I would try to put together a PR if you think this makes sense.

Before creating this issue, did you think of...:

• Have you checked closed issues for similar/related problems.
• Have you provided all helpful information available?
• Have you considered creating a demo so we can help you better?

Some of the sites I build make heavy use of overflowing divs. A good example where I used my own home-grown swup-like library: https://junge-akademie.adk.de/en/

Underneath the large teaser on the front page, two large overflowing divs scroll into view. Now if you scroll one or both of these divs down a bit, then navigate to one of the links and go back to the front page through the browsers back button, the div's previous scroll positions get restored automatically.

I would love to have this kind of feature available in swup, as well. Maybe the scroll plugin would be a good place to implement this? If you agree, I would try to implement it.

In the above example, I added a custom attribute [data-scrollable-id] to the overflowing divs, cached their scroll position and restored it on popstate. For swup, I could imagine an attribute like [data-swup-restore-scroll] and detecting popstate navigation on contentReplaced, which has an argument popstate for detecting navigation through the history API.

Before creating this issue, did you think of...:

• Have you checked closed issues for similar/related problems.
• Have you provided all helpful information available?
• Have you considered creating a demo so we can help you better?

I believe that in a previous version of this plugin the scroll position was always set to the top of the screen when navigating with the back button.

I have tried to set shouldResetScrollPosition: true on a new site I'm working on, but when navigating with the back button the scroll position is always remembered.

I have a scroll based animation on the homepage of a site that I always need to start at the top, so ideally I would like to completely disable the scroll restoration when navigating with the back button.

Current config

const swup = new Swup({
animateHistoryBrowsing: true,
plugins: [
new SwupJsPlugin(options),
new SwupBodyClassPlugin(),
new SwupScrollPlugin({
animateScroll: false,
doScrollingRightAway: false,
shouldResetScrollPosition: true,
}),
],
});

Is this possible to do? Thanks in advance.

Description of the issue

With the switch to scroll:top and scroll:anchor, we've lost the ability to distinguish top/anchor scrolling on the same page vs. top/anchor scrolling after a page load. Inside the scroll:* handler, there is no way of knowing if the scrolling is part of a visit or a same-page hash-scroll. This means that the animateScroll option is applied incorrectly.

How to reproduce the issue

Set animateScroll to non-equal values:

animateScroll: {
  betweenPages: false,
  samePageWithHash: true,
  samePage: true
}

Describe the problem

One cannot opt out of scrolling for individual links in a reliable way.

For example:

• Upon first use, shouldResetScrollPosition: (link) => !link.matches('.pls-restore') scrolls to the top if the scroll position before was top
• Even if it 'restores' scroll position, it still scrolls there - I'd like it to just remain at its position.

Describe the proposed solution

It would be great to have data-swup-no-scroll attribute tell the plugin to stay put and not scroll at all (being equivalent to not being used / vanilla swup behavior).

Alternatives considered

See #3 for an alternative (yet overly complicated) implementation.

How important is this feature to you?

• Nice to have
• Would make my life a lot easier
• I cannot use swup without it

What did you expect? 🧐

I would expect the following code to contain the correct visit.from.url and visit.to.url values from the latest page change:

swup.hooks.on('scroll:end', (visit) => {
	console.log(visit);
});

What actually happened? 😵‍💫

The visit.from.url and visit.to.url values are always the same, even if scrolling on the current page, or between different pages.

The values also appear incorrect, for example, if I navigate from / to /enquire, both visit.from.url and visit.to.url values are /. If I then navigate from /enquire to /, both visit.from.url and visit.to.url values are still /. Clicking on a link that has the same href value as the current page, also does not output the correct visit.from.url and visit.to.url values, in such case I would expect the same values.

Screen recording: https://cloud.cdrs.dev/88nSD2Gw

Swup and plugin versions 📝

@swup/body-class-plugin: 3.2.0
@swup/overlay-theme: 2.0.0
@swup/preload-plugin: 3.2.10
@swup/progress-plugin: 3.2.0
@swup/scripts-plugin: 2.1.0
@swup/scroll-plugin: 3.3.2
swup: 4.6.1

What browsers are you seeing the problem on? 🧭

Firefox, Chrome, Safari, Microsoft Edge

Relevant log output 🤓

animation: {animate: true, wait: false, name: undefined, native: false, scope: 'html', …}
cache: {read: true, write: true}
containers: ['#swup']
from: {url: '/'}
history: {action: 'push', popstate: false, direction: undefined}
id: 0.6330202602352362
scroll: {reset: true, target: undefined}
state: 1
to: {url: '/', hash: undefined}
trigger: {el: undefined, event: undefined}

URL to minimal reproduction 🔗

Note: For some reason the console output is not doing anything (sorry, I am new to online sandboxes)

https://codesandbox.io/p/devbox/empty-hill-kgn68f?workspaceId=6b038d9d-3724-456e-99a2-52aa6d31cc98

Checked all these? 📚

• I have read and searched the official docs
• I have checked discussions and existing issues for related problems
• I agree to follow this project's Code of Conduct
• I have provided all necessary information to the best of my knowledge

Describe the feature

• Add a visit.scroll.animate key that gets set right on visit:start by this plugin
• Allow other plugins (mainly a11y-plugin) to unset the flag and disable animations

Alternatives considered

• Use the existing suggested solution with matchMedia
• As a user, I'd prefer having an option on the a11y-plugin to group such behavior in one place

How important is this feature to you?

• Nice to have
• Would make my life a lot easier
• I cannot use swup without it

Hi Georgy, congrats on the refactor of Swup — it appears really well thought out! I really like the plugin architecture especially.

I've just built a new site using this version, but in testing have noticed that when using an anchor link from another page the Scroll Plugin isn't hitting the correct spot in Firefox. It scrolls to the correct position in Safari and Edge.

I'm using the following options (the issue occurs with doScrollingRightAway set to false as well):

    new SwupScrollPlugin({
      doScrollingRightAway: true, // default false
      scrollFriction: .35, // default 0.3
      scrollAcceleration: 0.04, // default 0.04
    })

Could this be a bug, or is there some sort of user error on my behalf I'm missing? Happy to provide a link to my staging site if that's helpful.

Thanks for your help.

Hi Georgy,

First of all. Great stuff on this plugin, it is really easy to use and works awesome!

I am trying to set animateScroll to false to disable the scrolling animation when navigating to another page.

However, it doesn't seem to work for me, no matter the value I use. The pages are always scrolling when I navigate to one and another as soon as I have a CTA that is positioned lower on a page.

My configuration is quite simple:

const swupOptions = {
    containers: ['#container'],
    plugins: [
        new SwupDebugPlugin(),
        new SwupBodyClassPlugin(),
        new SwupPreloadPlugin(),
        new SwupScrollPlugin({
            doScrollingRightAway: true,
            animateScroll: false,
            scrollFriction: 0.3,
            scrollAcceleration: 0.04
        }),
        new SwupJsPlugin(swupTransitions)
    ]
};

const swup = new Swup(swupOptions);

Thank you again for the plugin and any help!

While cleaning up I noticed that the docs about swup.scrollTo() are outdated:

https://swup.js.org/api/methods#scrollto

Which lead me to think closer about it. The current implementation in Scroll Plugin allows setting the animated argument, which doesn't really make sense here, since animating the scroll position would be the whole point of using the method in the first place.

We should probably rather discuss if we want to allow targeting a specific element to be scrolled, something like this (Pseudo Code):

swup.scrollTo(2000, {target: window|HtmlElement})

Making this possible would need enhancements of gmrchk/scrl, to add the possibility to target and handle specific elements other than window.

This is somewhat related to #26 .

If I click on a link to load new page, and once the page is loaded, it will keep scroll position of old page.
Meaning, if my button is somewhere in middle or bottom of the 1st page, and I click it, the new page will be scrolled down already...
Is this by default?

PS.
I'm able to fix this with window.scrollTo(0, 0); when refreshing JS with swup events...but I want to understand if this is by purpose or Im doing something wrong, cause I cant see why would it keep scroll position