Carousel: No listeners for mobile carousels with `data-bs-ride=boolean`

[louismaximepiton]

Prerequisites

• [X] I have searched for duplicate or closed issues
• [X] I have validated any HTML to avoid common problems
• [X] I have read the contributing guidelines

Describe the issue

Seems to be there for quite a long time.

The main issue is that if data-bs-ride is set to anything but carousel, the constructor() isn't called (whereas there's a check inside to avoid launch cycle()). This leads to unset (at least) touch listeners which, in mobile views, is kinda weird IMO.

I don't have the time to test it atm, but I think there's only a change on the selector would do the trick ?

Reduced test cases

Go on https://twbs-bootstrap.netlify.app/docs/5.2/components/carousel/#with-controls (which works well with mobile listeners) Go on https://twbs-bootstrap.netlify.app/docs/5.2/components/carousel/#with-indicators (which is broken with mobile listeners until we click one button)

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome, Firefox

What version of Bootstrap are you using?

5.2.2 (current_main_branch)

[GeoSot]

I am not sure if this is a bug 😕

http://localhost:9001/docs/5.2/components/carousel/#via-data-attributes

The data-bs-ride="carousel" attribute is used to mark a carousel as animating starting at page load. If you don’t use data-bs-ride="carousel" to initialize your carousel, you have to initialize it yourself. It cannot be used in combination with (redundant and unnecessary) explicit JavaScript initialization of the same carousel.

However, it can change behavior, auto-initializing itself, with one change on https://github.com/twbs/bootstrap/blob/3377ca49a11bdfcf521803a391dcc3ddab86c655/js/src/carousel.js#L64

to const SELECTOR_DATA_RIDE = '[data-bs-ride]'

[louismaximepiton]

Oh, haven't seen it before you highlighted it! Thanks a lot for that!

Instead of a bug, could it be an enhancement for v5.3 or v6 ? or should it stay as-is ?

[GeoSot]

/cc @patrickhlauke please give us your opinion on this

[Mrunmayi28]

Hi! I am new to the open source, I want to contribute and fix this issue.

[patrickhlauke]

so is the issue that currently the carousel is only auto-initialised when data-bs-ride="carousel" and otherwise it requires manual initialisation?

I'm guessing that auto-initialisation is not done for performance reasons (similar to why we don't auto-initialise popovers). and if performance is still a valid reason, it does make sense to me that it's opt-in except for ones that are set to automatically animated right away.

maybe we should make the need for manual initialisation (unless using data-bs-ride="carousel") more apparent/explicit in the docs?

also, don't the docs manually initialise all carousels? is that what's also missing?

alternatively, we could just have all carousels auto-initialise, or is that too much of a perf hit these days?

[louismaximepiton]

so is the issue that currently the carousel is only auto-initialised when data-bs-ride="carousel" and otherwise it requires manual initialisation?

Yes, IMO the carousels should be auto-initialized, but it might brings some breaking changes since people that were using it already initialize it.

I'm guessing that auto-initialisation is not done for performance reasons (similar to why we don't auto-initialise popovers). and if performance is still a valid reason, it does make sense to me that it's opt-in except for ones that are set to automatically animated right away.

I might be wrong but I feel like the constructor only brings listeners so I think it might be fine on perf issues (moreover, the listeners are set once there was a interaction with the carousel).

alternatively, we could just have all carousels auto-initialise, or is that too much of a perf hit these days?

Agree on this part modulo the breaking changes again 😄

[GeoSot]

As it seems a possible breaking change (these that initialize Carousel through constructor), maybe is better to postpone it for v6.

However, @louismaximepiton pointed something here. The carousel touch listeners are registered after the initialization, in contradiction with click event listeners that are registered on document

[patrickhlauke]

However, @louismaximepiton pointed something here. The carousel touch listeners are registered after the initialization, in contradiction with click event listeners that are registered on document

ah yes, event listeners should all register at the same time at initialisation, not just after first interaction

[MrUpsidown]

I had a hard time understanding why my carousel would not allow me to swipe unless I frist click on an indicator / control. To me this is a bug. At least there is nothing in the documentation that depicts this behavior.

[patrickhlauke]

yes, it's clearly a bug as far as i'm concerned. we just need somebody to tackle this now...

[GeoSot]

The solution is real simple if we are sure we want this attitude :) https://github.com/twbs/bootstrap/issues/37295#issuecomment-1276830684

[patrickhlauke]

The solution is real simple if we are sure we want this attitude :) #37295 (comment)

i think it's even simpler than that: no need to check if there's a data-bs-ride attribute at all, just attach the touch listeners on initialisation, all the time, just like with mouse listeners. touch functionality is not dependent on whether it autoscrolls or scrolls after first interaction.

[GeoSot]

So you propose to initialize carousels by default? (maybe it will break existing js implementations)

Note: The alternative, to listen globally for swipes, seems a bit eager, and at this point I cannot assure you if it will work easily.

For sure, if anyone wants to try, I will be glad to help him

[patrickhlauke]

The solution is real simple if we are sure we want this attitude :) #37295 (comment)

i think it's even simpler than that: no need to check if there's a data-bs-ride attribute at all, just attach the touch listeners on initialisation, all the time, just like with mouse listeners. touch functionality is not dependent on whether it autoscrolls or scrolls after first interaction.

it seems that the problem is in fact that our docs page does not explicitly initialise the carousels (apart from the auto-advancing ones which take care of themselves). so we don't actually do what we tell devs they need to do. and then the bit about authors needing to manually initialise is just hidden away and not made explicit/prominent.

so i think we can either go for full-on auto-initialisation (which yes, may have a slight performance hit and possibly break existing sites that did actually read the docs and did their manual initialisation, so it'd be a breaking change), OR we modify our docs page to actually do the manual initialisation, and make it far more prominent in our docs page that authors MUST do this themselves.