[BUG] overriding default active menu behavior is not working properly

[AMoktar]

Describe the bug

Overriding default active menu behavior is not working properly menu tree

[
            'text'        => 'pages',
            'url'         => 'admin/pages',
            'submenu' => [
                [
                    'text'        => 'review',
                    'url'         => 'admin/pages?state=0',
                   'active'      => ['admin/pages?state=0],
                ],
                [
                    'text'        => 'active',
                    'url'         => 'admin/pages?state=1',
                   'active'      => ['admin/pages?state=1'],
                ],
                [
                    'text'        => 'all',
                    'url'         => 'admin/pages',
                    'active'      => ['admin/pages'], // always set active
                ],
            ],
        ],

Expected behavior

I expeceted to override the default behavior as mentioned https://github.com/jeroennoten/Laravel-AdminLTE/wiki/Menu-Configuration#the-active-attribute

I expect to be active only of it is exact match . The last url is always active, that is not what I expected, since I've overriding the default behavior.

| Laravel | "laravel/framework": "^10.0" "jeroennoten/laravel-adminlte": "3.9.2"

Thanks

[dfsmania]

@AMoktar Can you add more details about what are you expecting (i.e, when you expect every menu item to be active) and what is currently happening?

[AMoktar]

Thanks 🙏 for your response .

I've updated the first post with expected behavior

This repo is amazing, thanks for your effort

[dfsmania]

@AMoktar thanks for the details. Actually, on the procedure that checks whether a menu item should be active, query parameters are only taken into account when they are present on the pattern specified on the active property. So yes, it won't work as you are expecting right now.

For example, when you navigate to https://<your_site>/admin/pages?state=0 and the procedure is checking whether the menu item all should be active, it compares with the result of $this->request->url() (the current requested URL without the query parameters) and then a match will be detected.

I will try to review this logic when I get some time to see if something can be done without breaking the current functionality, you can also check it on https://github.com/jeroennoten/Laravel-AdminLTE/blob/master/src/Menu/ActiveChecker.php and suggest some ideas if you like.

However, in the meantime, you may try to redefine the menu item all like:

[
    'text'        => 'all',
    'url'         => 'admin/pages?state=all', // or 'admin/pages?state=-1'
    'active'      => ['admin/pages?state=all'], // or 'admin/pages?state=-1'
],

And try to detect/implement that filter on the requested route/controller.

Other approach is to only have one menu item that access all the content of admin/pages without filters, and implement the filters on the same view with a redirection with query parameters. This is the approach I commonly use for these cases...

[AMoktar]

https://github.com/jeroennoten/Laravel-AdminLTE/blob/39da95927bce189e56998c606b2131bd6130c094/src/Menu/ActiveChecker.php#L139

        // If pattern is not a regex, check if the requested url matches the full url request.
        $request = $this->request->fullUrl();

This fixed the issue, but I couldn't manage to run test. Can you please tell how to run tests here ? Thanks

[dfsmania]

@AMoktar Tests will run once the PR is approved to run...

I have reviewed the history of changes of the ActiveChecker class and the logic you just changed was created that way to solve some previous issues. We can't just change it this way, because we will break the backward compatibility of the package.

Note that when you define the url atrributte of a menu item like foo/bar it should be active when the requested url is https://your.domain.com/foo/bar?param1=val1&param2=val2, since the base url is foo/bar, query parameters are just extra data for the base url. This is the reason the comparison to the full url is only made when query parameters are provided on the lookup pattern.

When I get some free time, I will try to check for other alternatives that don't break the backward compatibility...

[dfsmania]

Hi @AMoktar have you found a workaround for your situation without changing the logic of the ActiveChecker class?

I have concluded that adding support for your very particular situation will complicate too much the logic of the ActiveChecker class without any sense, because generally query parameters are used to provide extra information to the request URL, i.e., the base path of the requested URL is always the same and its usually considered to be the same page but with extra information.

So, by default, it's preferred that a menu entry with url=foo/bar will be active when the requested path is for/bar?param1=val1.

However, as I commented before, if you want to keep with your initial menu style, then you may redefine your menu entries like:

[
    'text' => 'pages',
    'submenu' => [
        [
            'text' => 'review',
            'url' => 'admin/pages?state=0',
            'active' => ['admin/pages?state=0],
        ],
        [
            'text' => 'active',
            'url' => 'admin/pages?state=1',
           'active' => ['admin/pages?state=1'],
        ],
        [
            'text' => 'all',
            'url' => 'admin/pages?state=all',
        ],
    ],
],

Then, you may create an extra route like:

Route::get(
    '/admin/pages?state=all',
    [App\Http\Controllers\<ControllerName::class>, '<controller_method>']
);

Where the controller class and the controller method will be the same you are using actually for the definition of admin/pages.

[AMoktar]

Thanks @dfsmania for your efforts I really appreciate that. Actually I didn't find any workaround. I'll give it a try .

[dfsmania]

@AMoktar Did you solve this? Can we close this issue?

[dfsmania]

Closed due to no more feedback. Also, the issue was related to a very particular use case.