Active menu item glitch

The CMS Questions
1

Not quite sure whats going on here but i cant see to get the active class on the currently open page, using code that previously worked fine (its common code i have a plugin to make it reusable).

  'siteMethods' => [
      'sitemenu' => function ($btnclass = 'btn-small', $activeclass = 'active') {
          $sitemenu = $this->menu()->toStructure();
          $menulist = '';
          foreach ($sitemenu as $menu) {
            $link = $menu->link()->toLinkObject();
            
            $active = '';
            if ($link->type() == 'page') {
                $active = ($this->find($menu->link()->toLinkObject()->value())->isOpen()) ? $activeclass : '';
            }

            $link = Html::a($link->url(), $link->title(), ['data-pjax' => '', 'data-uri' => $link->url(), 'title' => $link->title(), 'class' => $active]);
    
            $menulist .= $link;
          }
          return $menulist;
      }
  ]

I get an empty value for the class property on this line:

$link = Html::a($link->url(), $link->title(), ['data-pjax' => '', 'data-uri' => $link->url(), 'title' => $link->title(), 'class' => $active]);

If i change the class to some random property, it works 'clasdfsdfss' => $active - then i get active as the value! Bit weird but not sure why its not working.

Any ideas?

2

What does toLinkObject() do and what does it return?

3

Its the link field plugin, it detects the kind of url and returns the correct formatted one (normal, mailto, tel etc)

4

More weirdness… it does work if call the site method but give it another class name, for example, ‘open’.

<?= $site->sitemenu('menu-link', 'open') ?>

It seems to have an issue with the word ‘active’ :man_shrugging:

5

This code might not return a page object (could be shortend to $this->find($link->value()) anyway, since you defined that variable), so you should check before calling isOpen().

6

Interesting.

What I wrote above is nevertheless necessary.

7

Not sure I follow, isnt the ternerary checking that?

8

Something like this, right?

  'siteMethods' => [
      'sitemenu' => function ($btnclass = 'btn-small', $activeclass = 'active') {
          $sitemenu = $this->menu()->toStructure();
          $menulist = '';
          foreach ($sitemenu as $menu) {
            $link = $menu->link()->toLinkObject();
            
            $active = '';
            $current = $this->find($link->value());
                 
            if ($current !== null and $link->type() == 'page') {
                 $active = ($current->isOpen()) ? $activeclass : '';
            }

            $link = Html::a($link->url(), $link->title(), ['data-pjax' => '', 'data-uri' => $link->url(), 'title' => $link->title(), 'class' => $active]);
    
            $menulist .= $link;
          }
          return $menulist;
      }
  ]
9

Something like that, yes, although I’d change the order

if ($link->type() === 'page' && ( $current = $this->find($link->value()) ) ) {
  $active = ($current->isOpen()) ? $activeclass : '';
}
10

Thanks, I have updated it… still doesnt like the word active tho :frowning: I have solved it by changing the css file, but still bugging me as to what the reason is.

11

Yep, that would need some debugging…

12

Hm, I tested your example in a 3.5.3.1 Starterkit and it also works with the default active class.

I wonder if you have anything else anywhere in your code that interferes…

13

Thanks very much for trying. I dont think so, its pretty minimal site really. Only these plugins…

autoresize
kirby-locator
kirby3-mobile-detect
kirby3-schema
kirby3-seo
link-field

All the pages have the same controller…

<?php

return function ($page, $kirby, $site, $tag) {

  // SEO
  $seo = $kirby->controller('seo' , compact('page', 'site', 'kirby'));

    // SCRABBOOK
    $work = $kirby->collection("work");

    // TAG FILTER
    if($tag) {
      $work = $work->filterBy('tags', $tag, ',');
    }
  
    $tags = $work->pluck('tags', ',', true);
  
    asort($tags);
  
    $currenttag = $kirby->request()->params()->tag();
  
    $data = compact('tags', 'currenttag');

    return array_merge($seo, $data);

};

Thats it really.

14

What about pjax?

15

Thats a jquery plugin (its an old site i didnt build orginally - im upgrading it from Kirby 1), i think for preloading pages.

16

Ah ha! Finally found the culprit… dug through the javascript file which i didnt write…

$('.filter a').removeClass('active');

So javascript was interfering with my classes.

@texnixe thanks for helping me narrow it down. :tada: