metafizzy / flickity-fullscreen

Enable fullscreen view for Flickity carousels
46 stars 17 forks source link

Clicking fullscreen button fires click event on button beneath #8

Closed nroyall closed 6 years ago

nroyall commented 6 years ago

Hi! Thanks for the fantastic carousel! :)

I'm using the fullscreen option, and noticed a small issue with the button. I'm not sure whether it's an issue with the fullscreen JS or not.

When the fullscreen gallery is open, if you click on .flickity-fullscreen-button-exit, the fullscreen overlay closes and any button that's directly beneath the flickity button also receives a click event. So for example, I have a click event handler attached to a search button, placed directly beneath the flickity button, which triggers a click on the search button too.

It seems like the fullscreen positioning changes before the click event is finished, and exposes the element beneath to the click. Is there any way to prevent this?

screen shot 2018-05-10 at 3 19 13 pm

desandro commented 6 years ago

Thanks for reporting this issue. I'm having trouble reproducing it. Could you fork this demo to show this issue: https://codepen.io/desandro/pen/MGVROW

nroyall commented 6 years ago

Ah - it looks like it only happens on touch devices. If you select any touch device in the Chrome emulator (ie anything but the Laptop with HiDPI) the button click is triggered. I tested iPhoneX on BrowserStack, and my Nexus 5.

I added a color change here to make it easier to test on mobile, but the click is triggered in both pens. https://codepen.io/naomiroyall/pen/jxxGLW

desandro commented 6 years ago

Thank you for clearing that up! I'll have a fix in the next release.

In the meantime, you can resolve this bug by adding this duck punch

var exitButton = flkty.exitFullscreenButton;
exitButton.off( 'tap', exitButton.onTap );

See demo https://codepen.io/desandro/live/MGVROW

What's happening here: Flickity uses another library tap-listener to listen for click-like events for touch devices. This was necessary back in 2015, when mobile browsers had a 300 ms delay for emulated click events. Shortly after Flickity was released, browsers removed the delay and so tap-listener was not necessary any more (I now know).

So Flickity uses this tap event to hide the overlay. Then the emulated click event fires without the overlay, thus triggering the click event on any underlying element.

The fix is to remove the tap-listener tap event and use the click event, so the overlay is still present when that fires.

nroyall commented 6 years ago

Okay, great, that makes sense, thank you!

desandro commented 6 years ago

flickity-fullscreen v1.1.0 has been released. With this fixed added. Thanks again for reporting this issue 🌈🐻

jkosonen commented 5 years ago

I'm still having this issue with v 1.1.1, also same issue with the original flickity package.