Fix height calculation for lightbox with description and height set

[felixranesberger]

Fixes: https://github.com/biati-digital/glightbox/issues/323

When setting a description and a height on the lightbox using data attributes, the height value got ignored in the max-height calculation.

This pull request fixes this issue.

[smonist]

@biati-digital any possibility of merging this one? I really enjoy using GLightbox and this bugfix would make it even better 🙌🙌

[gingerchew]

@felixranesberger Actually, I missed something, we need you to run the development command before we can merge. There's a guide in CONTRIBUTING.md

[felixranesberger]

@felixranesberger Actually, I missed something, we need you to run the development command before we can merge. There's a guide in CONTRIBUTING.md

Do you mean running ESLint? If yes, ESLint currently throws a warning, because a variable is defined but not used.

This is not caused by my changes. I think it should be fixed using another pull request, to keep these changes separate. Otherwise it could cause confusion 👍

[gingerchew]

Yeah it’s a little confusing, enter npm run watch in the CLI, then save a file. That will run all the build scripts.

[felixranesberger]

I wouldn't advise for unknown contributors to build the distribution files. This should be done by the package maintainer, when a new release is issued. I could inject harmful code into the dist files, without anyone knowing.

You will also run into merge issues, when two pull request modify the JS and build their own dist files, because they don't have the changes of each other.

But if it's important to you, I can also build the dist files locally and commit them to this pull request.

I just wanted to give my perspective ✌️

[gingerchew]

I get that, it would be nice if we (collaborators and maintainers) could run those commands inside the repo without dealing with all the Actions and what not. Thankfully knocks on wood those issues haven't happened for us. So for now, if you can run those commands would appreciate it.

[felixranesberger]

I added a commit with the distribution files.

[gingerchew]

Awesome, much appreciated! @biati-digital should be all good now.

[smonist]

Is @biati-digital still active on GitHub (or at all)? I was not sure about this and sent them an inquiry via the contact form on the website. So far I have not received any response at all.

@gingerchew are you by any chance in direct contact with biati-digital? :)

[felixranesberger]

Any chance to get the pull requests #426 and 424 merged? 🤔 @gingerchew

[felixranesberger]

Since they seem to be reviewed by you

[gingerchew]

I'll reach out and see if they can give a final look, both of our jobs keep us pretty limited in the "free time" department.

[stale[bot]]

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

[felixranesberger]

I'm not sure if activating a stale bot that closes pull requests after inactivity is a smart idea, if the given project is pretty much unmaintained. This causes valid pull requests to just vanish. To be honest, I'm a bit frustrated, because there seems to be no one really interested in maintaining the project at the moment.

[gingerchew]

@felixranesberger I understand the frustration. I've brought up the issue with stale bot before, since most of what I do is undo the tags/messages it leaves. I chatted with @biati-digital briefly a couple months ago, we talked about some of the open issues/PRs. Most of the issues that have not been merged yet are fixed in the upcoming major change.

I will reach out again and see if I can get permission to merge these for a final version/stop gap before the next major update.

[biati-digital]

Hi everyone. Apologies for the delay. I've been working on v4, which got a total overhaul. Created a plugins API, revamped the website, docs, and more.

I'll be merging a few PRs and dropping the final update in the next days.