fnagel / generic-gallery

TYPO3 extension: Generic Gallery - One gallery to rule them all!
https://extensions.typo3.org/extension/generic_gallery/
GNU General Public License v3.0
7 stars 12 forks source link

Bootstrap 5 carousel examples do not start sliding automatically #59

Closed mediaessenz closed 7 months ago

mediaessenz commented 7 months ago

SInse Bootstap 5 the their data attributes are namespaced (-bs). This namespace is missing for the ride setting in

To fix, simply change this line:

<div id="gallery-{uid}" class="carousel slide" data-ride="carousel">

to:

<div id="gallery-{uid}" class="carousel slide" data-bs-ride="carousel">
fnagel commented 7 months ago

Thanks for the report!

I've checked the docs again and found this:

For performance reasons, carousels must be manually initialized using the carousel constructor method. Without initialization, some of the event listeners (specifically, the events needed touch/swipe support) will not be registered until a user has explicitly activated a control or indicator. The only exception are [autoplaying carousels](https://getbootstrap.com/docs/5.3/components/carousel/#autoplaying-carousels) with the data-bs-ride="carousel" attribute as these are initialized automatically on page load. If you’re using autoplaying carousels with the data attribute, don’t explicitly initialize the same carousels with the constructor method.

https://getbootstrap.com/docs/5.3/components/carousel/#how-it-works

If I understand this correctly, we either want to remove the data-ride="carousel" completely or remove the manual initialization:

<f:asset.script identifier="generic-gallery-{uid}">
    new bootstrap.Carousel('#gallery-{uid}');
</f:asset.script>

Having both will result in JS issues. What do you think? I vote for removing the data attribute as it's easier for users to change the JS if needed.

mediaessenz commented 7 months ago

Well seen. I did not realized this yet.

You could initalize the carousel only if it doesn't has the corresponding data-bs-ride attribute like so:

<f:asset.script identifier="generic-gallery-{uid}">
if (!('bs-ride' in document.getElementById('gallery-{uid}').dataset)) {
    new bootstrap.Carousel('#gallery-{uid}');
}
</f:asset.script>
mediaessenz commented 7 months ago

Btw. I have added a slighly modified version of the original template on this page, without the upper restriction and did not see any issues: https://bruchwerk-theater.de/ All three carousels are working fine without any JS errors...

fnagel commented 7 months ago

I think I would rather remove the data attribute completely. To me, it looks like that's the preferred way according to the docs. And no need to add a switch for a HTML structure not officially supported by TWBS.

fnagel commented 7 months ago

Fixed in latest master. Thanks for the contribution!