biati-digital / glightbox

Pure Javascript lightbox with mobile support. It can handle images, videos with autoplay, inline content and iframes
MIT License
2.01k stars 226 forks source link

Got wrong in v3.3.0, there is no "node" property in image object, is it bug? #478

Open mahui-cn opened 4 months ago

mahui-cn commented 4 months ago

Describe the bug

I just upgrade to v3.3.0, It run into error when I click images in PC (not mobile) whereas it is normal in v3.2.0

I check the source code, it got wrong in this line : const slideTriggerNode = this.elements[this.index].node; I also debug this in Chrome, it is sure that there is no node property in the image object, is this a new bug? 微信截图_20240401060841

the following is related piece of source code for your reference....

 // if a slide height is set via data-height, we want to use that
 // if not, we fall back to 100vh
const slideTriggerNode = this.elements[this.index].node;
const maxHeightValue = slideTriggerNode.getAttribute('data-height') ?? '100vh';

imgNode.setAttribute('style', `max-height: calc(${maxHeightValue} - ${descHeight}px)`);
description.setAttribute('style', `max-width: ${imgNode.offsetWidth}px;`);

Are you able to reproduce the bug in the demo site Yes|No.

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior A clear and concise description of what you expected to happen.

Post the code you are using Help me solve issues faster, if I can copy paste your code I can try to reproduce and fix the bug faster.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop:

Smartphone:

jignesh-travelopia commented 3 months ago

@biati-digital Having the same issue here after updating to 3.3.0, It started throwing an error. However, It only happens when the slide has a description added to it.

image
jignesh-travelopia commented 3 months ago

@biati-digital Looks like this.elements[this.index] is not valid slide element anymore and throwing an error. https://github.com/biati-digital/glightbox/blob/master/src/js/glightbox.js#L1091

const slideTriggerNode = this.elements[this.index].node;
const maxHeightValue = slideTriggerNode.getAttribute('data-height') ?? '100vh';

To fix this issue, you can directly use the slide variable, which is already a reference to the slide element

const maxHeightValue = slide.getAttribute('data-height') ?? '100vh';
vincurekf commented 3 days ago

I was having the same issue but with galleries created from code. Changing line 1091 like this fixed it:

const maxHeightValue = slideTriggerNode ? slideTriggerNode.getAttribute('data-height') ?? '100vh' : '100vh';

It looks for .node when there is not any.

mahui-cn commented 2 days ago

Have official version fixed this bug?

@biati-digital