data-lightbox attribute empty for text and image content element

[fritzmg]

Affected version(s)

4.12, 4.13, 5.x

Description

Currently the data-lightbox will be empty in content elements like text or image, since they do not call setLightboxGroupIdentifier on the figure themselves. However, this is a regression from previous Contao versions as Controller::addImageToTemplate would automatically generate a hash for the data-lightbox attribute, so that these images (depending on the actual lightbox implementation) are not grouped together. See also contao/core#3742

Now the question is: instead of having to call setLightboxGroupIdentifier should the figure builder simply always generate a lightbox ID, so that it behaves more like in previous Contao versions (though this was also not without its problems: #1774)? Or should ContentImage and ContentText for example be adjusted, so that they also set setLightboxGroupIdentifier.

[m-vo]

IMHO the current behavior was at least an intentional decision. I remember having discussed this with @ausi, but I do not remember the exact reasons and conclusions anymore. 😄

[ausi]

Now the question is: instead of having to call setLightboxGroupIdentifier should the figure builder simply always generate a lightbox ID

Would that be possible? AFAIR this mostly gets generated based on the ID of the content element?

[fritzmg]

Would that be possible? AFAIR this mostly gets generated based on the ID of the content element?

True, now we lack that information.

[ausi]

So I think we should call setLightboxGroupIdentifier() in these two content elements then.
Does addImageToTemplate() still set this automatically?

[fritzmg]

Does addImageToTemplate() still set this automatically?

As far as I can see, no. I think we should fix that may be.

[m-vo]

Should "no lightbox group id" not mean "do not group" rather than "group under ''"? Maybe this was the idea. We could generate a pseudo-random value based on the file path (probably in the template) if none was set, but I'd prefer if the FE scripts would act like this.

[m-vo]

From #1753 (comment)

[fritzmg]

Yeah, it just so happens that glightbox seems to group them by default and there does not seem to be an easy way to prevent this, but I'll research there further.

[m-vo]

Maybe just with a bit of JS? You should probably be able to use document.querySelectorAll('a[data-lightbox]') and set a random value if empty before initializing GLightbox without having to touch any code/template.

[fritzmg]

Yes, though that doesn't seem very elegant to me either 😁

[m-vo]

A "more elegant" way would be to explicitly initialize all GLightbox instances with their API instead of using the selector. But probably also more work for no real benefit. 🙂 Either way it's at least encapsulated within the realm of the lightbox library then.

[ausi]

I think this can be fixed in GLightbox by replacing the line in https://github.com/inspiredminds/contao-glightbox/blob/bea614852b47b64adbabc774ea3062c3aa33bb56/contao/templates/js_glightbox.html5#L13 with:

if (element.hasAttribute('data-lightbox')) {

@fritzmg WDYT?

[fritzmg]

I don't think that would fix it, since then every element with an empty data-lightbox attribute will still receive a data-gallery attribute and thus be grouped together?

In any case, the discussion shouldn't be so much about the specific lightbox implementation but whether we should reinstate the original behaviour or not.

[ausi]

As it works with both lightboxes from the core I would prefer to keep it as it is now, that is:

• <a href="" data-lightbox> means “open in lightbox, no grouping”
• <a href="" data-lightbox="abc"> means “open in lightbox” group all abcs together.

[fritzmg]

As it works with both lightboxes from the core

Another reason why I am bringing this up at all is because we discussed whether we want to remove or replace the current lightbox implementations that ship with the core, in particular we wanted to look for an alternative that does not require jQuery (see also #332). I created the inspiredminds/contao-glightbox extension to test how easy it is to use that lightbox with the existing HTML output of Contao so that we can evaluate whether we want to use that in the future may be.

[leofeyer]

As discussed in the Contao call, we want to keep this as outlined by @ausi in #4029 (comment).