laravel-menu icon indicating copy to clipboard operation
laravel-menu copied to clipboard

Cannot activate link

Open dvital1001 opened this issue 6 years ago • 6 comments

Hi! I tried this module but it didn't set "active" class in code:

       \Menu::make('HeadMenu', function ($menu) {
            $menu->add('About', 'about');
            $menu->about->attr(['class'=>'nav-item']);
            $menu->about->link->attr(['class' => 'nav-link']);
        });

If I comment one line it's work:

        \Menu::make('HeadMenu', function ($menu) {
            $menu->add('About', 'about');
            $menu->about->attr(['class'=>'nav-item']);
            //$menu->about->link->attr(['class' => 'nav-link']);
        });

Maybe I do something wrong? Thank you!

dvital1001 avatar Jul 21 '19 15:07 dvital1001

I wonder if this is related to #246 perhaps the active class is conflicting. I'll need to look into it.

dustingraham avatar Jul 21 '19 21:07 dustingraham

I don't use active class in any other case. Only it be in generated menu. Is this the correct behavior? It is don't comfortably for programming menu unfortunately.

dvital1001 avatar Jul 22 '19 09:07 dvital1001

excuse me my english ...

There is a this situation. User classes remove the active class.

dvital1001 avatar Jul 22 '19 19:07 dvital1001

Yes, sounds like there might be a bug with the active class overwriting the classes.

If you have control over the CSS you might avoid adding classes to the link, and instead use .nav-item > a to target it. This would just be a workaround until we figure out the class collision.

dustingraham avatar Jul 23 '19 17:07 dustingraham

I've ran into this today and took a look.

The problem is that the 'active' class is set to the link as soon as the Item is created and when the Link->attr() is used, if the 'class' key is set, the active class is overridden by the classes that are set in the array by the array_merge. (Link.php:102).

My code fix is below, I've rearranged the if a bit to add the fix, feel free to use it if you wish.

public function attr() {
    $args = func_get_args();

    if (isset($args[0])) {
        if (is_array($args[0])) {
            $this - > attributes = array_merge($this - > attributes, $args[0]);
        }
        elseif(isset($args[1])) {
                $this - > attributes[$args[0]] = $args[1];
            } else {
                return isset($this - > attributes[$args[0]]) ? $this - > attributes[$args[0]] : null;
            }
            //The fix
        if ($this - > isActive && isset($this - > attributes['class']) && !strpos($this - > attributes['class'], $this->builder->conf('active_class'))) {
            $this - > attributes['class'] = trim($this - > attributes['class'].
                " ".$this - > builder - > conf('active_class'));
        }
        return $this;
    }

    return $this - > attributes;
}

Just a quick explanation about the fix (I'm sure you're able to figure it out but it doesn't hurt to explain), it checks if the link is active, has the key class set and if that key doesn't have the active_class string in the, if this conditions apply, it adds the active class to the key. I didn't look into the code in depth so, if there is any other attribute that can be overridden it would be wise to fix it so that this doesn't show up in the future.

GoncaloTMelo avatar Aug 03 '19 22:08 GoncaloTMelo

I've ran into this today and took a look.

The problem is that the 'active' class is set to the link as soon as the Item is created and when the Link->attr() is used, if the 'class' key is set, the active class is overridden by the classes that are set in the array by the array_merge. (Link.php:102).

My code fix is below, I've rearranged the if a bit to add the fix, feel free to use it if you wish.

public function attr() {
    $args = func_get_args();

    if (isset($args[0])) {
        if (is_array($args[0])) {
            $this - > attributes = array_merge($this - > attributes, $args[0]);
        }
        elseif(isset($args[1])) {
                $this - > attributes[$args[0]] = $args[1];
            } else {
                return isset($this - > attributes[$args[0]]) ? $this - > attributes[$args[0]] : null;
            }
            //The fix
        if ($this - > isActive && isset($this - > attributes['class']) && !strpos($this - > attributes['class'], $this->builder->conf('active_class'))) {
            $this - > attributes['class'] = trim($this - > attributes['class'].
                " ".$this - > builder - > conf('active_class'));
        }
        return $this;
    }

    return $this - > attributes;
}

Just a quick explanation about the fix (I'm sure you're able to figure it out but it doesn't hurt to explain), it checks if the link is active, has the key class set and if that key doesn't have the active_class string in the, if this conditions apply, it adds the active class to the key. I didn't look into the code in depth so, if there is any other attribute that can be overridden it would be wise to fix it so that this doesn't show up in the future.

This fixed the issue, haven't you submitted the pull request? Or there's another way to fix the issue than modifying directly the vendor folder?

jdcifuentes avatar Aug 23 '19 20:08 jdcifuentes