dimsemenov / PhotoSwipe

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

v5-Beta Option to prevent overlapping captions. #1778

Open acwolff opened 3 years ago

acwolff commented 3 years ago

I did implement a caption as you proposed in the Caption document, but I removed the max-width in the css. To see the result, click the first thumbnail of my test album.

I would like to offer the option to prevent that the caption overlaps the image, so the height of the image should be reduced.

I have no idea whether I can implement this with my own extra code, I think this will require a change in the kernel.

If possible please add this option or show me how I can do it without changing the kernel.

amkdev commented 3 years ago

you can use the padding options

const lightbox = new PhotoSwipeLightbox({
  // may select multiple "galleries"
  gallerySelector: '#gallery--simple',

  // Elements within gallerySelector (slides)
  childSelector: 'a',

    paddingLeft: 0,
    paddingRight: 0,
    paddingTop: 0,
    paddingBottom: 32,    

  // Include PhotoSwipe Core
  // and use absolute path (that starts with http(s)://)
  pswpModule: '/v5/photoswipe/photoswipe.esm.js',

  // Include CSS file,
  // (if you haven't included in via <link>)
  pswpCSS: '/v5/photoswipe/photoswipe.css'
});
acwolff commented 3 years ago

@amkdev hanks for your hint!

I did test it see the first slide in this album

It is a start, but it should be improved, because:

  1. If there is no caption, like with my 3th slide, the image should use the the full viewport height, without a gap at the bottom.

  2. The value of paddingBottom should be larger in case of long captions, see my 2nd slide

  3. Sometimes no padding is requitred, see my 6th slide.

I should also like to append the caption to the image as you see here.

amkdev commented 3 years ago

you can't have it all, buddy - life is a compromise too! ;)

no, seriously: there is hardly a lightbox that offers so much flexibility. I think there could or should be a way to insert the caption into the image container instead of outside.

dimsemenov commented 3 years ago

@acwolff I'll post a plugin that adjusts an image based on its caption and vice-versa, in about a week or so.

acwolff commented 3 years ago

@acwolff I'll post a plugin that adjusts an image based on its caption and vice-versa, in about a week or so.

Great, thanks in advance!

dimsemenov commented 3 years ago

Early version of caption plugin is here https://github.com/dimsemenov/photoswipe-dynamic-caption-plugin

acwolff commented 3 years ago

@dimsemenov this looks very good!

I will test it on Monday, but before I test it I like to know whether it is possible to use attribute data-caption or another named data attribute to define the caption as I used in my simplified caption code example?

should I use this option:

captionContent: (slide) => { return slide.data.element.querySelector('a').getAttribute('caption’); ? }

acwolff commented 3 years ago

The correct captionContent option for mu data-caption attribute is:

        captionContent: (slide) => {
            return slide.data.element.dataset.caption;
        }

I am very happy with the first test you see in my test album.

But there are a few problems:

Click on the first thumbnail and vary the height of the window, thhen I see this: SidePanel Not all text is visible in the side-panel, but I can't read the rest because a scroll bar is missing.

I use almost no css code:

.pswp__dynamic-caption--aside {
  padding: 20px 15px 20px 20px;
  margin-top: 20px;
}

So why is the text not using the full width of the side panel? The empty right part of the side panel should also be used for text.

There is a lot of space on the left side of the image, so why is the image not shifted to the left, giving more space for text in the side panel.

Now open the 2nd slide and reswize the window untill you see this: MoreInfo Click on the 'More info' link

The text is now extended. Sometimes I see the extended text on the image, where I can't read it all because of a missing scroll bar and sometimes it is extended below the More info link, but then it is invisible.

But I did hope that the extended text was displayed in the side panel!

I use this code to toggle the more info text:

function toggleMore() {
    if ($('.moreInfo').is(":visible")) {
        $('.moreInfo').hide();
        $('.triggerMoreInfo').text(strMoreInfo);
    } else {
        stopSlideShow();
        $('.moreInfo').show();
        $('.triggerMoreInfo').text(strLessInfo)
    }
   pswp.updateSize(true);

}

I use pswp.updateSize(true); to force an update, is that correct or should I use other code?

Finally, my thumbnails page shows most times a scrollbar, but why do I see that scrollbar in the lightbox? The scrollbar is removed if you click the fullscreen button.

Thanks for this great plugin, I will test it further tomorrow.

acwolff commented 3 years ago

@dimsemenov, the biggest problem is still the missing vertical scroll bar in the side panel, but for the rest I am very happy with your plugin.

I see another small problem: I did add now my own css code:

.pswp__dynamic-caption--aside {
  top: 0px;
  max-width: 20%;
  color: #000000;
  background-color: rgba(255,255,255,1.0);
  padding-left: 14px;
  padding-right: 14px;
  margin-top: 60px;
}
.pswp__dynamic-caption--below {
  color: #000000;
  background-color: rgba(255,255,255,1.0);
  padding-left: 14px;
  padding-right: 14px;
}
.pswp__dynamic-caption--mobile {
  color: #000000;
  background-color: rgba(255,255,255,1.0);
  padding-left: 14px;
  padding-right: 14px;
}

See my test album.

Click on the first thumbnail and next make the window smaller. Although I use top: 0px; in .pswp__dynamic-caption--aside , the text in the side panel shifts down if I make the window smaller: SidePanel-2

dimsemenov commented 3 years ago

That plugin is not designed for such type of long captions. If you just want a sidebar - use paddingRight JS option during initialization and position the caption via CSS (as at https://photoswipe.com/v5/docs/caption/)

amkdev commented 3 years ago

@dimsemenov, the biggest problem is still the missing vertical scroll bar in the side panel, but for the rest I am very happy with your plugin.

grafik

acwolff commented 3 years ago

That plugin is not designed for such type of long captions. If you just want a sidebar - use paddingRight JS option during initialization and position the caption via CSS (as at https://photoswipe.com/v5/docs/caption/)

Ok I understand, in most cases the current implementation works fine, so it is not a big problem.

But please answer my other questions:

I use pswp.updateSize(true); to force an update if a More Info link is clicked, but I see no effect., so is that correct or should I use other code?

My thumbnails page shows most times a scrollbar, but why do I see that scrollbar in the lightbox?

Although I use top: 0px; in .pswp__dynamic-caption--aside , the text in the side panel shifts down if I make the window smaller, can I prevent that?

acwolff commented 3 years ago

Yes @amkdev that solves the scroll bar problem!

I use now this CSS code:

.pswp__dynamic-caption--aside {
  max-width: 20%;
  color: #000000;
  background-color: rgba(255,255,255,1.0);
  padding-left: 14px;
  padding-right: 14px;
  margin-top: 60px;
  max-height: 90vh;
  overflow-y: scroll;
}

See the effect n my test album.

Thanks a lot for your help!