Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Width and height are ignored when slide has a description #323

Closed
trych opened this issue Mar 7, 2022 · 9 comments · Fixed by #426
Closed

Width and height are ignored when slide has a description #323

trych opened this issue Mar 7, 2022 · 9 comments · Fixed by #426

Comments

@trych
Copy link

trych commented Mar 7, 2022

Describe the bug
The width and height attributes that I assign to an image via data-width="400px" or data-width="90vw" etc. are only taken into consideration as long as I do not add a data-description="Bla bla" attribute. As soon as I add one, the slide seems to take the full height (up to a maximum of 100vh - descriptionHeight).

Are you able to reproduce the bug in the demo site
Yes, here is a codesandbox of the issue: https://codesandbox.io/s/glightbox-test-inline-props-rypsmn?file=/index.html
And here is the corresponding live page: https://rypsmn.csb.app/

To Reproduce
Add an image with data-height and/or data-width attributes, set up glightbox. The width and height should now work as expected. Now add a data-description attribute with some text, the width and height are now ignored.

Expected behavior
Width and height should still be taken into consideration.

Post the code you are using
See the codesandbox above.

Desktop:

  • macOS 10.13.6
  • lastest Chrome, lastest Firefox, probably others as well

Smartphone:

  • the mobile lightbox does seem to work as expected, so no issue here.
@seamofreality
Copy link

This won't help you – but I've got the same issue. Was just pulling my hair out over it, glad to know it's not just me.

@jamesckemp
Copy link

Can confirm this happens for me too.

@jamesckemp
Copy link

Seems if you set the description position to the right then the image size is OK. Maybe the interim is to do that and modify the CSS.

@jamesckemp
Copy link

I ended up using slide_after_load to add a max-width to the .ginner-container.

lightbox.on( 'slide_after_load', ( data ) => {
	const image = data.slide.querySelector( 'img' );

	if ( image.complete ) {
		someFunction( image );
	} else {
		image.addEventListener( 'load', someFunction );
	}
} );

@acc987
Copy link

acc987 commented Sep 2, 2022

Might this have something to do with the following lines of code?

glightbox/src/js/glightbox.js

Lines 1081 to 1090 in 29e365d

if (winWidth <= 768) {
let imgNode = image.querySelector('img');
//imgNode.setAttribute('style', '');
} else if (descriptionResize) {
let descHeight = description.offsetHeight;
let imgNode = image.querySelector('img');
imgNode.setAttribute('style', `max-height: calc(100vh - ${descHeight}px)`);
description.setAttribute('style', `max-width: ${imgNode.offsetWidth}px;`);
}

This could also explain why it works on mobile, because the code of the else if seems like it's not entered on mobile:

glightbox/src/js/glightbox.js

Lines 1065 to 1069 in 29e365d

if (winWidth <= 768) {
_.addClass(document.body, 'glightbox-mobile');
} else {
_.removeClass(document.body, 'glightbox-mobile');
}

From what I can tell, setAttribute would overwrite the style attribute, thus invalidating the following assignment(s), right?

if (data.hasOwnProperty('_hasCustomWidth') && data._hasCustomWidth) {
img.style.width = data.width;
}
if (data.hasOwnProperty('_hasCustomHeight') && data._hasCustomHeight) {
img.style.height = data.height;
}

I am not sure whether the use of elem.setAttribute('style', ...) is deliberate, but you could try replacing that with elem.style.maxHeight = ... and see whether that helps. You might still have to make sure that the style attribute exists in the first place, for example when no width/height has been set for an image. (I am not that familiar with JavaScript nor with the entirety of GLightbox, so I cannot provide a full/proper solution to your problem, unfortunately.)

@ynamite
Copy link

ynamite commented Dec 16, 2022

Setting either a title or a description overrides the widthsetting btw.
Any workarounds?

@felixranesberger
Copy link
Contributor

I've searched for over a hour to get this to working.
As suggested above, I'm also certain that the issue lies here:

imgNode.setAttribute('style', `max-height: calc(100vh - ${descHeight}px)`); 

But I couldn't find a way, to access the initial link element to access the data-height attribute.

Copy link

stale bot commented Dec 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Dec 15, 2023
@felixranesberger
Copy link
Contributor

Why would you activate a stale bot to close inactive issues, when the repository is unmaintained.
This just causes valid issues to be closed sooner or later, due to inactivity by the maintainer.

@stale stale bot removed the wontfix label Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants