fancyapps / ui

A library of JavaScript UI components, includes the best lightbox - Fancybox5
https://fancyapps.com/
Other
785 stars 98 forks source link

After closing via button or swipe-up gives duplicate url history #264

Closed PimHaarsma closed 6 months ago

PimHaarsma commented 2 years ago

Hello! Thanks for the great plugin!

I found a small annoying bug. To reproduce:

ChiaYuSu commented 2 years ago

I have the same question too.

ChiaYuSu commented 2 years ago

Hi, @PimHaarsma. I fixed the problem.

You can add this script $.fancybox.defaults.hash=false in the javascript.

It works as I tried in the console to set the hash to false.

Ref : https://fancyapps.com/docs/ui/fancybox/plugins/hash

PimHaarsma commented 2 years ago

Hello @ChiaYuSu. Thanks for the workaround! I like the hash-plugin, but I will use this fix, until the problem is fixed.

fancyapps commented 2 years ago

Hi,

Sorry to disappoint, but it won't be fixed soon (if ever). I've made several attempts to create a hash plugin that would work perfectly in all cases, but it's like an impossible mission. I will try to explain. You might think that it would be easy to implement it, because all you have to do is to write smth like this:

if (user_presses_back_button() and fancybox_is_open()) {
  close_fancybox();
  prevent_user_from_navigating_back();
}

And the problem is - 1) you can't detect if the user actually presses the back button; 2) you can not prevent navigation. So what can you do? Well, you can observe URL hash change and create/replace history record. And as you create listeners for Fancybox close event and hash change, then there is a good chance that they will trigger each other.

I'm currently working on v5 and will definitely try to solve this puzzle again when it's time to rewrite this plugin (to TypeScript).

snitchyuk commented 2 years ago

I'm not an coder so please ignore me if this is a stupid suggestion - but as pressing the browsers back button results in closing the gallery, could the close button in the gallery simply be changed to something like "history.back()"? Wouldn't this result in the gallery closing, and then the back button working as expected?

fancyapps commented 2 years ago

@snitchyuk Unfortunately, you can not know what exactly happens. You can just observe URL hash changes, watch for onpopstate events and something like that. And there can be various scenarios. For example, user might change URL hash value manually. Then, if the code contains history.back(), browser would navigate unexpectedly.

9mm commented 1 year ago

I definitely agree this is painful. @fancyapps at the very least, perhaps you can offer an option to use history replace instead of history push, so at the very least when they open an image, and close on the image, and then hit back, it doesnt cause them to click a bunch of times. But then, at the very least, it would still work if someone wants to deep-link a URL to an image, thats the main purpose of me using it is so when someone copies the URL and another person clicks, it automatically opens the image. I actually dont like the history aspect because theres tons of images and it makes navigating back painful

CodeTappert commented 1 year ago

I definitely agree this is painful. @fancyapps at the very least, perhaps you can offer an option to use history replace instead of history push, so at the very least when they open an image, and close on the image, and then hit back, it doesnt cause them to click a bunch of times. But then, at the very least, it would still work if someone wants to deep-link a URL to an image, thats the main purpose of me using it is so when someone copies the URL and another person clicks, it automatically opens the image. I actually dont like the history aspect because theres tons of images and it makes navigating back painful

This sounds like a very good solution to the problem. I second this. Would be really nice feature to have the option to choose

lubomirblazekcz commented 9 months ago
window.addEventListener('popstate', () => {
    window._fancyboxClosedWithPopstate = true
})

Fancybox.bind('[data-fancybox]', {
    on: {
        ready: () => {
            window._fancyboxClosedWithPopstate = false
        },
        shouldClose: (_, e) => {
            if (!window._fancyboxClosedWithPopstate) {
                e.preventDefault()
                window.history.back()
            }
        }
    }
})

This is my solution, every closing event with fancybox does window.history.back(), so the browser navigation works as expected. It's a bit hacky because shouldClose doesn't return which action triggered close event.

fancyapps commented 9 months ago

Hello to all!

The Hash plugin has been redesigned in the latest release. It works a little better, but unfortunately still not perfect. Sorry, but I don't think it's technically possible to get this to work reliably in any situation. Therefore, in future releases, the Hash plugin will not be included in the core code, but will be provided as an option.

fancyapps commented 6 months ago

Well, since there don't seem to be any new reports of issues with this, I assume it's working fine now.