barbajs / barba

Create badass, fluid and smooth transitions between your website’s pages
https://barba.js.org/
MIT License
11.62k stars 474 forks source link

Browser loads new page if a link contains an anchor #53

Closed jdoubleu closed 6 years ago

jdoubleu commented 8 years ago

While using Barba I mentioned that my browser loads a new page when a link contains a hash (e.g. another-page.html#section1).

Scenario: I'm on page A.html. This page contains a link to B.html#anchor. If I click the link, the browser will load the page and not Barba. On page A.html is a second link A.html#somewhere. Clicking on this link should not trigger Barba but the browser to jump to #somewhere.

After investigating the source code I found out, that Barba doesn't handle clicks on links with a hash but letting the browser performing the event. See https://github.com/luruke/barba.js/blob/188de0364c1cc3e046fb389fdd36cb53303800bf/src/Pjax/Pjax.js#L190 which will check if Barba manages to get the new page or ignore the click. And here https://github.com/luruke/barba.js/blob/188de0364c1cc3e046fb389fdd36cb53303800bf/src/Pjax/Pjax.js#L232 it decides to ignore the request.

The comment on line 231 suggests that links with a hash shouldn't be handled by Barba because the browser should jump to the anchor. However there should be a check, that the target link equals the current link (without a hash). If it does not it's obviously another page.

But when changing this behavior how will be the anchor "jump" be handled? Maybe by adding the hash via window.location = "#hash" after the new page was loaded? This could be realized with Promises?

luruke commented 8 years ago

Hi @jdoubleu, I'm in vacation at the moment. I'll try to reply in the next few days

Joel-Mercier commented 8 years ago

👍 +1 noticed this as well

maxschulmeister commented 7 years ago

did anybody already write a workaround for this? Otherwise I'll give it a try. Saving the Hash, then removing it and apply it after transition sounds like a straight forward idea.

maxschulmeister commented 7 years ago

so the easiest workaround for this is editing barba.js on line 1341 from:

//Ignore case when a hash is being tacked on the current URL
        if (href.indexOf('#') > -1)
          return false;

to:

 //Ignore case only for same page hash urls, not for different page hash urls       
       if (location.href.indexOf(href) > -1 && href.indexOf('#') > -1)
          return false;

you might want to consider adding an option or replacing it with this, as using hashes for triggering other functions such as filters etc. is pretty common.

luruke commented 7 years ago

Hello @max-schu Actually it should already ignore link of the same page:

    //In case you're trying to load the same page
    if (Utils.cleanLink(href) == Utils.cleanLink(location.href))
      return false;

.cleanLink it removes the hash from an url.

Does it works for you?

maxschulmeister commented 7 years ago

hi @luruke,

yes it works perfectly fine. perhaps url.split('#')[0]; instead of url.replace(/#.*/, ''); is a better option performance wise, but im not sure about that.

the reason of my workaround is another one tho.

e.g. In my project i have an anchor in index.html which links me to "shop.html#gold", the reason i'm doing this is, because #gold triggers a filter function, which only show me products of the "gold" collection. However with

 if (href.indexOf('#') > -1)
          return false;

the href="shop.html#gold" is ignored by barba.js While i think its good to ignore urls with a hash on the same page (for scrolling to anchors) it should not ignore hrefs that link to another page with a hash. Thats why i changed it, so it only returns flase, if the href contain a hash AND links to the same page

you get my point?

awesome module btw 🔥

timharwood commented 7 years ago

+1 for this issue (and for the awesome module comment)

luruke commented 7 years ago

@max-schu Sorry for the late reply, I've been very busy. Yes, it makes sense. But if you are using standard Fragment identifier the browser will not scroll to the specified element if the transition is made with PJAX.

In your case I guess you're detecting the filter on the page shop.html via JS, right? If so, it would make more sense using a get parameter? no?

maxschulmeister commented 7 years ago

totally makes sense. didnt see it that way. I'm sure there a way to use get parameters, im just not quite familliar with these so i decided to use hashes. However maybe it's an option to manually execute scrolling to an anchor after the page transition is done and url contains a hash. After all this would be a rather specific improvment, i'm sure anyone facing this issue, can make his way through with a workaround.

luruke commented 7 years ago

Yep, scrolling to the element could be a solution. But I would like to go through the specification to understand better how needs to work in specific cases (for example what if the element is hidden and etc.).

I will face this thing on the new release :)

Thank you again

AlexandraKlein commented 7 years ago

+1 with Isotope filtering.

<a href="/work#filter=.filter-item--vfx" tabindex="0">More VFX</a>

Anchor to go to work page filtered already by items that are only "VFX".

Barba ignores this and reloads the page.

AlexandraKlein commented 7 years ago

The following solved the issue for me:

 Barba.Pjax.originalPreventCheck = Barba.Pjax.preventCheck;

    Barba.Pjax.preventCheck = function(evt, element) {
        if ($(element).attr('href') && $(element).attr('href').indexOf('#') > -1)
            return true;
        else
            return Barba.Pjax.originalPreventCheck(evt, element)
    };
markus-kre commented 7 years ago

Is there any update or new ideas on this issue..? I tried all posted proposals and nothing worked for me! Thanks

AndreNolden commented 6 years ago

I worked around with a little function. So because href="/#myanchor" doesn´t work with barba.js as described above, I only link to / and then scroll to anchor after timeout.

<a href="/" onclick="pageScroll('myanchor')">

function pageScroll(x) {

  setTimeout(function () {
    $('html, body').animate({
          scrollTop: $('#'+x).offset().top
      }, 1000);
  }, 1000);

}
Spica2 commented 6 years ago

+1 noticed this as well

Kwapi commented 6 years ago

Same here

HectorLS commented 6 years ago

@AlexandraKlein solution works sweet for me, then i manage by my own the scrollTo if there is a hash in the URL, thank you so much

ryandc commented 6 years ago

@HectorLS would you be able to share your solution Hector? Getting this working right and finding the right solution is proving a little illusive to me!

deanc commented 5 years ago

Here's a non-jQuery fix based off @AlexandraKlein 's code:

    Barba.Pjax.originalPreventCheck = Barba.Pjax.preventCheck;

    Barba.Pjax.preventCheck = function(evt, element) {
      if (
        element.getAttribute("href") &&
        element.getAttribute("href").indexOf("#") > -1
      )
        return true;
      else return Barba.Pjax.originalPreventCheck(evt, element);
    };
ekfuhrmann commented 5 years ago

@deanc Solution works great, but I was getting errors when element was null. As such, I made a slight tweak that checked for element in order to clean up any errors while still preserving the overall fix.

Barba.Pjax.originalPreventCheck = Barba.Pjax.preventCheck;

Barba.Pjax.preventCheck = function(evt, element) {
  if (
    element &&
    element.getAttribute('href') &&
    element.getAttribute('href').indexOf('#') > -1
  )
    return true;
  else return Barba.Pjax.originalPreventCheck(evt, element);
};
panayotoff commented 4 years ago

Here is what I do ( barba2 )

barba.hooks.beforeEnter((data) => { // Also update menu here updateHardReloadLinks(data.next.href); });

    /**
     * Updates links that point to the current page
     */
    function updateHardReloadLinks(url) {
      const currentUrl = url || window.location.href;

      Array.from(
        document.querySelectorAll('[data-barba-prevent]')
      ).forEach((link) => link.removeAttribute('data-barba-prevent'));

      Array.from(document.querySelectorAll('a')).forEach((link) => {
        if (barba.url.clean(link.href) === barba.url.clean(currentUrl))
          link.setAttribute('data-barba-prevent', true);
      });
    }

    /**
     * Prevent Barba hard reload
     */
    document.addEventListener('click', (event) => {
      if (event.target.closest('[data-barba-prevent]')) {
        event.preventDefault();
      }
    });
   updateHardReloadLinks()
milovincent commented 3 years ago

this seems to still not be working. none of these workarounds seem to function either. are there any other, similar libraries for this stuff i can use instead?

ryandc commented 3 years ago

https://swup.js.org/

On Wed, Jan 20, 2021 at 10:17 PM Milo Szecket notifications@github.com wrote:

this seems to still not be working. none of these workarounds seem to function either. are there any other, similar libraries for this stuff i can use instead?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/barbajs/barba/issues/53#issuecomment-763986744, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGM6P52ITYCATL4IMJV4LTS25I7XANCNFSM4CM5D6IQ .

nedzen commented 3 years ago

I had the same (or similar) issue and the solution was to timeout the changing of the url#hash.

// without setTimeout it throws an error
      setTimeout(function () {
        // set the hash
        if (storedHash) location.hash = storedHash;
      }, 100 );

See error below:

Uncaught RangeError: Maximum call stack size exceeded.
    at t.e.force (barba.umd.js:1)
    at t.e.go (barba.umd.js:1)
    at t.e.D (barba.umd.js:1)
    at t.e.force (barba.umd.js:1)
    at t.e.go (barba.umd.js:1)
    at t.e.D (barba.umd.js:1)
    at t.e.force (barba.umd.js:1)
    at t.e.go (barba.umd.js:1)
    at t.e.D (barba.umd.js:1)
    at t.e.force (barba.umd.js:1)

Solution

{
    namespace: "clippings",

    afterEnter(data) {
      const cliplnk = document.querySelectorAll('a.timestamp');

      cliplnk.forEach(  el => el.addEventListener("click", () => {
          // Store data
          let hash = el.id;
          localStorage.setItem('anchor', hash);
        }
      ));

      // Get data
      const storedHash = localStorage.getItem('anchor');

      // without setTimeout it throws an error
      setTimeout(function () {
        // set the hash
        if (storedHash) location.hash = storedHash;
      }, 100 );

      setTimeout(function () {
        localStorage.removeItem('anchor');
        console.log('cleared hash');
      }, 500 );
    },
  }
xavierfoucrier commented 1 year ago

FYI, a recent similar issue (not fixed yet) recently happen to another developer.

See https://github.com/barbajs/barba/issues/720.