sulu / sulu-docs

Sulu documentation
https://docs.sulu.io
22 stars 118 forks source link

Add documentation for sulu_navigation_is_active Twig function #702

Open sanderquirynen opened 2 years ago

sanderquirynen commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 9 months ago

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)];
    }
}