PhotoSwipe icon indicating copy to clipboard operation
PhotoSwipe copied to clipboard

PhotoSwipe with turbolink causes page to refresh after close

Open chamnap opened this issue 9 years ago • 16 comments

Firstly, I would like to say thanks to your hard work on this awesome library.

I'm integrate this library on my rails 4 project with turbolinks. Everything works expected except when I close the popup, the page is refreshed. However, if I take the turbolink library out, it looks fine. I'm not sure what happen, and what I should do. See this, http://www.snapyshop.com/products/54a2f6886c6936668d380000

chamnap avatar Dec 30 '14 19:12 chamnap

Hard to tell. Get unminified version of PhotoSwipe and debug what line causes the issue, function returnToOriginal (https://github.com/dimsemenov/PhotoSwipe/blob/master/src/js/history.js#L167) is executed after gallery is closed. Or you can just disable history (history:false).

dimsemenov avatar Dec 30 '14 20:12 dimsemenov

@chamnap Were you able to figure out a solution for this besides removing turbolinks? I have the same issue right now.

mitchnick avatar Jan 14 '15 16:01 mitchnick

@chamnap @mitchnick I'm experiencing the same just now. Haven't had a chance to dig in yet (initial experimenting with PS for our app). Have you figured anything out?

inlikealion avatar Mar 17 '15 19:03 inlikealion

To whom who still struggle with this issue. I was able to fix it by changing this line : https://github.com/dimsemenov/PhotoSwipe/blob/master/dist/photoswipe.js#L3620

if(_urlChangedOnce) {
    history.back();
}

to :

if(_urlChangedOnce) {
    history.pushState("", document.title,  _windowLoc.pathname + _windowLoc.search );
}

Turbolinks seems to use its own pushState, so it will avoid conflicting with the native one.

abdelbk avatar Jul 16 '15 13:07 abdelbk

@abdelbk :+1:

caiotarifa avatar Jul 25 '15 00:07 caiotarifa

What is the status of this? I am using the photoswipe-rails gem and have had to deactivate history for now. I could fork the gem but be great to get this fixed in photoswipe as I suspect a lot of people are having this issue.

patricklindsay avatar Oct 21 '15 23:10 patricklindsay

@patricklindsay A pull request is already opened to fix this issue in the photoswipe-rails gem. However, the owner of the gem believes that the fix should be made directly into photoswipe.

abdelbk avatar Oct 21 '15 23:10 abdelbk

Yes I can see that, which is why I'm curious to see if the fix will be made here :)

patricklindsay avatar Oct 21 '15 23:10 patricklindsay

@patricklindsay, @abdelbk, I doubt that I should introduce any platform-related (think: RoR, Django, etc.) patches/changes in photoswipe-rails (mainly because I'm not sure that I'll be able to keep up with PhotoSwipe changes & manually/semi-automatically patch photoswipe-rails every time), that is all. I just won't have the time to go over every new release.

skakri avatar Oct 22 '15 00:10 skakri

The reason why I don't want to merge suggested edits by @abdelbk, is that it creates new history record, instead of removing one after gallery is closed. So if user opens gallery, closes it, then again opens and closes, It'll take 5 "back" button clicks to go to the previous page.

Besides that, developer of a site would have to add additional checks to find out if PhotoSwipe URL (&gid=XX&pidXX) was set on purpose to open gallery, or by mistake when going "back".

What might work is changing all occurences of pushState with replaceState in history.js (it would, of couse, break "back button to close" feature). We can add some option in history.js that would control which method should be used.

I'd appreciate if anyone who works a lot with rails would send pull request that adds such option, or I'll implement it by myself. Does Rails have a method that would emulate history.back without breaking anything?

dimsemenov avatar Oct 22 '15 06:10 dimsemenov

@dimsemenov I doubt that. Turbolinks is pretty annoying. As stated in the documentation (https://github.com/rails/turbolinks/blob/master/README.md#events), The back button can't be caught to prevent page reload. I believe that the only current solution is to follow your suggestion and add an additional option.

abdelbk avatar Oct 25 '15 16:10 abdelbk

:+1:

patricklindsay avatar Oct 26 '15 12:10 patricklindsay

@dimsemenov Has there been any progress on this? I've just hit this exactly and am unsure what the best option is at this point?

rctneil avatar Apr 25 '20 10:04 rctneil

+1 I am still running into this as well

Lordnibbler avatar Apr 26 '20 07:04 Lordnibbler

I am still hitting this issue. I did post about it in the Turbolinks repo here: https://github.com/turbolinks/turbolinks/issues/549

It sounds like the history entry needs to be in a format that Turbolinks understands (for whatever reason that is) and the example I was shown was:

Turbolinks
    .controller
    .pushHistoryWithLocationAndRestorationIdentifier(path, Turbolinks.uuid())

Anyone able to tweak Photoswipe to allow this method?

rctneil avatar Aug 12 '20 21:08 rctneil

if(_urlChangedOnce) {
    history.pushState("", document.title,  _windowLoc.pathname + _windowLoc.search );
}

This fix is absolutely perfect decision for me!

vladtabachuk avatar Feb 27 '21 10:02 vladtabachuk