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

Add documentation for sulu_navigation_is_active Twig function #702

Open
sanderquirynen opened this issue Mar 24, 2022 · 4 comments
Open

Comments

@sanderquirynen
Copy link

There seems to be a very handy Twig function sulu_navigation_is_active, but it's not documented anywhere.
It's also not used in the demo project (as far as I can tell), so I assume a lot of people will miss it.

I'd be happy to create a pull-request, but I'm a bit clueless on how to do this.

@alexander-schranz
Copy link
Member

Must say I did not know about that one but if I look at the pull request of it there are some edge cases which maybe don't work with it, example when working together with static routes or none sulu page routes. As in that case request.resourceLocator could not be set:

Mostly a starts with is an easier and more bulletproof solution here like its done in the demo:

https://github.com/sulu/sulu-demo/blob/bd4c742e6d5e7fb5fdff050bd829e90023431c18/templates/includes/navbar.html.twig#L17

So not sure if we should maybe deprecate it 🤔

@sanderquirynen
Copy link
Author

Thats what we use most of the time as well. I came across the above function while looking for a solution to shorten all my if-statements in Twig (since I noticed I was writing "app.request.uri starts with" a lot).

I ended up using something like this

public function navIsActive($itemPath): bool
{
    $path = $this->requestStack->getCurrentRequest()->getPathInfo();

    return str_starts_with($path, $itemPath);
}

I'm not sure if this kind of function is a Sulu-responsibility though.

@alexander-schranz
Copy link
Member

alexander-schranz commented Mar 24, 2022

You should give the full Uri into the navIsActive and use the full uri of the currentRequest. Else you could have false active elements when having multiple webspaces and links to other webspaces in your application navigation.

E.g.:

{% set itemUrl = sulu_content_path(item.url, item.webspace) %}
{% if sulu_navigation_is_active(itemUrl) %}
return str_starts_with($itemUrl, $this->requestStack->getCurrentRequest()->getUri());

@rogamoore
Copy link
Contributor

This is our current implementation, which allows either exact match (but ignoring query params) or sub path matching (e.g. when on https://www.example.com/somepath/subpath, https://www.example.com/somepath is considered active).

It also takes care of an edge case that can happen when using just a simple str_starts_with:

final readonly class NavigationHelperTwigRuntime implements RuntimeExtensionInterface
{
    public function __construct(
        private RequestStack $requestStack,
    ) {}

    public function isActiveLink(string $targetUrl, bool $allowSubPath = true): bool
    {
        $currentRequest = $this->requestStack->getCurrentRequest();

        if (null === $currentRequest) {
            return false;
        }

        $requestUrl =
            $currentRequest->getSchemeAndHttpHost().$currentRequest->getBaseUrl().$currentRequest->getPathInfo();

        return $requestUrl === $targetUrl || ($allowSubPath && $this->isSubPath(
            $requestUrl,
            $targetUrl,
        ));
    }

    private function isSubPath(string $requestUrl, string $targetUrl): bool
    {
        return str_starts_with($requestUrl, $targetUrl) && '/' === $requestUrl[strlen($targetUrl)];
    }
}

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

No branches or pull requests

3 participants