Feature: Add frontpage slider functionality, solves #162

[matthias-buttgereit]

This PR adds a slider functionality implemented with the bootstrap carousel class.

• Added tab "Slider" to "Content" admin settings
• Added setting to activate the slider feature
• Added setting to have up to 6 slides, each slide consisting of a ...
  • background image
  • link for clicking on that image (optional)
  • caption text being displayed on top of the image (optional)
  • content text being displayed under the caption (optional)
  • title for the image and title for the link to improve accessibility (optional)
• Added setting for where to place the slider on the frontpage
• Added settings corresponding to the bootstrap carousel options

[matthias-buttgereit]

We opted to give the elements that are displayed on top of the slider images (text + nav elements) a light drop-shadow to make them stand out against a light background.

Additionally we gave the slides a small padding and round corners to get the design more in line with the boxes of the courses being displayed on the frontpage.

[abias]

Hey @matthias-buttgereit and @ShUl0w ,

thank you for providing this PR!
It is currently marked as "Draft". Do you still plan to improve it or is it ready to be reviewed?

Thanks,
Alex

[ShUl0w]

Hey @abias,

sorry for having this still in Draft-state, that was in fact unintentional; it kind of got lost in our current holiday schedule. Please feel free to review the feature, as I did a finishing review on our end last week. I can't switch the ticket to open and @matthias-buttgereit is currently out of office.

One thing I'd like to add to @matthias-buttgereit comment regarding deviations from the original issue is that we made a background image mandatory for a slide to be rendered. Generally speaking, the default styling of the Bootstrap carousel doesn't play nice with the default Moodle 4.X styling (and boost_union, for that matter), as content would practically be invisible when only text is displayed in a slide.

If it's a client mandated feature we can obviously change this relatively quickly, however I'd appreciate a review on your hand anyway, as I believe there might be additional changes required in addition to the original issue given the reason stated above.

Bests,
Tim

[abias]

Hi @matthias-buttgereit, @ShUl0w and @Diphylla ,

many thanks for working on this PR!

It's been really a while since you submitted this patch and I am sorry that it remained unhandled for so long.

I was now finally able to process it and I dared to rework it in several aspects:

• I rebased the branch onto latest master (of course)
• I bumped version (as you have introduced new settings)
• I moved the SCSS code from theme_boost_union_get_slider_scss() into post.scss. We have other SCSS code there, too, regardless if the particular feature is activated or not.
• I changed all boolean settings to use the THEME_BOOST_UNION_SETTING_SELECT_NO / THEME_BOOST_UNION_SETTING_SELECT_YES constants which we use in Boost Union for boolean settings normally
• I fixed the missing documentation and example content in slider.mustache
• I replaced the slider's killswitch setting with an implicit implementation which simply checks if any slide is enabled to make the settings simpler
• I added some more per-slide settings: link source, link target and order position
• I enhanced the sliderfrontpageposition setting to allow the admin to place the slider below the advert tiles as well
• I set the slide's 'active' class with PHP and not with JS
• I removed the hardcoded shadows of the texts and made them configurable, similar to the configuration options which have been added to the advertisement tiles recently
• I enhanced and improved the Behat tests
• I did some more refinements to align the contributed code with the existing codebase

Please note that even if this is a quite long list, your contribution was well done anyway! I am grateful for well-crafted contributions like yours, but sometimes several things have to be aligned during the review.

Regarding your comment that the background image is currently mandatory but could be optional, I would agree with your current approach. However, as you said, it should not be impossible to make the background image optional and allow text-only slides. However, this would require some more CSS work (and ideally Bootstrap 5 which provides a dark variant would have landed in Moodle then). I will create a follow-up issue for this task.

Unfortunately, you haven't granted push access to your PR branch, so I was not able to push my review changes to update this PR. Because of that, I have now created another PR #523 which replaces this one here. I will merge it as soon as Github actions has turned green there.

Cheers,
Alex