Bootstrap4 carousel doesn't render correctly

[Adam]

I tried to put a carousel into index.tpl using the example from the bootstrap documentation[1] but it's broken in a number of ways; the arrows and indicators don't render in the correct position and the animation does not run as expected and instead ends up as a sort of awkward shuffle (I can post an example if requested).

I've narrowed down the problem to a number of overrides in the raw theme in Mahara which my theme is based on. If I remove the _carousel.scss from components/ and the carousel related code from _bootsrap-variables.scss and theme.js then the carousel works as expected. [2-4]

A search through the commit log shows some fairly old commits related to carousel and image gallery.[5]

[1] https://getbootstrap.com/docs/4.3/components/carousel/#with-indicators
[2] https://git.mahara.org/mahara/mahara/-/blob/master/htdocs/theme/raw/sass/components/_carousel.scss
[3] https://git.mahara.org/mahara/mahara/-/blob/master/htdocs/theme/raw/sass/utilities/_bootstrap-variables.scss#L1063
[4] https://git.mahara.org/mahara/mahara/-/blob/master/htdocs/theme/raw/js/theme.js#L91
[5] https://git.mahara.org/mahara/mahara/-/commits/master?search=carousel

[Kristina Hoeppner (kris-hoeppner)]

Hi Adam,

There is no reason why the image gallery could not use the regular Bootstrap carousel other than the feature of jumping to the first and last image in the gallery.

If we ignore that for a moment, Evonne and I suggest to update the current carousel gallery slideshow to the latest Bootstrap 4 code carousel. We would want to make the 'next' and 'previous' indicators more accessible though as light grey and white are not be visible on all backgrounds.

We haven't yet figured out how to get the bottom indicators in, replacing the fast-forward icons. We also propose to keep the description below the image for accessibility reasons, but need to think a bit more about that because it would affect all instances where you might also display a regular carousel that has descriptions.

Evonne is tinkering, and once she has a proposal, the link to the code review item will show up here.

Thank you
Kristina

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/11918

[Evonne Cheung (evonne)]

Image gallery has now been updated so that the bootstrap carousels no longer contradicted with it. In the update we have removed the fast-forward icons in the gallery, which may need reviewing at a later stage if the need arises. The arrows has been given a background to pass contrast standards, and the captions appear below the image for the same reason. The indications styles from bootstrap examples remain as they currently are in bootstrap, as they don't appear in the gallery markup.

Cheers
Evonne

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/11918
Committed: https://git.mahara.org/mahara/mahara/commit/607496dced78e2780fe3bd9bd45cbe345a43988f
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 607496dced78e2780fe3bd9bd45cbe345a43988f
Author: Evonne <email address hidden>
Date: Wed Aug 11 13:51:22 2021 +1200

Bug 1939165 Bootstrap4 carousel styling

Aligning gallery slideshow carousel styling with bootstrap 4's example
and to make it accessible, including the caption which would appear
under the image instead of on top of the image.

behatnotneeded

Change-Id: I0ec6268a1e64b5ec25833314beeb8fa1ddd7c857

[Adam (adam-jtm30)]

This doesn't seem to have fixed much for me for example the window.carouselHeight function in theme.js still sets the position of the controls incorrectly so they are positioned at the top of the carousel.

[Kristina Hoeppner (kris-hoeppner)]

Hi Adam,

Can you please provide a bit more info or a screenshot? Did you use the Carousel example at https://getbootstrap.com/docs/4.3/components/carousel (without the indicator, just the controls)?

Thank you
Kristina

[Kristina Hoeppner (kris-hoeppner)]

Hi Adam,

Are you trying to use the carousel in a 'Text' or 'Note' block or in a static page? It only works when you put it into a template. HTMLpurifier strips out empty <span>. If you want to use it in a text block controlled by TinyMCE, you would need to create a rule for HTMLpurifier to not strip specific syntax out.

This might be similarly done as in https://reviews.mahara.org/#/c/11547/ (go to /lib/web.php itself for more examples.

Cheers
Kristina

[Adam (adam-jtm30)]

Here's a short video showing the problem. I copied the "carousel with controls" example from bootstrap documentation that you mentioned into templates/index.tpl. As you can see it doesn't animate in the same way as on the bootstrap site and the controls are pushed up to the top of the div.

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/11995

[Evonne Cheung (evonne)]

Hi Adam

I tried to replicate what you did in index.tpl but wasn't able to, although I did see that the indicators were slightly higher than they should be due to the lack of a clearer for the dashboard boxes, which I've put into this latest patch. But what I did notice in your video is that the indicators are still the old colours, which could mean that you haven't compiled your theme yet. If you could apply this latest patch, run "make css" in the terminal, and then check if the indicators are white icon on black background, this will show whether the patch has been applied properly.

I've attached a screenshot to show how the indicators should look.

Cheers
Evonne

[Adam (adam-jtm30)]

Hi Evonne,

Sorry about that! I thought I had gulp waiting to compile my themes on change but it seems it didn't trigger. All looks good now, thanks a lot.

Thanks for your help too Kristina.

Cheers,
Adam.

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/11995
Committed: https://git.mahara.org/mahara/mahara/commit/940dce536191e188f67578fcafc1a2a05f1fd528
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 940dce536191e188f67578fcafc1a2a05f1fd528
Author: Evonne <email address hidden>
Date: Mon Sep 13 11:01:32 2021 +1200

Bug 1939165 Bootstrap4 carousel styling fixes #2

Adding a clearer which fixes the indicators on dashboard if it's
placed under the three dashboard boxes. Also removed text decoration
on the indicators.

behatnotneeded

Change-Id: Ic2a4ebc7d9462949af334f0c21576d36e5e03f60