metafizzy / flickity-fullscreen

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

Vanilla javascript fullscreenChange event only passes one argument to the handler #12

Closed gonssal closed 6 years ago

gonssal commented 6 years ago

It seems that the fullscreenChange event handler, when used in vanilla JS, only gets a single isFullscreen argument instead of the two documented in the readme. So I think it should be:

flkty.on( 'fullscreenChange', function( isFullscreen ) {...} );

Codepen: https://codepen.io/anon/pen/dKWaLy

Not sure about the jQuery namespaced event, but I think it's the same.

desandro commented 6 years ago

Hello! Thanks for reporting this issue. Actually, it's a bit confusing. Some Flickity events do have an event argument, which matches the jQuery arguments. But others, like fullscreenChange do not. It's documented as such:

// jQuery
$carousel.on( 'fullscreenChange.flickity', function( event, isFullscreen ) {...});

// vanilla JS
flkty.on( 'fullscreenChange', function( isFullscreen ) {...});
gonssal commented 6 years ago

I used this project's README as documentation, and it says:

flkty.on( 'fullscreenChange', function( event, isFullscreen ) {...} );

So I guess there's the issue. Thanks for answering.

desandro commented 6 years ago

Ah! Thanks for that. Fixed!