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

V5 Beta: Problems with options paddingTop and paddingBottom #1807

Closed acwolff closed 3 years ago

acwolff commented 3 years ago

I use on a PC a padding of 60px:

import PhotoSwipeLightbox from './res/photoswipe-lightbox.esm.js'; 
import PhotoSwipeDynamicCaption from './res/photoswipe-dynamic-caption-plugin.esm.js';
const options = {
  gallerySelector: '#gallery',
  childSelector: 'a',
  paddingTop: 60,
  paddingBottom: 60,

But this padding is not always used, see my test album. It is OK for a wide window, but if I make the window smaller, the padding disappears:

PC-wide

PC_small

On an iPhone, I like to use the whole screen for the image. To implement that I use this code:

lightbox.on('afterInit', () => {
    if (iPhone) {
      pswp.options.paddingTop= 0;
      pswp.options.paddingBottom= 0;
    }
})

where iPhone is set to true in $(document).ready(function()

However this does not work correctly on my iPhone, the bottom padding is too large, I think the image should reach the bottom:

iPhone-1

If I rotate the iPhone 90 degrees and rotate back, I see:

iPhone-2

So apparently the code given above has no effect at all.

Which code should I use to get this correctly working?

dimsemenov commented 3 years ago

You can do something like

lightbox.on('beforeResize', () => {
  const opts = lightbox.pswp.options;
  if (isIPhone) {
    opts.paddingTop = 0;
  } else {
    opts.paddingTop = 40;
  }
});

Plugins like dynamic-caption may also adjust this, so make sure the issue isn't coming from there.

acwolff commented 3 years ago

@dimemenov that works fine if the Dynamic-caption plugin is not used, but with the Dynamic caption it is not working.

So my next question is: How can I get it working with the Dynamic-caption plugin?

I tried to add too:

captionPlugin.on('beforeResize', () => {
    const opts = lightbox.pswp.options;
    if (iPhone) {
        opts.paddingTop = 0;
        opts.paddingBottom = 0;
    } else {
        opts.paddingTop = 60;
        opts.paddingBottom = 60;
    }
});

but that does not work (Uncaught TypeError: captionPlugin.on is not a function).

BTW: I am still testing with version 5.1.0, are you publishing a new version in short time? In that case I will wait for that new version, otherwise I will load tomorrow the latest version.

I am almost ready with testing, do you have any idea when you will release version 5? I do not want to release my application before you release version 5!

dimsemenov commented 3 years ago

So my next question is: How can I get it working with the Dynamic-caption plugin?

That's not something that's supported, you might want to look at mobile layout breakpoint option https://github.com/dimsemenov/photoswipe-dynamic-caption-plugin#mobilelayoutbreakpoint-600

I am still testing with version 5.1.0, are you publishing a new version in short time?

There are no breaking changes planned for now, I'm using v5 in two prod projects, so feel free to use it if you can't find any bugs.

acwolff commented 3 years ago

That's not something that's supported, you might want to look at mobile layout breakpoint option

That is a good advice, I use now this code:

lightbox.on('beforeResize', () => {
    const opts = lightbox.pswp.options;
    if (iPhone) {
        opts.paddingTop = 0;
        opts.paddingBottom = 0;
    } else {
        opts.paddingTop = 60;
        opts.paddingBottom = 60;
    }
});
const captionPlugin = new PhotoSwipeDynamicCaption(lightbox, {
    type: 'auto',
    mobileLayoutBreakpoint: 750,
    captionContent: (slide) => {
        try {
            return slide.data.element.dataset.caption;
        }
        catch(err) {
            return null; 
        }

    }
})

and that solves the problem!

Thanks you very much for your help!