dimsemenov / PhotoSwipe

JavaScript image gallery for mobile and desktop, modular, framework independent
http://photoswipe.com
MIT License
24.27k stars 3.31k forks source link

Script gets broken when swipe inside next/prev buttons #1075

Open maxceem opened 8 years ago

maxceem commented 8 years ago

This bug can be reproduced here in the demo gallery http://photoswipe.com/

  1. Run gallery by clicking any preview
  2. Swipe inside of "previous" or "next" arrow button to change slides. So you should start and finish swipe guesture inside button which has quite enough invisible size to do this (you can focus a button to see it's dimensions).
  3. After 2-3 swipes in such way, two errors will appear in console one by one for different swipes. Uncaught TypeError: Cannot read property 'center' of undefined (line: _panOffset.x = _currPanBounds.center.x;) Uncaught TypeError: Cannot read property 'lastChild' of null (line: img = item.container.lastChild;)
  4. If you continue swiping this way up to 5 swipes, images will desappear and cannot be shown anymore no matter which way we change slides now. Only caption will continue changing.

I've found this bug by making prev/next buttons which take the whole screen like facebook does for it's slideshows.

SebastianSchirmer commented 8 years ago

Just installed photoswipe and angular-photoswipe (https://github.com/m00s/angular-photoswipe) as we would like to use this awesome lib in an cordova/ionic v1 project. After including the code initially, we are receiving this error as well, however without pagination buttons. It seems to appear constantly, even after some seconds without interacting with the app. Will try to provide more details.

bummzack commented 8 years ago

Getting the same problem when trying to have a large clickable area for the prev/next buttons (eg. covering left and right half of the screen).

Swiping causes the issue described by @maxceem

Dacesilian commented 7 years ago

I can confirm this bug, error is in _setImageSize: line 2895 Uncaught TypeError: Cannot read property 'lastChild' of undefined(…)

This is solution:

if(typeof item.container === 'undefined'){
    return;
}
7iomka commented 7 years ago

I do not see this bug in the new version (v 5.0) http://photoswipe.com/test/photoswipe.min.js http://photoswipe.com/test/photoswipe-ui.min.js

nilebma commented 7 years ago

UPDATE : the following solution does not fix the problem after all, see my next post below.

To fix this pb, I just set up a little before the two problematic lines (around line 1068, on photoswipe.js version 4.1.1).

`

 updateCurrZoomItem: function(emulateSetContent) {

     //SOME CODE

     //THE TEST I ADDED
     if(_currPanBounds.hasOwnProperty('center') && _currPanBounds.center){

         // THE TWO BUGGY LINES
         _panOffset.x = _currPanBounds.center.x;
         _panOffset.y = _currPanBounds.center.y;

      }

    //SOME CODE
}

`

I don't know if my modifications can break something elsewhere, so far so good on my side.

nilebma commented 7 years ago

My previous solution does just hide the error message in the console, but still some problems exist and I end up with missing images in my gallery.

The scenario where this problem happens is the following :

So I think this problem occurs when the gallery has not been initialised yet (in the DOM ?) and that the goTo(imageIndex) function is called on this gallery.

I tried to wait for the afterInit event before running the command but the problem persists. My ugly solution is to wait a 4s delay before doing any goTo() call in my code.

Maybe a solution would be to implement a new event to be sure that the gallery has been completely initialized (in particular when everything in the DOM is ready maybe ?).

BTW thanks for all the great work on this library 👍

chrisjung-dev commented 6 years ago

Had this problem with the buttons in FF as well. Seems like an issue with getting the index from DOM. Parsing the index number string from DOM to an int, everything is fine.

    const clickThumbnail = (e) => {
        e = e || window.event;
        e.preventDefault ? e.preventDefault() : e.returnValue = false;
        const eTarget = e.target || e.srcElement;

        // this is not working
        // const index = eTarget.parentNode.dataset.index;

        // this is...
        const index = parseInt(eTarget.parentNode.dataset.index, 10);

        openLightbox(index)
    };
florianmutter commented 5 years ago

I have the same issue as @nilebma. Calling next() too fast also causes that issue. So the problem cannot be solved by passing a number instead of a string. I tried waiting for the image imageLoadComplete event but that does not seem to help.