setting open and close effect individually

[blueswen]

Setting effect individually with effect data attribute is not working. To fixing this issue, I updated parseConfig to load closeEffect and openEffect setting form data attributes, then using effects from each slide object instead of global settings.

[gingerchew]

Couple of things, first being please review the CONTRIBUTING.md if you haven't.

There's no updated build, which makes me think you didn't test out your changes on the Demo site. If you open up the project and run npm run watch, and then save a file inside of src/ to get a new minified version, you should get your new version to test.

A couple of comments regarding the actual pull request:

• Since the way open/close effects are handled is being changed, should the slide effect should also follow the same convention?
• data-open-effect, data-close-effect, and data-slide-effect are kind of a mouthful (not your fault, just how data-* attributes are), I wonder if there is a better way to handle that
• You left a console.log in your code

Normally, we'd prefer hashing out some of that in a bug report or a feature request.

My own two cents, there is such a thing as too much configuration. Fixing the broken aspects of the data-effect attribute should still be done, I won't argue against that. The documentation for animations can already be hard to groq, they are probably one of the more complex features that GLightbox has. It is a complex situation that is being made more complex (I think partially because it was already complex).

I'm also not opposed to hashing this stuff out here. I defer to @biati-digital for most of this though, since these feel like bigger additions to me.

Also nice work on the MKDocs plugin! Very clean!

[blueswen]

@gingerchew Thanks for reviewing and reminding, I actually forgot to add the dist files. The dist files and the redundant console.log issues has been fixed. I also created two codepens to reproduce the issue GLightbox from CDN and new version witch able to fix this fixed version. Because I can not validate this changes with ie11, hopes these codepens could help for testing.

Since the slide effect controlling logic is much more complicated than the two others, which I am not sure I can deal them correctly, and setting the slide effect individually seems not a common use case. I just reimplemented the open and close effects.

Using data-open-effect and data-close-effect as attribute name can reuse the extend logic for data-desc-position, and also follows the Lightbox options convention.

Thank you for the compliment. GLightbox is a great package compatible with a lot of environments. Grad to help more people can enjoy the benefits of GLightbox!

[biati-digital]

Hi @blueswen thanks for the PR, i think @gingerchew has a good point about having a lot of options, not even sure if people are using the advanced options to create custom animations (personally i never needed that in any project) or if having different slide in/out effects for each slide is even useful, definitely it would be great if we can think of a better solution.

Would love to know if you find those features useful or if you have used them in a project.

[stale]

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]

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]

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