Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot activate link #247

Open
dvital1001 opened this issue Jul 21, 2019 · 6 comments
Open

Cannot activate link #247

dvital1001 opened this issue Jul 21, 2019 · 6 comments

Comments

@dvital1001
Copy link

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!

@dustingraham
Copy link
Collaborator

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

@dvital1001
Copy link
Author

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
Copy link
Author

excuse me my english ...

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

@dustingraham
Copy link
Collaborator

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.

@GoncaloTMelo
Copy link

GoncaloTMelo commented Aug 3, 2019

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.

@jdcifuentes
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants