Hello,

I am trying to use the bootstrap_carousel integration with for example the following renderable array in a preprocess field:

$variables['carousel'] = [
  '#theme' => 'bootstrap_carousel',
  '#controls' => TRUE,
  '#indicators' => TRUE,
  '#interval' => $interval,
  '#slides' => $slides,
];

The indicators are generated automatically in a preprocess inside the bootstrap theme. Which is great thanks for the automation.

The problem I have is that the "#target" is not passed to the bootstrap/templates/bootstrap/item-list--bootstrap-carousel-indicators.html.twig

So the data-target attributes is empty and the indicators does not work.

I think it is due to the item_list hook_theme that does not register a target variable.

I will try to make a patch (this week).

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Grimreaper created an issue. See original summary.

Grimreaper’s picture

Assigned: Grimreaper » Unassigned
Status: Active » Needs review
FileSize
1.93 KB

Hello,

Here is a patch that use the context variable, declared in the item_list theme, to pass the target id to the indicator item list.

Thanks for the review.

markcarver’s picture

Status: Needs review » Needs work
+++ b/templates/bootstrap/item-list--bootstrap-carousel-indicators.html.twig
@@ -22,7 +22,7 @@
+      <li{{ item.attributes.addClass(item_classes) }} data-target="{{ context.target }}" data-slide-to="{{ loop.index0 }}"></li>

Using context will be confusing since twig also has the global variable: _context.

We can use #context internally, but item_list__bootstrap_carousel_indicators should be preprocessed to convert it to a normal target variable from #context.

Grimreaper’s picture

Status: Needs work » Needs review
FileSize
2.26 KB

Hello,

Thanks for your suggestion. I agree that a context variable can be misleading.

Here is a patch that take your suggestion into account.

Thanks for the review.

markcarver’s picture

Status: Needs review » Needs work
  1. +++ b/src/Plugin/Preprocess/ItemListBootstrapCarouselIndicators.php
    @@ -0,0 +1,31 @@
    +    // Add a target variable from the item list context data.
    +    $context = $variables->offsetGet('context', []);
    +
    +    if (isset($context['target'])) {
    +      $variables->target = $context['target'];
    +    }
    

    All that's needed here is

    $variables->target = $variables->getContext('target');
    

    See Variables::getContext.

  2. +++ b/src/Plugin/Preprocess/ItemListBootstrapCarouselIndicators.php
    @@ -0,0 +1,31 @@
    +    // Ensure all attributes are proper objects.
    +    $this->preprocessAttributes();
    

    This shouldn't be needed.

markcarver’s picture

Also, considering that this is a theme hook suggestion and in this base theme, I think it makes sense to call parent::preprocessVariables() before everything in case sub-themes add a preprocessor on item_list.

Grimreaper’s picture

Status: Needs work » Needs review
FileSize
2.08 KB

Thanks for the review.

Here is an updated patch.

markcarver’s picture

Status: Needs review » Fixed
markcarver’s picture

A new issue should be created for #start_index that does the same thing actually.

Grimreaper’s picture

Thanks for the commit :)

Grimreaper’s picture

Ok, I will see that during the week or this weekend.

Grimreaper’s picture

Adding follow up issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.